From af81ceae3851f1d3a38f1a30a08c6db39f3779d9 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 16 Nov 2022 13:54:48 -0600 Subject: [PATCH 1/4] Add method to grab default compilation width of a number --- crates/compiler/mono/src/layout.rs | 27 +++------------------------ crates/compiler/types/src/num.rs | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/crates/compiler/mono/src/layout.rs b/crates/compiler/mono/src/layout.rs index 64e13f3bd2..872914c286 100644 --- a/crates/compiler/mono/src/layout.rs +++ b/crates/compiler/mono/src/layout.rs @@ -2276,33 +2276,12 @@ impl<'a> Layout<'a> { env: &mut Env<'a, '_>, range: NumericRange, ) -> Cacheable> { - use roc_types::num::IntLitWidth; - - // If we chose the default int layout then the real var might have been `Num *`, or - // similar. In this case fix-up width if we need to. Choose I64 if the range says - // that the number will fit, otherwise choose the next-largest number layout. - // // We don't pass the range down because `RangedNumber`s are somewhat rare, they only // appear due to number literals, so no need to increase parameter list sizes. - let num_layout = match range { - NumericRange::IntAtLeastSigned(w) | NumericRange::NumAtLeastSigned(w) => { - [IntLitWidth::I64, IntLitWidth::I128] - .iter() - .find(|candidate| candidate.is_superset(&w, true)) - .expect("if number doesn't fit, should have been a type error") - } - NumericRange::IntAtLeastEitherSign(w) | NumericRange::NumAtLeastEitherSign(w) => [ - IntLitWidth::I64, - IntLitWidth::U64, - IntLitWidth::I128, - IntLitWidth::U128, - ] - .iter() - .find(|candidate| candidate.is_superset(&w, false)) - .expect("if number doesn't fit, should have been a type error"), - }; + let num_layout = range.default_compilation_width(); + cacheable(Ok(Layout::int_literal_width_to_int( - *num_layout, + num_layout, env.target_info, ))) } diff --git a/crates/compiler/types/src/num.rs b/crates/compiler/types/src/num.rs index 88ba52e37f..cd4879b31c 100644 --- a/crates/compiler/types/src/num.rs +++ b/crates/compiler/types/src/num.rs @@ -121,6 +121,29 @@ impl NumericRange { } } } + + /// Chooses the int width to compile this ranged number into. + /// I64 is chosen if the range says that the number will fit, + /// otherwise the next-largest number layout is chosen. + pub fn default_compilation_width(&self) -> IntLitWidth { + *match self { + NumericRange::IntAtLeastSigned(w) | NumericRange::NumAtLeastSigned(w) => { + [IntLitWidth::I64, IntLitWidth::I128] + .iter() + .find(|candidate| candidate.is_superset(&w, true)) + .expect("if number doesn't fit, should have been a type error") + } + NumericRange::IntAtLeastEitherSign(w) | NumericRange::NumAtLeastEitherSign(w) => [ + IntLitWidth::I64, + IntLitWidth::U64, + IntLitWidth::I128, + IntLitWidth::U128, + ] + .iter() + .find(|candidate| candidate.is_superset(&w, false)) + .expect("if number doesn't fit, should have been a type error"), + } + } } #[derive(Clone, Copy, PartialEq, Eq, Debug)] From 330504131646b9dada8373bfd5d9e8cb289910b1 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 16 Nov 2022 13:55:15 -0600 Subject: [PATCH 2/4] Add Debug derives in lambda set compaction --- crates/compiler/derive_key/src/lib.rs | 2 +- crates/compiler/solve/src/specialize.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/compiler/derive_key/src/lib.rs b/crates/compiler/derive_key/src/lib.rs index a994a622e3..c9b6e3731f 100644 --- a/crates/compiler/derive_key/src/lib.rs +++ b/crates/compiler/derive_key/src/lib.rs @@ -71,7 +71,7 @@ pub enum Derived { } /// The builtin ability member to derive. -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug)] pub enum DeriveBuiltin { ToEncoder, Decoder, diff --git a/crates/compiler/solve/src/specialize.rs b/crates/compiler/solve/src/specialize.rs index 49b39eb0d4..53db854639 100644 --- a/crates/compiler/solve/src/specialize.rs +++ b/crates/compiler/solve/src/specialize.rs @@ -605,6 +605,7 @@ enum SpecializationTypeKey { SingleLambdaSetImmediate(Symbol), } +#[derive(Debug)] enum SpecializeDecision { Specialize(SpecializationTypeKey), Drop, From 9c8a4ec027749c2ca276160366c4ac4d618f73d0 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 16 Nov 2022 13:57:03 -0600 Subject: [PATCH 3/4] Choose hash implementation for ranged number based on default width Closes #4416 --- crates/compiler/derive_key/src/hash.rs | 90 ++++++++++++++--------- crates/compiler/solve/tests/solve_expr.rs | 16 ++++ crates/compiler/types/src/num.rs | 21 ++++++ 3 files changed, 94 insertions(+), 33 deletions(-) diff --git a/crates/compiler/derive_key/src/hash.rs b/crates/compiler/derive_key/src/hash.rs index 94842e669f..7eec881038 100644 --- a/crates/compiler/derive_key/src/hash.rs +++ b/crates/compiler/derive_key/src/hash.rs @@ -105,42 +105,29 @@ impl FlatHash { // FlatType::Func(..) => Err(Underivable), }, - Content::Alias(sym, _, real_var, _) => match sym { - Symbol::NUM_U8 | Symbol::NUM_UNSIGNED8 => { - Ok(SingleLambdaSetImmediate(Symbol::HASH_ADD_U8)) - } - Symbol::NUM_U16 | Symbol::NUM_UNSIGNED16 => { - Ok(SingleLambdaSetImmediate(Symbol::HASH_ADD_U16)) - } - Symbol::NUM_U32 | Symbol::NUM_UNSIGNED32 => { - Ok(SingleLambdaSetImmediate(Symbol::HASH_ADD_U32)) - } - Symbol::NUM_U64 | Symbol::NUM_UNSIGNED64 => { - Ok(SingleLambdaSetImmediate(Symbol::HASH_ADD_U64)) - } - Symbol::NUM_U128 | Symbol::NUM_UNSIGNED128 => { - Ok(SingleLambdaSetImmediate(Symbol::HASH_ADD_U128)) - } - Symbol::NUM_I8 | Symbol::NUM_SIGNED8 => { - Ok(SingleLambdaSetImmediate(Symbol::HASH_HASH_I8)) - } - Symbol::NUM_I16 | Symbol::NUM_SIGNED16 => { - Ok(SingleLambdaSetImmediate(Symbol::HASH_HASH_I16)) - } - Symbol::NUM_I32 | Symbol::NUM_SIGNED32 => { - Ok(SingleLambdaSetImmediate(Symbol::HASH_HASH_I32)) - } - Symbol::NUM_I64 | Symbol::NUM_SIGNED64 => { - Ok(SingleLambdaSetImmediate(Symbol::HASH_HASH_I64)) - } - Symbol::NUM_I128 | Symbol::NUM_SIGNED128 => { - Ok(SingleLambdaSetImmediate(Symbol::HASH_HASH_I128)) - } + Content::Alias(sym, _, real_var, _) => match num_symbol_to_hash_lambda(sym) { + Some(lambda) => Ok(lambda), // NB: I believe it is okay to unwrap opaques here because derivers are only used // by the backend, and the backend treats opaques like structural aliases. - _ => Self::from_var(subs, real_var), + None => Self::from_var(subs, real_var), }, - Content::RangedNumber(_) => Err(Underivable), + Content::RangedNumber(range) => { + // Find the integer we're going to compile to, that'll tell us what lambda we + // should resolve to. + // + // Note that at this point, we don't need to update the underlying type variable. + // That's because + // + // - If the type variable always had a ground constructor after solving, we would + // have already refined the ranged number during obligation checking. + // + // - If the type variable was generalized, then this branch is only reached + // during monomorphization, at which point we always choose a default layout + // for ranged numbers, without concern for reification to a ground type. + let chosen_width = range.default_compilation_width(); + let lambda = num_symbol_to_hash_lambda(chosen_width.symbol()).unwrap(); + Ok(lambda) + } // Content::RecursionVar { structure, .. } => Self::from_var(subs, structure), // @@ -153,3 +140,40 @@ impl FlatHash { } } } + +const fn num_symbol_to_hash_lambda(symbol: Symbol) -> Option { + use FlatHash::*; + match symbol { + Symbol::NUM_U8 | Symbol::NUM_UNSIGNED8 => { + Some(SingleLambdaSetImmediate(Symbol::HASH_ADD_U8)) + } + Symbol::NUM_U16 | Symbol::NUM_UNSIGNED16 => { + Some(SingleLambdaSetImmediate(Symbol::HASH_ADD_U16)) + } + Symbol::NUM_U32 | Symbol::NUM_UNSIGNED32 => { + Some(SingleLambdaSetImmediate(Symbol::HASH_ADD_U32)) + } + Symbol::NUM_U64 | Symbol::NUM_UNSIGNED64 => { + Some(SingleLambdaSetImmediate(Symbol::HASH_ADD_U64)) + } + Symbol::NUM_U128 | Symbol::NUM_UNSIGNED128 => { + Some(SingleLambdaSetImmediate(Symbol::HASH_ADD_U128)) + } + Symbol::NUM_I8 | Symbol::NUM_SIGNED8 => { + Some(SingleLambdaSetImmediate(Symbol::HASH_HASH_I8)) + } + Symbol::NUM_I16 | Symbol::NUM_SIGNED16 => { + Some(SingleLambdaSetImmediate(Symbol::HASH_HASH_I16)) + } + Symbol::NUM_I32 | Symbol::NUM_SIGNED32 => { + Some(SingleLambdaSetImmediate(Symbol::HASH_HASH_I32)) + } + Symbol::NUM_I64 | Symbol::NUM_SIGNED64 => { + Some(SingleLambdaSetImmediate(Symbol::HASH_HASH_I64)) + } + Symbol::NUM_I128 | Symbol::NUM_SIGNED128 => { + Some(SingleLambdaSetImmediate(Symbol::HASH_HASH_I128)) + } + _ => None, + } +} diff --git a/crates/compiler/solve/tests/solve_expr.rs b/crates/compiler/solve/tests/solve_expr.rs index 74e72ff688..bac6ca4b2f 100644 --- a/crates/compiler/solve/tests/solve_expr.rs +++ b/crates/compiler/solve/tests/solve_expr.rs @@ -8220,4 +8220,20 @@ mod solve_expr { "{} -> Task", ) } + + #[test] + fn choose_ranged_num_for_hash() { + infer_queries!( + indoc!( + r#" + app "test" provides [main] to "./platform" + + main = + \h -> Hash.hash h 7 + # ^^^^^^^^^ + "# + ), + @"Hash#Hash.hash(1) : a, I64 -[[Hash.hashI64(12)]]-> a | a has Hasher" + ) + } } diff --git a/crates/compiler/types/src/num.rs b/crates/compiler/types/src/num.rs index cd4879b31c..1306db905f 100644 --- a/crates/compiler/types/src/num.rs +++ b/crates/compiler/types/src/num.rs @@ -1,3 +1,5 @@ +use roc_module::symbol::Symbol; + use crate::subs::Variable; /// A bound placed on a number because of its literal value. @@ -304,6 +306,25 @@ impl IntLitWidth { } } } + + pub const fn symbol(&self) -> Symbol { + match self { + IntLitWidth::U8 => Symbol::NUM_U8, + IntLitWidth::U16 => Symbol::NUM_U16, + IntLitWidth::U32 => Symbol::NUM_U32, + IntLitWidth::U64 => Symbol::NUM_U64, + IntLitWidth::U128 => Symbol::NUM_U128, + IntLitWidth::I8 => Symbol::NUM_I8, + IntLitWidth::I16 => Symbol::NUM_I16, + IntLitWidth::I32 => Symbol::NUM_I32, + IntLitWidth::I64 => Symbol::NUM_I64, + IntLitWidth::I128 => Symbol::NUM_I128, + IntLitWidth::Nat => Symbol::NUM_NAT, + IntLitWidth::F32 => Symbol::NUM_F32, + IntLitWidth::F64 => Symbol::NUM_F64, + IntLitWidth::Dec => Symbol::NUM_DEC, + } + } } #[derive(Clone, Copy, PartialEq, Eq, Debug)] From f7e03830584940d4250e6e2e78e52f9fd02a6f6b Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 16 Nov 2022 13:59:11 -0600 Subject: [PATCH 4/4] Drop dead reference --- crates/compiler/types/src/num.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/compiler/types/src/num.rs b/crates/compiler/types/src/num.rs index 1306db905f..5a85e0a1bb 100644 --- a/crates/compiler/types/src/num.rs +++ b/crates/compiler/types/src/num.rs @@ -132,7 +132,7 @@ impl NumericRange { NumericRange::IntAtLeastSigned(w) | NumericRange::NumAtLeastSigned(w) => { [IntLitWidth::I64, IntLitWidth::I128] .iter() - .find(|candidate| candidate.is_superset(&w, true)) + .find(|candidate| candidate.is_superset(w, true)) .expect("if number doesn't fit, should have been a type error") } NumericRange::IntAtLeastEitherSign(w) | NumericRange::NumAtLeastEitherSign(w) => [ @@ -142,7 +142,7 @@ impl NumericRange { IntLitWidth::U128, ] .iter() - .find(|candidate| candidate.is_superset(&w, false)) + .find(|candidate| candidate.is_superset(w, false)) .expect("if number doesn't fit, should have been a type error"), } }