From 38e8e6c59e131590a3258660b1cbc79e34a9016e Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 1 Jan 2025 17:27:45 -0600 Subject: [PATCH 01/10] Bugfix association of inferred-tag-extension variables These are inferred vars, not rigids. --- crates/compiler/constrain/src/expr.rs | 44 +++++++++++++++++++-------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/crates/compiler/constrain/src/expr.rs b/crates/compiler/constrain/src/expr.rs index a90566161d..d27e47d56f 100644 --- a/crates/compiler/constrain/src/expr.rs +++ b/crates/compiler/constrain/src/expr.rs @@ -1959,6 +1959,7 @@ fn constrain_function_def( signature, new_rigid_variables, new_infer_variables, + has_explicit_inference_variables: _, } = instantiate_rigids_simple( types, &annotation.signature, @@ -2313,6 +2314,7 @@ fn constrain_destructure_def( signature, new_rigid_variables, new_infer_variables, + has_explicit_inference_variables: _, } = instantiate_rigids( types, constraints, @@ -2420,6 +2422,7 @@ fn constrain_value_def( signature, new_rigid_variables, new_infer_variables, + has_explicit_inference_variables: _, } = instantiate_rigids_simple( types, &annotation.signature, @@ -2913,6 +2916,7 @@ fn constrain_typed_def( signature, new_rigid_variables, new_infer_variables, + has_explicit_inference_variables: _, } = instantiate_rigids( types, constraints, @@ -3752,8 +3756,13 @@ fn constrain_closure_size( pub struct InstantiateRigids { pub signature: Index, - pub new_rigid_variables: Vec, - pub new_infer_variables: Vec, + pub new_rigid_variables: Vec>, + pub new_infer_variables: Vec>, + /// Whether the annotation has explicit inference variables `_`. + /// Annotations with inference variables are handled specially during typechecking of mutually recursive defs, + /// because they are not guaranteed to be generalized (XREF(rec-def-strategy)). + /// Ideally, this special-casing would be removed in the future. + pub has_explicit_inference_variables: bool, } #[derive(PartialEq, Eq)] @@ -3802,19 +3811,23 @@ fn instantiate_rigids( new_rigid_variables.extend(introduced_vars.lambda_sets.iter().copied()); // ext-infer vars are always freshly introduced in this annotation - new_rigid_variables.extend(introduced_vars.infer_ext_in_output.iter().copied()); + new_infer_variables.extend(introduced_vars.infer_ext_in_output.iter().copied()); - new_infer_variables.extend(introduced_vars.inferred.iter().map(|v| v.value)); + let has_explicit_inference_variables = !introduced_vars.inferred.is_empty(); + new_infer_variables.extend(introduced_vars.inferred.iter().copied()); // Instantiate rigid variables if !rigid_substitution.is_empty() { annotation.substitute_variables(&rigid_substitution); } - types.from_old_type(&annotation) + ( + types.from_old_type(&annotation), + has_explicit_inference_variables, + ) }; - let signature = generate_fresh_ann(types); + let (signature, has_explicit_inference_variables) = generate_fresh_ann(types); { // If this is a recursive def, we must also generate a fresh annotation to be used as the // type annotation that will be used in the first def headers introduced during the solving @@ -3825,7 +3838,7 @@ fn instantiate_rigids( // So, we generate a fresh annotation here, and return a separate fresh annotation below; // the latter annotation is the one used to construct the finalized type. let annotation_index = if is_recursive_def == IsRecursiveDef::Yes { - generate_fresh_ann(types) + generate_fresh_ann(types).0 } else { signature }; @@ -3848,6 +3861,7 @@ fn instantiate_rigids( signature, new_rigid_variables, new_infer_variables, + has_explicit_inference_variables, } } @@ -3883,11 +3897,11 @@ fn instantiate_rigids_simple( // lambda set vars are always freshly introduced in this annotation new_rigid_variables.extend(introduced_vars.lambda_sets.iter().copied()); - // ext-infer vars are always freshly introduced in this annotation - new_rigid_variables.extend(introduced_vars.infer_ext_in_output.iter().copied()); + let has_explicit_inference_variables = !introduced_vars.inferred.is_empty(); + let mut new_infer_variables: Vec> = introduced_vars.inferred.clone(); - let new_infer_variables: Vec = - introduced_vars.inferred.iter().map(|v| v.value).collect(); + // ext-infer vars are always freshly introduced in this annotation + new_infer_variables.extend(introduced_vars.infer_ext_in_output.iter().copied()); // Instantiate rigid variables if !rigid_substitution.is_empty() { @@ -3898,6 +3912,7 @@ fn instantiate_rigids_simple( signature: types.from_old_type(&annotation), new_rigid_variables, new_infer_variables, + has_explicit_inference_variables, } } @@ -3981,6 +3996,7 @@ fn constraint_recursive_function( signature, new_rigid_variables, new_infer_variables, + has_explicit_inference_variables: _, } = instantiate_rigids_simple( types, &annotation.signature, @@ -4250,6 +4266,7 @@ pub fn rec_defs_help_simple( signature, new_rigid_variables, new_infer_variables, + has_explicit_inference_variables, } = instantiate_rigids_simple( types, &annotation.signature, @@ -4260,7 +4277,7 @@ pub fn rec_defs_help_simple( let loc_pattern = Loc::at(loc_symbol.region, Pattern::Identifier(loc_symbol.value)); - let is_hybrid = !new_infer_variables.is_empty(); + let is_hybrid = has_explicit_inference_variables; hybrid_and_flex_info.vars.extend(new_infer_variables); @@ -4537,6 +4554,7 @@ fn rec_defs_help( signature, new_rigid_variables, new_infer_variables, + has_explicit_inference_variables, } = instantiate_rigids( types, constraints, @@ -4548,7 +4566,7 @@ fn rec_defs_help( IsRecursiveDef::Yes, ); - let is_hybrid = !new_infer_variables.is_empty(); + let is_hybrid = has_explicit_inference_variables; hybrid_and_flex_info.vars.extend(&new_infer_variables); From 280d479a245a62403013a571586015a267a3c476 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 1 Jan 2025 18:15:30 -0600 Subject: [PATCH 02/10] Store rigid vars with location during constraining --- crates/compiler/can/src/constraint.rs | 35 +++++++-- crates/compiler/constrain/src/expr.rs | 99 ++++++++++++++----------- crates/compiler/constrain/src/module.rs | 10 ++- crates/compiler/solve/src/solve.rs | 18 ++--- 4 files changed, 101 insertions(+), 61 deletions(-) diff --git a/crates/compiler/can/src/constraint.rs b/crates/compiler/can/src/constraint.rs index c217e4adc1..f03a82afe0 100644 --- a/crates/compiler/can/src/constraint.rs +++ b/crates/compiler/can/src/constraint.rs @@ -18,6 +18,7 @@ pub struct Constraints { pub constraints: Vec, pub type_slices: Vec, pub variables: Vec, + loc_variables: Vec>, pub loc_symbols: Vec<(Symbol, Region)>, pub let_constraints: Vec, pub categories: Vec, @@ -42,6 +43,7 @@ impl std::fmt::Debug for Constraints { .field("types", &"") .field("type_slices", &self.type_slices) .field("variables", &self.variables) + .field("loc_variables", &self.loc_variables) .field("loc_symbols", &self.loc_symbols) .field("let_constraints", &self.let_constraints) .field("categories", &self.categories) @@ -75,6 +77,7 @@ impl Constraints { let constraints = Vec::new(); let type_slices = Vec::with_capacity(16); let variables = Vec::new(); + let loc_variables = Vec::new(); let loc_symbols = Vec::new(); let let_constraints = Vec::new(); let mut categories = Vec::with_capacity(16); @@ -126,6 +129,7 @@ impl Constraints { constraints, type_slices, variables, + loc_variables, loc_symbols, let_constraints, categories, @@ -377,6 +381,17 @@ impl Constraints { Slice::new(start as _, length as _) } + pub fn loc_variable_slice(&mut self, it: I) -> Slice> + where + I: IntoIterator>, + { + let start = self.loc_variables.len(); + self.loc_variables.extend(it); + let length = self.loc_variables.len() - start; + + Slice::new(start as _, length as _) + } + fn def_types_slice(&mut self, it: I) -> DefTypes where I: IntoIterator)>, @@ -473,8 +488,8 @@ impl Constraints { generalizable: Generalizable, ) -> Constraint where - I1: IntoIterator, - I2: IntoIterator, + I1: IntoIterator>, + I2: IntoIterator>, I3: IntoIterator)>, I3::IntoIter: ExactSizeIterator, { @@ -485,8 +500,8 @@ impl Constraints { self.constraints.push(ret_constraint); let let_constraint = LetConstraint { - rigid_vars: self.variable_slice(rigid_vars), - flex_vars: self.variable_slice(flex_vars), + rigid_vars: self.loc_variable_slice(rigid_vars), + flex_vars: self.variable_slice(flex_vars.into_iter().map(|v| v.value)), def_types: self.def_types_slice(def_types), defs_and_ret_constraint, generalizable, @@ -534,7 +549,7 @@ impl Constraints { self.constraints.push(module_constraint); let let_contraint = LetConstraint { - rigid_vars: self.variable_slice(rigid_vars), + rigid_vars: self.loc_variable_slice(rigid_vars.into_iter().map(Loc::at_zero)), flex_vars: self.variable_slice(flex_vars), def_types: self.def_types_slice(def_types), defs_and_ret_constraint, @@ -809,6 +824,14 @@ impl std::ops::Index for Constraints { } } +impl std::ops::Index>> for Constraints { + type Output = [Loc]; + + fn index(&self, slice: Slice>) -> &Self::Output { + &self.loc_variables[slice.indices()] + } +} + #[derive(Clone, Copy, Debug)] pub struct Eq( pub TypeOrVar, @@ -914,7 +937,7 @@ pub struct Generalizable(pub bool); #[derive(Debug, Clone)] pub struct LetConstraint { - pub rigid_vars: Slice, + pub rigid_vars: Slice>, pub flex_vars: Slice, pub def_types: DefTypes, pub defs_and_ret_constraint: Index<(Constraint, Constraint)>, diff --git a/crates/compiler/constrain/src/expr.rs b/crates/compiler/constrain/src/expr.rs index d27e47d56f..0e7254d950 100644 --- a/crates/compiler/constrain/src/expr.rs +++ b/crates/compiler/constrain/src/expr.rs @@ -37,8 +37,8 @@ use soa::{Index, Slice}; /// This is for constraining Defs #[derive(Default, Debug)] -pub struct Info { - pub vars: Vec, +struct Info { + pub vars: Vec>, pub constraints: Vec, pub def_types: VecMap>, } @@ -224,7 +224,7 @@ fn constrain_untyped_closure( let cons = [ constraints.let_constraint( [], - pattern_state.vars, + pattern_state.vars.into_iter().map(Loc::at_zero), pattern_state.headers, pattern_state_constraints, returns_constraint, @@ -1273,7 +1273,7 @@ pub fn constrain_expr( let body_constraints = constraints.and_constraint(body_cons); let when_body_con = constraints.let_constraint( [], - pattern_vars, + pattern_vars.into_iter().map(Loc::at_zero), pattern_headers, pattern_constraints, body_constraints, @@ -2202,7 +2202,7 @@ fn constrain_function_def( ), constraints.let_constraint( [], - argument_pattern_state.vars, + argument_pattern_state.vars.into_iter().map(Loc::at_zero), argument_pattern_state.headers, defs_constraint, returns_constraint, @@ -3065,7 +3065,7 @@ fn constrain_typed_def( constraints.store(fx_type_index, fx_var, std::file!(), std::line!()), constraints.let_constraint( [], - argument_pattern_state.vars, + argument_pattern_state.vars.into_iter().map(Loc::at_zero), argument_pattern_state.headers, defs_constraint, returns_constraint, @@ -3624,14 +3624,15 @@ fn constrain_stmt_def( /// Recursive defs should always use `constrain_recursive_defs`. pub(crate) fn constrain_def_make_constraint( constraints: &mut Constraints, - annotation_rigid_variables: impl Iterator, - annotation_infer_variables: impl Iterator, + annotation_rigid_variables: impl Iterator>, + annotation_infer_variables: impl Iterator>, def_expr_con: Constraint, after_def_con: Constraint, def_pattern_state: PatternState, generalizable: Generalizable, ) -> Constraint { - let all_flex_variables = (def_pattern_state.vars.into_iter()).chain(annotation_infer_variables); + let all_flex_variables = + (def_pattern_state.vars.into_iter().map(Loc::at_zero)).chain(annotation_infer_variables); let pattern_constraints = constraints.and_constraint(def_pattern_state.constraints); let def_pattern_and_body_con = constraints.and_constraint([pattern_constraints, def_expr_con]); @@ -3648,8 +3649,8 @@ pub(crate) fn constrain_def_make_constraint( fn constrain_value_def_make_constraint( constraints: &mut Constraints, - new_rigid_variables: Vec, - new_infer_variables: Vec, + new_rigid_variables: Vec>, + new_infer_variables: Vec>, expr_con: Constraint, body_con: Constraint, symbol: Loc, @@ -3670,7 +3671,7 @@ fn constrain_value_def_make_constraint( constraints.let_constraint( new_rigid_variables, - [expr_var], + [Loc::at(symbol.region, expr_var)], headers, def_con, body_con, @@ -3680,8 +3681,8 @@ fn constrain_value_def_make_constraint( fn constrain_function_def_make_constraint( constraints: &mut Constraints, - new_rigid_variables: Vec, - new_infer_variables: Vec, + new_rigid_variables: Vec>, + new_infer_variables: Vec>, expr_con: Constraint, body_con: Constraint, def_pattern_state: PatternState, @@ -3699,7 +3700,7 @@ fn constrain_function_def_make_constraint( constraints.let_constraint( new_rigid_variables, - def_pattern_state.vars, + def_pattern_state.vars.into_iter().map(Loc::at_zero), def_pattern_state.headers, def_con, body_con, @@ -3781,8 +3782,8 @@ fn instantiate_rigids( headers: &mut VecMap>, is_recursive_def: IsRecursiveDef, ) -> InstantiateRigids { - let mut new_rigid_variables = vec![]; - let mut new_infer_variables = vec![]; + let mut new_rigid_variables: Vec> = vec![]; + let mut new_infer_variables: Vec> = vec![]; let mut generate_fresh_ann = |types: &mut Types| { let mut annotation = annotation.clone(); @@ -3799,19 +3800,24 @@ fn instantiate_rigids( Vacant(vacant) => { // It's possible to use this rigid in nested defs vacant.insert(named.variable()); - new_rigid_variables.push(named.variable()); + new_rigid_variables.push(Loc::at(named.first_seen(), named.variable())); } } } // wildcards are always freshly introduced in this annotation - new_rigid_variables.extend(introduced_vars.wildcards.iter().map(|v| v.value)); + new_rigid_variables.extend(introduced_vars.wildcards.iter().copied()); // lambda set vars are always freshly introduced in this annotation - new_rigid_variables.extend(introduced_vars.lambda_sets.iter().copied()); + new_rigid_variables.extend(introduced_vars.lambda_sets.iter().map(|&v| Loc::at_zero(v))); // ext-infer vars are always freshly introduced in this annotation - new_infer_variables.extend(introduced_vars.infer_ext_in_output.iter().copied()); + new_infer_variables.extend( + introduced_vars + .infer_ext_in_output + .iter() + .map(|&v| Loc::at_zero(v)), + ); let has_explicit_inference_variables = !introduced_vars.inferred.is_empty(); new_infer_variables.extend(introduced_vars.inferred.iter().copied()); @@ -3872,7 +3878,7 @@ fn instantiate_rigids_simple( ftv: &mut MutMap, // rigids defined before the current annotation ) -> InstantiateRigids { let mut annotation = annotation.clone(); - let mut new_rigid_variables: Vec = Vec::new(); + let mut new_rigid_variables: Vec> = Vec::new(); let mut rigid_substitution: MutMap = MutMap::default(); for named in introduced_vars.iter_named() { @@ -3886,22 +3892,27 @@ fn instantiate_rigids_simple( Vacant(vacant) => { // It's possible to use this rigid in nested defs vacant.insert(named.variable()); - new_rigid_variables.push(named.variable()); + new_rigid_variables.push(Loc::at(named.first_seen(), named.variable())); } } } // wildcards are always freshly introduced in this annotation - new_rigid_variables.extend(introduced_vars.wildcards.iter().map(|v| v.value)); + new_rigid_variables.extend(introduced_vars.wildcards.iter().copied()); // lambda set vars are always freshly introduced in this annotation - new_rigid_variables.extend(introduced_vars.lambda_sets.iter().copied()); + new_rigid_variables.extend(introduced_vars.lambda_sets.iter().map(|&v| Loc::at_zero(v))); let has_explicit_inference_variables = !introduced_vars.inferred.is_empty(); let mut new_infer_variables: Vec> = introduced_vars.inferred.clone(); // ext-infer vars are always freshly introduced in this annotation - new_infer_variables.extend(introduced_vars.infer_ext_in_output.iter().copied()); + new_infer_variables.extend( + introduced_vars + .infer_ext_in_output + .iter() + .map(|&v| Loc::at_zero(v)), + ); // Instantiate rigid variables if !rigid_substitution.is_empty() { @@ -3979,7 +3990,7 @@ fn constraint_recursive_function( let expr_con = attach_resolution_constraints(constraints, env, expr_con); let def_con = expr_con; - flex_info.vars.push(expr_var); + flex_info.vars.push(Loc::at_zero(expr_var)); flex_info.constraints.push(def_con); flex_info.def_types.insert( loc_symbol.value, @@ -4125,7 +4136,7 @@ fn constraint_recursive_function( constraints.store(fx_type_index, fx_var, std::file!(), std::line!()), constraints.let_constraint( [], - argument_pattern_state.vars, + argument_pattern_state.vars.into_iter().map(Loc::at_zero), argument_pattern_state.headers, state_constraints, returns_constraint, @@ -4157,10 +4168,8 @@ fn constraint_recursive_function( // aligns with what the (mutually-)recursive signature says, so finish // generalization of the function. let rigids = new_rigid_variables; - let flex = def_pattern_state - .vars - .into_iter() - .chain(new_infer_variables); + let flex_pattern_vars = def_pattern_state.vars.into_iter().map(Loc::at_zero); + let flex = flex_pattern_vars.chain(new_infer_variables); constraints.let_constraint( rigids, @@ -4233,7 +4242,8 @@ pub fn rec_defs_help_simple( let opt_annotation = &declarations.annotations[index]; let loc_expr = &declarations.expressions[index]; - expr_regions.push(loc_expr.region); + let expr_region = loc_expr.region; + expr_regions.push(expr_region); match opt_annotation { None => { @@ -4251,7 +4261,9 @@ pub fn rec_defs_help_simple( let def_con = expr_con; - hybrid_and_flex_info.vars.push(expr_var); + hybrid_and_flex_info + .vars + .push(Loc::at(expr_region, expr_var)); hybrid_and_flex_info.constraints.push(def_con); hybrid_and_flex_info .def_types @@ -4329,7 +4341,7 @@ pub fn rec_defs_help_simple( rigid_info.constraints.push(constraints.let_constraint( new_rigid_variables, - [expr_var], + [Loc::at(expr_region, expr_var)], [], // no headers introduced (at this level) def_con, Constraint::True, @@ -4539,7 +4551,9 @@ fn rec_defs_help( let def_con = expr_con; - hybrid_and_flex_info.vars.extend(def_pattern_state.vars); + hybrid_and_flex_info + .vars + .extend(def_pattern_state.vars.into_iter().map(Loc::at_zero)); hybrid_and_flex_info.constraints.push(def_con); hybrid_and_flex_info .def_types @@ -4688,7 +4702,7 @@ fn rec_defs_help( constraints.store(fx_type_index, fx_var, std::file!(), std::line!()), constraints.let_constraint( [], - argument_pattern_state.vars, + argument_pattern_state.vars.into_iter().map(Loc::at_zero), argument_pattern_state.headers, state_constraints, returns_constraint, @@ -4724,7 +4738,6 @@ fn rec_defs_help( if is_hybrid { // TODO this is not quite right, types that are purely rigid should not // be stored as hybrid! - // However it might not be possible to fix this before types SoA lands. hybrid_and_flex_info.vars.extend(&new_rigid_variables); hybrid_and_flex_info.constraints.push(def_con); hybrid_and_flex_info @@ -4734,10 +4747,9 @@ fn rec_defs_help( rigid_info.vars.extend(&new_rigid_variables); let rigids = new_rigid_variables; - let flex = def_pattern_state - .vars - .into_iter() - .chain(new_infer_variables); + let flex_pattern_vars = + def_pattern_state.vars.into_iter().map(Loc::at_zero); + let flex = flex_pattern_vars.chain(new_infer_variables); rigid_info.constraints.push(constraints.let_constraint( rigids, @@ -4784,10 +4796,11 @@ fn rec_defs_help( .extend(def_pattern_state.headers); } else { rigid_info.vars.extend(&new_rigid_variables); + let flex_vars = def_pattern_state.vars.into_iter().map(Loc::at_zero); rigid_info.constraints.push(constraints.let_constraint( new_rigid_variables, - def_pattern_state.vars, + flex_vars, [], // no headers introduced (at this level) def_con, Constraint::True, diff --git a/crates/compiler/constrain/src/module.rs b/crates/compiler/constrain/src/module.rs index 09e29b5211..470039d4c8 100644 --- a/crates/compiler/constrain/src/module.rs +++ b/crates/compiler/constrain/src/module.rs @@ -83,7 +83,7 @@ fn constrain_params( let cons = constraints.let_constraint( [], - state.vars, + state.vars.into_iter().map(Loc::at_zero), state.headers, pattern_constraints, constraint, @@ -199,8 +199,12 @@ pub fn frontload_ability_constraints( def_pattern_state.vars.push(signature_var); - let rigid_variables = vars.rigid_vars.iter().chain(vars.able_vars.iter()).copied(); - let infer_variables = vars.flex_vars.iter().copied(); + let rigid_variables = (vars.rigid_vars.iter()) + .chain(vars.able_vars.iter()) + .copied() + .map(Loc::at_zero); + + let infer_variables = vars.flex_vars.iter().copied().map(Loc::at_zero); let signature_expectation = constraints.push_expected_type(Expected::NoExpectation(signature_index)); diff --git a/crates/compiler/solve/src/solve.rs b/crates/compiler/solve/src/solve.rs index 34078a6fec..a0e7a0f9ca 100644 --- a/crates/compiler/solve/src/solve.rs +++ b/crates/compiler/solve/src/solve.rs @@ -419,7 +419,7 @@ fn solve( // check that things went well dbg_do!(ROC_VERIFY_RIGID_LET_GENERALIZED, { - let rigid_vars = &env.constraints.variables[let_con.rigid_vars.indices()]; + let rigid_vars = &env.constraints[let_con.rigid_vars]; // NOTE the `subs.redundant` check does not come from elm. // It's unclear whether this is a bug with our implementation @@ -427,9 +427,9 @@ fn solve( // or that it just never came up in elm. let mut it = rigid_vars .iter() - .filter(|&var| { - !env.subs.redundant(*var) - && env.subs.get_rank(*var) != Rank::GENERALIZED + .filter(|loc_var| { + let var = loc_var.value; + !env.subs.redundant(var) && env.subs.get_rank(var) != Rank::GENERALIZED }) .peekable(); @@ -1007,11 +1007,11 @@ fn solve( let ret_constraint = &env.constraints.constraints[offset + 1]; let flex_vars = &env.constraints.variables[let_con.flex_vars.indices()]; - let rigid_vars = &env.constraints.variables[let_con.rigid_vars.indices()]; + let rigid_vars = &env.constraints[let_con.rigid_vars]; let pool_variables = &env.constraints.variables[pool_slice.indices()]; - if matches!(&ret_constraint, True) && let_con.rigid_vars.is_empty() { + if matches!(&ret_constraint, True) && rigid_vars.is_empty() { debug_assert!(pool_variables.is_empty()); env.introduce(rank, flex_vars); @@ -1025,7 +1025,7 @@ fn solve( }); state - } else if let_con.rigid_vars.is_empty() && let_con.flex_vars.is_empty() { + } else if rigid_vars.is_empty() && let_con.flex_vars.is_empty() { // items are popped from the stack in reverse order. That means that we'll // first solve then defs_constraint, and then (eventually) the ret_constraint. // @@ -1068,11 +1068,11 @@ fn solve( // Introduce the variables of this binding, and extend the pool at our binding // rank. - for &var in rigid_vars.iter().chain(flex_vars.iter()) { + for &var in rigid_vars.iter().map(|v| &v.value).chain(flex_vars.iter()) { env.subs.set_rank(var, binding_rank); } pool.reserve(rigid_vars.len() + flex_vars.len()); - pool.extend(rigid_vars.iter()); + pool.extend(rigid_vars.iter().map(|v| &v.value)); pool.extend(flex_vars.iter()); // Now, run our binding constraint, generalize, then solve the rest of the From c3d77b884102a981729d5247c0620f84b49f63bd Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 1 Jan 2025 21:46:11 -0600 Subject: [PATCH 03/10] Unify let-introduction in a single path Remove branches on determining how let-bindings are introduced to the scope. This is maybe a little more inefficient, but I think it is a huge simplification. One additional change this required was changing how fx suffixes are checked. The current implementation would add additional constraints for patterns in let bindings conditionally. However, this is unnecessary. I believe it is sufficient to check the fx suffix by running the checks on all introduced symbols after the type is well known (i.e. the body is checked). --- crates/compiler/can/src/constraint.rs | 17 -- crates/compiler/constrain/src/pattern.rs | 41 +-- crates/compiler/load/tests/test_reporting.rs | 4 +- crates/compiler/solve/src/solve.rs | 257 +++++++------------ 4 files changed, 99 insertions(+), 220 deletions(-) diff --git a/crates/compiler/can/src/constraint.rs b/crates/compiler/can/src/constraint.rs index f03a82afe0..569ea9a07b 100644 --- a/crates/compiler/can/src/constraint.rs +++ b/crates/compiler/can/src/constraint.rs @@ -619,23 +619,6 @@ impl Constraints { Constraint::FxCall(constraint_index) } - pub fn fx_pattern_suffix( - &mut self, - symbol: Symbol, - type_index: TypeOrVar, - region: Region, - ) -> Constraint { - let constraint = FxSuffixConstraint { - kind: FxSuffixKind::Pattern(symbol), - type_index, - region, - }; - - let constraint_index = index_push_new(&mut self.fx_suffix_constraints, constraint); - - Constraint::FxSuffix(constraint_index) - } - pub fn fx_record_field_unsuffixed(&mut self, variable: Variable, region: Region) -> Constraint { let type_index = Self::push_type_variable(variable); let constraint = FxSuffixConstraint { diff --git a/crates/compiler/constrain/src/pattern.rs b/crates/compiler/constrain/src/pattern.rs index d896ca9bf8..8c2d06c08c 100644 --- a/crates/compiler/constrain/src/pattern.rs +++ b/crates/compiler/constrain/src/pattern.rs @@ -6,7 +6,7 @@ use roc_can::pattern::Pattern::{self, *}; use roc_can::pattern::{DestructType, ListPatterns, RecordDestruct, TupleDestruct}; use roc_collections::all::{HumanIndex, SendMap}; use roc_collections::VecMap; -use roc_module::ident::{IdentSuffix, Lowercase}; +use roc_module::ident::Lowercase; use roc_module::symbol::Symbol; use roc_region::all::{Loc, Region}; use roc_types::subs::Variable; @@ -249,16 +249,7 @@ pub fn constrain_pattern( expected: PExpectedTypeIndex, state: &mut PatternState, ) { - constrain_pattern_help( - types, - constraints, - env, - pattern, - region, - expected, - state, - true, - ); + constrain_pattern_help(types, constraints, env, pattern, region, expected, state); } #[allow(clippy::too_many_arguments)] @@ -270,7 +261,6 @@ pub fn constrain_pattern_help( region: Region, expected: PExpectedTypeIndex, state: &mut PatternState, - is_shallow: bool, ) { match pattern { Underscore => { @@ -300,27 +290,6 @@ pub fn constrain_pattern_help( .push(constraints.is_open_type(type_index)); } - // Identifiers introduced in nested patterns get let constraints - // and therefore don't need fx_pattern_suffix constraints. - if is_shallow { - match symbol.suffix() { - IdentSuffix::None => { - // Unsuffixed identifiers should be constrained after we know if they're functions - state - .delayed_fx_suffix_constraints - .push(constraints.fx_pattern_suffix(*symbol, type_index, region)); - } - IdentSuffix::Bang => { - // Bang suffixed identifiers are always required to be functions - // We constrain this before the function's body, - // so that we don't think it's pure and complain about leftover statements - state - .constraints - .push(constraints.fx_pattern_suffix(*symbol, type_index, region)); - } - } - } - state.headers.insert( *symbol, Loc { @@ -350,7 +319,6 @@ pub fn constrain_pattern_help( subpattern.region, expected, state, - false, ) } @@ -584,7 +552,6 @@ pub fn constrain_pattern_help( loc_pattern.region, expected, state, - false, ); pat_type @@ -683,7 +650,6 @@ pub fn constrain_pattern_help( loc_guard.region, expected, state, - false, ); RecordField::Demanded(pat_type) @@ -807,7 +773,6 @@ pub fn constrain_pattern_help( loc_pat.region, expected, state, - false, ); } @@ -864,7 +829,6 @@ pub fn constrain_pattern_help( loc_pattern.region, expected, state, - false, ); } @@ -930,7 +894,6 @@ pub fn constrain_pattern_help( loc_arg_pattern.region, arg_pattern_expected, state, - false, ); // Next, link `whole_var` to the opaque type of "@Id who" diff --git a/crates/compiler/load/tests/test_reporting.rs b/crates/compiler/load/tests/test_reporting.rs index ca7be76442..7bb2c20ab9 100644 --- a/crates/compiler/load/tests/test_reporting.rs +++ b/crates/compiler/load/tests/test_reporting.rs @@ -15615,7 +15615,7 @@ All branches in an `if` must have the same type! @r###" ── MISSING EXCLAMATION in /code/proj/Main.roc ────────────────────────────────── - This is an effectful function, but its name does not indicate so: + This function is effectful, but its name does not indicate so: 10│ forEach! = \l, f -> ^ @@ -15652,7 +15652,7 @@ All branches in an `if` must have the same type! @r###" ── UNNECESSARY EXCLAMATION in /code/proj/Main.roc ────────────────────────────── - This is a pure function, but its name suggests otherwise: + This function is pure, but its name suggests otherwise: 12│ mapOk = \result, fn! -> ^^^ diff --git a/crates/compiler/solve/src/solve.rs b/crates/compiler/solve/src/solve.rs index a0e7a0f9ca..20f3e00d66 100644 --- a/crates/compiler/solve/src/solve.rs +++ b/crates/compiler/solve/src/solve.rs @@ -211,18 +211,7 @@ enum Work<'a> { constraint: &'a Constraint, }, CheckForInfiniteTypes(LocalDefVarsVec<(Symbol, Loc)>), - /// The ret_con part of a let constraint that does NOT introduces rigid and/or flex variables - LetConNoVariables { - scope: &'a Scope, - rank: Rank, - let_con: &'a LetConstraint, - - /// The variables used to store imported types in the Subs. - /// The `Contents` are copied from the source module, but to - /// mimic `type_to_var`, we must add these variables to `Pools` - /// at the correct rank - pool_variables: &'a [Variable], - }, + CheckSuffixFx(LocalDefVarsVec<(Symbol, Loc)>), /// The ret_con part of a let constraint that introduces rigid and/or flex variables /// /// These introduced variables must be generalized, hence this variant @@ -288,56 +277,6 @@ fn solve( continue; } - Work::LetConNoVariables { - scope, - rank, - let_con, - pool_variables, - } => { - // NOTE be extremely careful with shadowing here - let offset = let_con.defs_and_ret_constraint.index(); - let ret_constraint = &env.constraints.constraints[offset + 1]; - - // Add a variable for each def to new_vars_by_env. - let local_def_vars = LocalDefVarsVec::from_def_types( - env, - rank, - problems, - abilities_store, - obligation_cache, - &mut can_types, - aliases, - let_con.def_types, - ); - - env.pools.get_mut(rank).extend(pool_variables); - - let mut new_scope = scope.clone(); - for (symbol, loc_var) in local_def_vars.iter() { - check_ability_specialization( - env, - rank, - abilities_store, - obligation_cache, - awaiting_specializations, - problems, - *symbol, - *loc_var, - ); - - new_scope.insert_symbol_var_if_vacant(*symbol, loc_var.value); - } - - stack.push(Work::Constraint { - scope: env.arena.alloc(new_scope), - rank, - constraint: ret_constraint, - }); - // Check for infinite types first - stack.push(Work::CheckForInfiniteTypes(local_def_vars)); - - continue; - } Work::LetConIntroducesVariables { scope, rank, @@ -456,14 +395,8 @@ fn solve( new_scope.insert_symbol_var_if_vacant(*symbol, loc_var.value); - solve_suffix_fx( - env, - problems, - host_exposed_symbols, - FxSuffixKind::Let(*symbol), - loc_var.value, - &loc_var.region, - ); + // At the time of introduction, promote explicitly-effectful symbols. + promote_effectful_symbol(env, FxSuffixKind::Let(*symbol), loc_var.value); } // Note that this vars_by_symbol is the one returned by the @@ -473,20 +406,42 @@ fn solve( mark: final_mark, }; - // Now solve the body, using the new vars_by_symbol which includes - // the assignments' name-to-variable mappings. - stack.push(Work::Constraint { - scope: env.arena.alloc(new_scope), - rank, - constraint: ret_constraint, - }); - // Check for infinite types first - stack.push(Work::CheckForInfiniteTypes(local_def_vars)); + let next_work = [ + // Check for infinite types first + Work::CheckForInfiniteTypes(local_def_vars.clone()), + // Now solve the body, using the new vars_by_symbol which includes + // the assignments' name-to-variable mappings. + Work::Constraint { + scope: env.arena.alloc(new_scope), + rank, + constraint: ret_constraint, + }, + // Finally, check the suffix fx, after we have solved all types. + Work::CheckSuffixFx(local_def_vars), + ]; + + for work in next_work.into_iter().rev() { + stack.push(work); + } state = state_for_ret_con; continue; } + + Work::CheckSuffixFx(local_def_vars) => { + for (symbol, loc_var) in local_def_vars.iter() { + solve_suffix_fx( + env, + problems, + host_exposed_symbols, + FxSuffixKind::Let(*symbol), + loc_var.value, + &loc_var.region, + ); + } + continue; + } }; state = match constraint { @@ -1004,100 +959,64 @@ fn solve( let offset = let_con.defs_and_ret_constraint.index(); let defs_constraint = &env.constraints.constraints[offset]; - let ret_constraint = &env.constraints.constraints[offset + 1]; let flex_vars = &env.constraints.variables[let_con.flex_vars.indices()]; let rigid_vars = &env.constraints[let_con.rigid_vars]; let pool_variables = &env.constraints.variables[pool_slice.indices()]; - if matches!(&ret_constraint, True) && rigid_vars.is_empty() { - debug_assert!(pool_variables.is_empty()); - - env.introduce(rank, flex_vars); - - // If the return expression is guaranteed to solve, - // solve the assignments themselves and move on. - stack.push(Work::Constraint { - scope, - rank, - constraint: defs_constraint, - }); - - state - } else if rigid_vars.is_empty() && let_con.flex_vars.is_empty() { - // items are popped from the stack in reverse order. That means that we'll - // first solve then defs_constraint, and then (eventually) the ret_constraint. - // - // Note that the LetConSimple gets the current env and rank, - // and not the env/rank from after solving the defs_constraint - stack.push(Work::LetConNoVariables { - scope, - rank, - let_con, - pool_variables, - }); - stack.push(Work::Constraint { - scope, - rank, - constraint: defs_constraint, - }); - - state + // If the let-binding is generalizable, work at the next rank (which will be + // the rank at which introduced variables will become generalized, if they end up + // staying there); otherwise, stay at the current level. + let binding_rank = if let_con.generalizable.0 { + rank.next() } else { - // If the let-binding is generalizable, work at the next rank (which will be - // the rank at which introduced variables will become generalized, if they end up - // staying there); otherwise, stay at the current level. - let binding_rank = if let_con.generalizable.0 { - rank.next() - } else { - rank - }; + rank + }; - // determine the next pool - if binding_rank.into_usize() < env.pools.len() { - // Nothing to do, we already accounted for the next rank, no need to - // adjust the pools - } else { - // we should be off by one at this point - debug_assert_eq!(binding_rank.into_usize(), 1 + env.pools.len()); - env.pools.extend_to(binding_rank.into_usize()); - } + // determine the next pool + if binding_rank.into_usize() < env.pools.len() { + // Nothing to do, we already accounted for the next rank, no need to + // adjust the pools + } else { + // we should be off by one at this point + debug_assert_eq!(binding_rank.into_usize(), 1 + env.pools.len()); + env.pools.extend_to(binding_rank.into_usize()); + } - let pool: &mut Vec = env.pools.get_mut(binding_rank); + let pool: &mut Vec = env.pools.get_mut(binding_rank); - // Introduce the variables of this binding, and extend the pool at our binding - // rank. - for &var in rigid_vars.iter().map(|v| &v.value).chain(flex_vars.iter()) { - env.subs.set_rank(var, binding_rank); - } - pool.reserve(rigid_vars.len() + flex_vars.len()); - pool.extend(rigid_vars.iter().map(|v| &v.value)); - pool.extend(flex_vars.iter()); + // Introduce the variables of this binding, and extend the pool at our binding + // rank. + for &var in rigid_vars.iter().map(|v| &v.value).chain(flex_vars.iter()) { + env.subs.set_rank(var, binding_rank); + } + pool.reserve(rigid_vars.len() + flex_vars.len()); + pool.extend(rigid_vars.iter().map(|v| &v.value)); + pool.extend(flex_vars.iter()); - // Now, run our binding constraint, generalize, then solve the rest of the - // program. - // - // Items are popped from the stack in reverse order. That means that we'll - // first solve the defs_constraint, and then (eventually) the ret_constraint. - // - // NB: LetCon gets the current scope's env and rank, not the env/rank from after solving the defs_constraint. - // That's because the defs constraints will be solved in next_rank if it is eligible for generalization. - // The LetCon will then generalize variables that are at a higher rank than the rank of the current scope. - stack.push(Work::LetConIntroducesVariables { - scope, - rank, - let_con, - pool_variables, - }); - stack.push(Work::Constraint { - scope, - rank: binding_rank, - constraint: defs_constraint, - }); + // Now, run our binding constraint, generalize, then solve the rest of the + // program. + // + // Items are popped from the stack in reverse order. That means that we'll + // first solve the defs_constraint, and then (eventually) the ret_constraint. + // + // NB: LetCon gets the current scope's env and rank, not the env/rank from after solving the defs_constraint. + // That's because the defs constraints will be solved in next_rank if it is eligible for generalization. + // The LetCon will then generalize variables that are at a higher rank than the rank of the current scope. + stack.push(Work::LetConIntroducesVariables { + scope, + rank, + let_con, + pool_variables, + }); + stack.push(Work::Constraint { + scope, + rank: binding_rank, + constraint: defs_constraint, + }); - state - } + state } IsOpenType(type_index) => { let actual = either_type_index_to_var( @@ -1773,6 +1692,20 @@ fn solve_suffix_fx( } } +fn promote_effectful_symbol(env: &mut InferenceEnv<'_>, kind: FxSuffixKind, variable: Variable) { + if kind.suffix() != IdentSuffix::Bang { + return; + } + if !matches!( + env.subs.get_content_without_compacting(variable), + Content::FlexVar(_) + ) { + return; + } + env.subs + .set_content(variable, Content::Structure(FlatType::EffectfulFunc)); +} + fn chase_alias_content(subs: &Subs, mut var: Variable) -> (Variable, &Content) { loop { match subs.get_content_without_compacting(var) { @@ -2147,7 +2080,7 @@ fn check_ability_specialization( } } -#[derive(Debug)] +#[derive(Debug, Clone)] enum LocalDefVarsVec { Stack(arrayvec::ArrayVec), Heap(Vec), From 9df9353f02c61f43630b2bb9efe08a1baa1edea9 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 1 Jan 2025 23:12:35 -0600 Subject: [PATCH 04/10] Store lambda set variables as flex inference variables This is actually correct - the rigid approach is not. Lambda set variables should be inferred in-scope. --- crates/compiler/constrain/src/expr.rs | 15 ++++++--------- .../calls_result_in_unique_specializations.txt | 2 +- ...ursive_annotation_independently_issue_5256.txt | 4 ++-- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/crates/compiler/constrain/src/expr.rs b/crates/compiler/constrain/src/expr.rs index 0e7254d950..f628ba42b1 100644 --- a/crates/compiler/constrain/src/expr.rs +++ b/crates/compiler/constrain/src/expr.rs @@ -3808,9 +3808,9 @@ fn instantiate_rigids( // wildcards are always freshly introduced in this annotation new_rigid_variables.extend(introduced_vars.wildcards.iter().copied()); - // lambda set vars are always freshly introduced in this annotation - new_rigid_variables.extend(introduced_vars.lambda_sets.iter().map(|&v| Loc::at_zero(v))); + let has_explicit_inference_variables = !introduced_vars.inferred.is_empty(); + new_infer_variables.extend(introduced_vars.inferred.iter().copied()); // ext-infer vars are always freshly introduced in this annotation new_infer_variables.extend( introduced_vars @@ -3818,9 +3818,8 @@ fn instantiate_rigids( .iter() .map(|&v| Loc::at_zero(v)), ); - - let has_explicit_inference_variables = !introduced_vars.inferred.is_empty(); - new_infer_variables.extend(introduced_vars.inferred.iter().copied()); + // lambda set vars are always freshly introduced in this annotation + new_infer_variables.extend(introduced_vars.lambda_sets.iter().map(|&v| Loc::at_zero(v))); // Instantiate rigid variables if !rigid_substitution.is_empty() { @@ -3900,12 +3899,8 @@ fn instantiate_rigids_simple( // wildcards are always freshly introduced in this annotation new_rigid_variables.extend(introduced_vars.wildcards.iter().copied()); - // lambda set vars are always freshly introduced in this annotation - new_rigid_variables.extend(introduced_vars.lambda_sets.iter().map(|&v| Loc::at_zero(v))); - let has_explicit_inference_variables = !introduced_vars.inferred.is_empty(); let mut new_infer_variables: Vec> = introduced_vars.inferred.clone(); - // ext-infer vars are always freshly introduced in this annotation new_infer_variables.extend( introduced_vars @@ -3913,6 +3908,8 @@ fn instantiate_rigids_simple( .iter() .map(|&v| Loc::at_zero(v)), ); + // lambda set vars are always freshly introduced in this annotation + new_infer_variables.extend(introduced_vars.lambda_sets.iter().map(|&v| Loc::at_zero(v))); // Instantiate rigid variables if !rigid_substitution.is_empty() { diff --git a/crates/compiler/uitest/tests/recursion/calls_result_in_unique_specializations.txt b/crates/compiler/uitest/tests/recursion/calls_result_in_unique_specializations.txt index 94dc9e3d5d..5f8d20fae3 100644 --- a/crates/compiler/uitest/tests/recursion/calls_result_in_unique_specializations.txt +++ b/crates/compiler/uitest/tests/recursion/calls_result_in_unique_specializations.txt @@ -7,7 +7,7 @@ main = # ^^^ a, Bool -[[foo(1)]]-> Str bar = \_ -> foo {} Bool.true -# ^^^ {}, Bool -[[]]-> Str +# ^^^ {}, Bool -[[foo(1)]]-> Str foo "" Bool.false # ^^^{inst} Str, Bool -[[foo(1)]]-> Str diff --git a/crates/compiler/uitest/tests/recursion/constrain_recursive_annotation_independently_issue_5256.txt b/crates/compiler/uitest/tests/recursion/constrain_recursive_annotation_independently_issue_5256.txt index 537ed97f2a..433d4aef27 100644 --- a/crates/compiler/uitest/tests/recursion/constrain_recursive_annotation_independently_issue_5256.txt +++ b/crates/compiler/uitest/tests/recursion/constrain_recursive_annotation_independently_issue_5256.txt @@ -5,8 +5,8 @@ main = map : { f1: (I64 -> I64) } -> List I64 map = \{ f1 } -> List.concat [f1 1] (map { f1 }) -# ^^^ { f1 : I64 -[[]]-> I64 } -[[map(2)]]-> List I64 -# ^^^ { f1 : I64 -[[]]-> I64 } -[[map(2)]]-> List I64 +# ^^^ { f1 : I64 -[[f(1)]]-> I64 } -[[map(2)]]-> List I64 +# ^^^ { f1 : I64 -[[f(1)]]-> I64 } -[[map(2)]]-> List I64 map { f1: f } From f5961cbb220b16b681050381818eccad611ecb17 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Thu, 2 Jan 2025 00:51:48 -0600 Subject: [PATCH 05/10] Drop debug assert I don't think this assert is actually accurate. --- crates/compiler/solve/src/deep_copy.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/compiler/solve/src/deep_copy.rs b/crates/compiler/solve/src/deep_copy.rs index 0ea8b2e657..c2a75a29b2 100644 --- a/crates/compiler/solve/src/deep_copy.rs +++ b/crates/compiler/solve/src/deep_copy.rs @@ -329,10 +329,6 @@ fn deep_copy_var_help( } let new_ambient_function_var = work!(ambient_function_var); - debug_assert_ne!( - ambient_function_var, new_ambient_function_var, - "lambda set cloned but its ambient function wasn't?" - ); subs.set_content_unchecked( lambda_set_var, From a0461679ddd1e0547dd0b9aec5967ac3bb7ce9f6 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Thu, 2 Jan 2025 14:26:37 -0600 Subject: [PATCH 06/10] Restrict usages of type variables in non-generalized contexts Type variables can only be used on functions (and in number literals as a carve-out for now). In all other cases, a type variable takes on a single, concrete type based on later usages. This check emits errors when this is violated. The implementation is to check the rank of a variable after it could be generalized. If the variable is not generalized but annotated as a type variable, emit an error. --- Cargo.lock | 1 + crates/compiler/builtins/roc/Num.roc | 2 +- crates/compiler/can/src/constraint.rs | 2 +- crates/compiler/debug_flags/src/lib.rs | 16 -- crates/compiler/load/tests/test_reporting.rs | 148 ++++++++++++------ .../compiler/lower_params/src/type_error.rs | 6 +- crates/compiler/solve/src/solve.rs | 65 ++++---- crates/compiler/solve_problem/src/lib.rs | 7 +- crates/compiler/types/Cargo.toml | 1 + crates/compiler/types/src/subs.rs | 34 +++- crates/compiler/types/src/types.rs | 7 +- crates/compiler/unify/src/unify.rs | 4 +- crates/reporting/src/error/type.rs | 51 +++++- 13 files changed, 230 insertions(+), 114 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b432181ebd..5237211da7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3282,6 +3282,7 @@ dependencies = [ name = "roc_types" version = "0.0.1" dependencies = [ + "bitflags 1.3.2", "bumpalo", "roc_collections", "roc_debug_flags", diff --git a/crates/compiler/builtins/roc/Num.roc b/crates/compiler/builtins/roc/Num.roc index 110e4581bb..8804555bec 100644 --- a/crates/compiler/builtins/roc/Num.roc +++ b/crates/compiler/builtins/roc/Num.roc @@ -532,7 +532,7 @@ pi = 3.14159265358979323846264338327950288419716939937510 ## Circle constant (τ) tau : Frac * -tau = 2 * pi +tau = 6.2831853071795864769252867665590057683943387987502 # ------- Functions ## Convert a number to a [Str]. diff --git a/crates/compiler/can/src/constraint.rs b/crates/compiler/can/src/constraint.rs index 569ea9a07b..51127f7ee7 100644 --- a/crates/compiler/can/src/constraint.rs +++ b/crates/compiler/can/src/constraint.rs @@ -915,7 +915,7 @@ pub struct DefTypes { pub loc_symbols: Slice<(Symbol, Region)>, } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq)] pub struct Generalizable(pub bool); #[derive(Debug, Clone)] diff --git a/crates/compiler/debug_flags/src/lib.rs b/crates/compiler/debug_flags/src/lib.rs index 873432d069..eca3d0325b 100644 --- a/crates/compiler/debug_flags/src/lib.rs +++ b/crates/compiler/debug_flags/src/lib.rs @@ -95,22 +95,6 @@ flags! { /// Prints all type variables entered for fixpoint-fixing. ROC_PRINT_FIXPOINT_FIXING - /// Verifies that after let-generalization of a def, any rigid variables in the type annotation - /// of the def are indeed generalized. - /// - /// Note that rigids need not always be generalized in a def. For example, they may be - /// constrained by a type from a lower rank, as `b` is in the following def: - /// - /// F a : { foo : a } - /// foo = \arg -> - /// x : F b - /// x = arg - /// x.foo - /// - /// Instead, this flag is useful for checking that in general, introduction is correct, when - /// chainging how defs are constrained. - ROC_VERIFY_RIGID_LET_GENERALIZED - /// Verifies that an `occurs` check indeed only contains non-recursive types that need to be /// fixed-up with one new recursion variable. /// diff --git a/crates/compiler/load/tests/test_reporting.rs b/crates/compiler/load/tests/test_reporting.rs index 7bb2c20ab9..013ab9aca7 100644 --- a/crates/compiler/load/tests/test_reporting.rs +++ b/crates/compiler/load/tests/test_reporting.rs @@ -1519,18 +1519,18 @@ mod test_reporting { from_annotation_if, indoc!( r" - x : Num.Int * + x : Num.Int _ x = if Bool.true then 3.14 else 4 x " ), - @r" + @r###" ── TYPE MISMATCH in /code/proj/Main.roc ──────────────────────────────────────── Something is off with the `then` branch of this `if` expression: - 4│ x : Num.Int * + 4│ x : Num.Int _ 5│ x = if Bool.true then 3.14 else 4 ^^^^ @@ -1544,14 +1544,14 @@ mod test_reporting { Tip: You can convert between integers and fractions using functions like `Num.toFrac` and `Num.round`. - " + "### ); test_report!( from_annotation_when, indoc!( r" - x : Num.Int * + x : Num.Int _ x = when True is _ -> 3.14 @@ -1559,12 +1559,12 @@ mod test_reporting { x " ), - @r" + @r###" ── TYPE MISMATCH in /code/proj/Main.roc ──────────────────────────────────────── Something is off with the body of the `x` definition: - 4│ x : Num.Int * + 4│ x : Num.Int _ 5│ x = 6│> when True is 7│> _ -> 3.14 @@ -1579,7 +1579,7 @@ mod test_reporting { Tip: You can convert between integers and fractions using functions like `Num.toFrac` and `Num.round`. - " + "### ); test_report!( @@ -1907,7 +1907,7 @@ mod test_reporting { from_annotation_complex_pattern, indoc!( r" - { x } : { x : Num.Int * } + { x } : { x : Num.Int _ } { x } = { x: 4.0 } x @@ -1918,7 +1918,7 @@ mod test_reporting { Something is off with the body of this definition: - 4│ { x } : { x : Num.Int * } + 4│ { x } : { x : Num.Int _ } 5│ { x } = { x: 4.0 } ^^^^^^^^^^ @@ -2044,18 +2044,18 @@ mod test_reporting { missing_fields, indoc!( r" - x : { a : Num.Int *, b : Num.Frac *, c : Str } + x : { a : Num.Int _, b : Num.Frac _, c : Str } x = { b: 4.0 } x " ), - @r" + @r###" ── TYPE MISMATCH in /code/proj/Main.roc ──────────────────────────────────────── Something is off with the body of the `x` definition: - 4│ x : { a : Num.Int *, b : Num.Frac *, c : Str } + 4│ x : { a : Num.Int _, b : Num.Frac _, c : Str } 5│ x = { b: 4.0 } ^^^^^^^^^^ @@ -2072,7 +2072,7 @@ mod test_reporting { } Tip: Looks like the c and a fields are missing. - " + "### ); // this previously reported the message below, not sure which is better @@ -3445,7 +3445,7 @@ mod test_reporting { x : AList Num.I64 Num.I64 x = ACons 0 (BCons 1 (ACons "foo" BNil )) - y : BList a a + y : BList _ _ y = BNil { x, y } @@ -4186,9 +4186,8 @@ mod test_reporting { RBTree k v : [Node NodeColor k v (RBTree k v) (RBTree k v), Empty] # Create an empty dictionary. - empty : RBTree k v - empty = - Empty + empty : {} -> RBTree k v + empty = \{} -> Empty empty " @@ -11129,10 +11128,10 @@ All branches in an `if` must have the same type! import Decode exposing [decoder] - main = - myDecoder : Decoder (a -> a) fmt where fmt implements DecoderFormatting - myDecoder = decoder + myDecoder : Decoder (_ -> _) _ + myDecoder = decoder + main = myDecoder "# ), @@ -11141,12 +11140,12 @@ All branches in an `if` must have the same type! This expression has a type that does not implement the abilities it's expected to: - 7│ myDecoder = decoder - ^^^^^^^ + 6│ myDecoder = decoder + ^^^^^^^ I can't generate an implementation of the `Decoding` ability for - a -> a + * -> * Note: `Decoding` cannot be generated for functions. "### @@ -11162,10 +11161,10 @@ All branches in an `if` must have the same type! A := {} - main = - myDecoder : Decoder {x : A} fmt where fmt implements DecoderFormatting - myDecoder = decoder + myDecoder : Decoder {x : A} _ + myDecoder = decoder + main = myDecoder "# ), @@ -11174,8 +11173,8 @@ All branches in an `if` must have the same type! This expression has a type that does not implement the abilities it's expected to: - 9│ myDecoder = decoder - ^^^^^^^ + 8│ myDecoder = decoder + ^^^^^^^ I can't generate an implementation of the `Decoding` ability for @@ -11425,11 +11424,10 @@ All branches in an `if` must have the same type! import Decode exposing [decoder] - main = - myDecoder : Decoder {x : Str, y ? Str} fmt where fmt implements DecoderFormatting - myDecoder = decoder + myDecoder : Decoder {x : Str, y ? Str} _ + myDecoder = decoder - myDecoder + main = myDecoder "# ), @r###" @@ -11437,8 +11435,8 @@ All branches in an `if` must have the same type! This expression has a type that does not implement the abilities it's expected to: - 7│ myDecoder = decoder - ^^^^^^^ + 6│ myDecoder = decoder + ^^^^^^^ I can't generate an implementation of the `Decoding` ability for @@ -14047,11 +14045,10 @@ All branches in an `if` must have the same type! import Decode exposing [decoder] - main = - myDecoder : Decoder (U32, Str) fmt where fmt implements DecoderFormatting - myDecoder = decoder + myDecoder : Decoder (U32, Str) _ + myDecoder = decoder - myDecoder + main = myDecoder "# ) ); @@ -14064,11 +14061,10 @@ All branches in an `if` must have the same type! import Decode exposing [decoder] - main = - myDecoder : Decoder (U32, {} -> {}) fmt where fmt implements DecoderFormatting - myDecoder = decoder + myDecoder : Decoder (U32, {} -> {}) _ + myDecoder = decoder - myDecoder + main = myDecoder "# ), @r###" @@ -14076,8 +14072,8 @@ All branches in an `if` must have the same type! This expression has a type that does not implement the abilities it's expected to: - 7│ myDecoder = decoder - ^^^^^^^ + 6│ myDecoder = decoder + ^^^^^^^ I can't generate an implementation of the `Decoding` ability for @@ -15933,4 +15929,66 @@ All branches in an `if` must have the same type! Str -> {} "# ); + + test_report!( + invalid_generic_literal, + indoc!( + r#" + module [v] + + v : * + v = 1 + "# + ), + @r###" + ── TYPE MISMATCH in /code/proj/Main.roc ──────────────────────────────────────── + + Something is off with the body of the `v` definition: + + 3│ v : * + 4│ v = 1 + ^ + + The body is a number of type: + + Num * + + But the type annotation on `v` says it should be: + + * + + Tip: The type annotation uses the type variable `*` to say that this + definition can produce any type of value. But in the body I see that + it will only produce a `Num` value of a single specific type. Maybe + change the type annotation to be more specific? Maybe change the code + to be more general? + "### + ); + + test_report!( + invalid_generic_literal_list, + indoc!( + r#" + module [v] + + v : List * + v = [] + "# + ), + @r###" + ── TYPE VARIABLE IS NOT GENERIC in /code/proj/Main.roc ───────────────────────── + + This type variable has a single type: + + 3│ v : List * + ^ + + Type variables tell me that they can be used with any type, but they + can only be used with functions. All other values have exactly one + type. + + Hint: If you would like the type to be inferred for you, use an + underscore _ instead. + "### + ); } diff --git a/crates/compiler/lower_params/src/type_error.rs b/crates/compiler/lower_params/src/type_error.rs index 538f14b34f..a5e5c6a1df 100644 --- a/crates/compiler/lower_params/src/type_error.rs +++ b/crates/compiler/lower_params/src/type_error.rs @@ -105,7 +105,8 @@ pub fn remove_module_param_arguments( | TypeError::ExpectedEffectful(_, _) | TypeError::UnsuffixedEffectfulFunction(_, _) | TypeError::SuffixedPureFunction(_, _) - | TypeError::InvalidTryTarget(_, _, _) => {} + | TypeError::InvalidTryTarget(_, _, _) + | TypeError::TypeIsNotGeneralized(..) => {} } } } @@ -213,6 +214,7 @@ fn drop_last_argument(err_type: &mut ErrorType) { | ErrorType::Alias(_, _, _, _) | ErrorType::Range(_) | ErrorType::Error - | ErrorType::EffectfulFunc => {} + | ErrorType::EffectfulFunc + | ErrorType::InferenceVar => {} } } diff --git a/crates/compiler/solve/src/solve.rs b/crates/compiler/solve/src/solve.rs index 20f3e00d66..aabe4547f6 100644 --- a/crates/compiler/solve/src/solve.rs +++ b/crates/compiler/solve/src/solve.rs @@ -15,15 +15,12 @@ use bumpalo::Bump; use roc_can::abilities::{AbilitiesStore, MemberSpecializationInfo}; use roc_can::constraint::Constraint::{self, *}; use roc_can::constraint::{ - Cycle, FxCallConstraint, FxSuffixConstraint, FxSuffixKind, LetConstraint, OpportunisticResolve, - TryTargetConstraint, + Cycle, FxCallConstraint, FxSuffixConstraint, FxSuffixKind, Generalizable, LetConstraint, + OpportunisticResolve, TryTargetConstraint, }; use roc_can::expected::{Expected, PExpected}; use roc_can::module::ModuleParams; use roc_collections::{VecMap, VecSet}; -use roc_debug_flags::dbg_do; -#[cfg(debug_assertions)] -use roc_debug_flags::ROC_VERIFY_RIGID_LET_GENERALIZED; use roc_error_macros::internal_error; use roc_module::ident::IdentSuffix; use roc_module::symbol::{ModuleId, Symbol}; @@ -32,8 +29,8 @@ use roc_region::all::{Loc, Region}; use roc_solve_problem::TypeError; use roc_solve_schema::UnificationMode; use roc_types::subs::{ - self, Content, FlatType, GetSubsSlice, Mark, OptVariable, Rank, Subs, TagExt, UlsOfVar, - Variable, + self, Content, ErrorTypeContext, FlatType, GetSubsSlice, Mark, OptVariable, Rank, Subs, TagExt, + UlsOfVar, Variable, }; use roc_types::types::{Category, Polarity, Reason, RecordField, Type, TypeExtension, Types, Uls}; use roc_unify::unify::{ @@ -356,29 +353,13 @@ fn solve( generalize(env, young_mark, visit_mark, rank.next()); debug_assert!(env.pools.get(rank.next()).is_empty(), "variables left over in let-binding scope, but they should all be in a lower scope or generalized now"); - // check that things went well - dbg_do!(ROC_VERIFY_RIGID_LET_GENERALIZED, { - let rigid_vars = &env.constraints[let_con.rigid_vars]; - - // NOTE the `subs.redundant` check does not come from elm. - // It's unclear whether this is a bug with our implementation - // (something is redundant that shouldn't be) - // or that it just never came up in elm. - let mut it = rigid_vars - .iter() - .filter(|loc_var| { - let var = loc_var.value; - !env.subs.redundant(var) && env.subs.get_rank(var) != Rank::GENERALIZED - }) - .peekable(); - - if it.peek().is_some() { - let failing: Vec<_> = it.collect(); - println!("Rigids {:?}", &rigid_vars); - println!("Failing {failing:?}"); - debug_assert!(false); - } - }); + let named_variables = &env.constraints[let_con.rigid_vars]; + check_named_variables_are_generalized( + env, + problems, + named_variables, + let_con.generalizable, + ); let mut new_scope = scope.clone(); for (symbol, loc_var) in local_def_vars.iter() { @@ -1636,6 +1617,30 @@ fn solve( state } +fn check_named_variables_are_generalized( + env: &mut InferenceEnv<'_>, + problems: &mut Vec, + named_variables: &[Loc], + generalizable: Generalizable, +) { + for loc_var in named_variables { + let is_generalized = env.subs.get_rank(loc_var.value) == Rank::GENERALIZED; + if !is_generalized { + // TODO: should be OF_PATTERN if on the LHS of a function, otherwise OF_VALUE. + let polarity = Polarity::OF_VALUE; + let ctx = ErrorTypeContext::NON_GENERALIZED_AS_INFERRED; + let error_type = env + .subs + .var_to_error_type_contextual(loc_var.value, ctx, polarity); + problems.push(TypeError::TypeIsNotGeneralized( + loc_var.region, + error_type, + generalizable, + )); + } + } +} + fn solve_suffix_fx( env: &mut InferenceEnv<'_>, problems: &mut Vec, diff --git a/crates/compiler/solve_problem/src/lib.rs b/crates/compiler/solve_problem/src/lib.rs index b02e81282c..0fd0f9585f 100644 --- a/crates/compiler/solve_problem/src/lib.rs +++ b/crates/compiler/solve_problem/src/lib.rs @@ -1,7 +1,7 @@ //! Provides types to describe problems that can occur during solving. use std::{path::PathBuf, str::Utf8Error}; -use roc_can::constraint::{ExpectEffectfulReason, FxSuffixKind}; +use roc_can::constraint::{ExpectEffectfulReason, FxSuffixKind, Generalizable}; use roc_can::expr::TryKind; use roc_can::{ constraint::FxCallKind, @@ -50,6 +50,7 @@ pub enum TypeError { UnsuffixedEffectfulFunction(Region, FxSuffixKind), SuffixedPureFunction(Region, FxSuffixKind), InvalidTryTarget(Region, ErrorType, TryKind), + TypeIsNotGeneralized(Region, ErrorType, Generalizable), } impl TypeError { @@ -80,6 +81,7 @@ impl TypeError { TypeError::UnsuffixedEffectfulFunction(_, _) => Warning, TypeError::SuffixedPureFunction(_, _) => Warning, TypeError::InvalidTryTarget(_, _, _) => RuntimeError, + TypeError::TypeIsNotGeneralized(..) => RuntimeError, } } @@ -101,7 +103,8 @@ impl TypeError { | TypeError::ExpectedEffectful(region, _) | TypeError::UnsuffixedEffectfulFunction(region, _) | TypeError::SuffixedPureFunction(region, _) - | TypeError::InvalidTryTarget(region, _, _) => Some(*region), + | TypeError::InvalidTryTarget(region, _, _) + | TypeError::TypeIsNotGeneralized(region, _, _) => Some(*region), TypeError::UnfulfilledAbility(ab, ..) => ab.region(), TypeError::Exhaustive(e) => Some(e.region()), TypeError::CircularDef(c) => c.first().map(|ce| ce.symbol_region), diff --git a/crates/compiler/types/Cargo.toml b/crates/compiler/types/Cargo.toml index 3a9b4905e1..f8e5e0a816 100644 --- a/crates/compiler/types/Cargo.toml +++ b/crates/compiler/types/Cargo.toml @@ -22,3 +22,4 @@ bumpalo.workspace = true static_assertions.workspace = true soa.workspace = true +bitflags.workspace = true diff --git a/crates/compiler/types/src/subs.rs b/crates/compiler/types/src/subs.rs index f350c2f609..3a84d7fcc0 100644 --- a/crates/compiler/types/src/subs.rs +++ b/crates/compiler/types/src/subs.rs @@ -4,6 +4,7 @@ use crate::types::{ Polarity, RecordField, RecordFieldsError, TupleElemsError, TypeExt, Uls, }; use crate::unification_table::{self, UnificationTable}; +use bitflags::bitflags; use roc_collections::all::{FnvMap, ImMap, ImSet, MutSet, SendMap}; use roc_collections::{VecMap, VecSet}; use roc_error_macros::internal_error; @@ -50,10 +51,24 @@ impl fmt::Debug for Mark { } } -#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] -pub enum ErrorTypeContext { - None, - ExpandRanges, +bitflags! { + pub struct ErrorTypeContext : u8 { + const NONE = 1 << 0; + /// List all number types that satisfy number range constraints. + const EXPAND_RANGES = 1 << 1; + /// Re-write non-generalized types like to inference variables. + const NON_GENERALIZED_AS_INFERRED = 1 << 2; + } +} + +impl ErrorTypeContext { + fn expand_ranges(&self) -> bool { + self.contains(Self::EXPAND_RANGES) + } + + fn non_generalized_as_inferred(&self) -> bool { + self.contains(Self::NON_GENERALIZED_AS_INFERRED) + } } struct ErrorTypeState { @@ -2055,7 +2070,7 @@ impl Subs { } pub fn var_to_error_type(&mut self, var: Variable, observed_pol: Polarity) -> ErrorType { - self.var_to_error_type_contextual(var, ErrorTypeContext::None, observed_pol) + self.var_to_error_type_contextual(var, ErrorTypeContext::empty(), observed_pol) } pub fn var_to_error_type_contextual( @@ -4020,6 +4035,13 @@ fn content_to_err_type( match content { Structure(flat_type) => flat_type_to_err_type(subs, state, flat_type, pol), + RigidVar(..) | RigidAbleVar(..) + if state.context.non_generalized_as_inferred() + && subs.get_rank(var) != Rank::GENERALIZED => + { + ErrorType::InferenceVar + } + FlexVar(opt_name) => { let name = match opt_name { Some(name_index) => subs.field_names[name_index.index()].clone(), @@ -4123,7 +4145,7 @@ fn content_to_err_type( } RangedNumber(range) => { - if state.context == ErrorTypeContext::ExpandRanges { + if state.context.expand_ranges() { let mut types = Vec::new(); for var in range.variable_slice() { types.push(var_to_err_type(subs, state, *var, pol)); diff --git a/crates/compiler/types/src/types.rs b/crates/compiler/types/src/types.rs index ce41314962..6f63a80019 100644 --- a/crates/compiler/types/src/types.rs +++ b/crates/compiler/types/src/types.rs @@ -3679,6 +3679,7 @@ pub enum ErrorType { /// If the name was auto-generated, it will start with a `#`. FlexVar(Lowercase), RigidVar(Lowercase), + InferenceVar, EffectfulFunc, /// If the name was auto-generated, it will start with a `#`. FlexAbleVar(Lowercase, AbilitySet), @@ -3733,6 +3734,7 @@ impl ErrorType { FlexVar(v) | RigidVar(v) | FlexAbleVar(v, _) | RigidAbleVar(v, _) => { taken.insert(v.clone()); } + InferenceVar => {} Record(fields, ext) => { fields .iter() @@ -3912,13 +3914,14 @@ fn write_debug_error_type_help(error_type: ErrorType, buf: &mut String, parens: Infinite => buf.push('∞'), Error => buf.push('?'), FlexVar(name) | RigidVar(name) => buf.push_str(name.as_str()), - FlexAbleVar(name, symbol) | RigidAbleVar(name, symbol) => { + InferenceVar => buf.push('_'), + FlexAbleVar(name, abilities) | RigidAbleVar(name, abilities) => { let write_parens = parens == Parens::InTypeParam; if write_parens { buf.push('('); } buf.push_str(name.as_str()); - write!(buf, "{} {:?}", roc_parse::keyword::IMPLEMENTS, symbol).unwrap(); + write!(buf, "{} {:?}", roc_parse::keyword::IMPLEMENTS, abilities).unwrap(); if write_parens { buf.push(')'); } diff --git a/crates/compiler/unify/src/unify.rs b/crates/compiler/unify/src/unify.rs index d5829a4170..81bc5edfa9 100644 --- a/crates/compiler/unify/src/unify.rs +++ b/crates/compiler/unify/src/unify.rs @@ -356,9 +356,9 @@ fn unify_help( } } else { let error_context = if mismatches.contains(&Mismatch::TypeNotInRange) { - ErrorTypeContext::ExpandRanges + ErrorTypeContext::EXPAND_RANGES } else { - ErrorTypeContext::None + ErrorTypeContext::empty() }; let type1 = env.var_to_error_type_contextual(var1, error_context, observed_pol); diff --git a/crates/reporting/src/error/type.rs b/crates/reporting/src/error/type.rs index 05f9aa0553..8d4513729a 100644 --- a/crates/reporting/src/error/type.rs +++ b/crates/reporting/src/error/type.rs @@ -513,6 +513,38 @@ pub fn type_problem<'b>( severity, }) } + TypeIsNotGeneralized(region, actual_type, generalizable) => { + let doc = alloc.stack([ + alloc.reflow("This type variable has a single type:"), + alloc.region(lines.convert_region(region), severity), + if !generalizable.0 { + alloc.concat([ + alloc.reflow("Type variables tell me that they can be used with any type, but they can only be used with functions. All other values have exactly one type."), + ]) + } else { + alloc.concat([ + alloc.reflow("Type variables tell me that they can be used as any type."), + alloc.reflow("But, I found that this type can only be used as this single type:"), + alloc.type_block(to_doc(alloc, Parens::Unnecessary, actual_type).0) + ]) + }, + alloc.concat([ + alloc.hint(""), + alloc.reflow( + "If you would like the type to be inferred for you, use an underscore ", + ), + alloc.type_str("_"), + alloc.reflow(" instead."), + ]), + ]); + + Some(Report { + title: "TYPE VARIABLE IS NOT GENERIC".to_string(), + filename, + doc, + severity, + }) + } } } @@ -656,9 +688,9 @@ fn underivable_hint<'b>( }, ]))), NotDerivableContext::UnboundVar => { - let v = match typ { - ErrorType::FlexVar(v) => v, - ErrorType::RigidVar(v) => v, + let formatted_var = match typ { + ErrorType::FlexVar(v) | ErrorType::RigidVar(v) => alloc.type_variable(v.clone()), + ErrorType::InferenceVar => alloc.type_str("_"), _ => internal_error!("unbound variable context only applicable for variables"), }; @@ -671,7 +703,7 @@ fn underivable_hint<'b>( alloc.inline_type_block(alloc.concat([ alloc.keyword(roc_parse::keyword::WHERE), alloc.space(), - alloc.type_variable(v.clone()), + formatted_var, alloc.space(), alloc.keyword(roc_parse::keyword::IMPLEMENTS), alloc.space(), @@ -2868,6 +2900,7 @@ fn to_doc_help<'b>( ), Infinite => alloc.text("∞"), Error => alloc.text("?"), + InferenceVar => alloc.text("_"), FlexVar(lowercase) if is_generated_name(&lowercase) => { let &usages = gen_usages @@ -3082,6 +3115,7 @@ fn count_generated_name_usages<'a>( RigidVar(name) | RigidAbleVar(name, _) => { debug_assert!(!is_generated_name(name)); } + InferenceVar => {} EffectfulFunc => {} Type(_, tys) => { stack.extend(tys.iter().map(|t| (t, only_unseen))); @@ -3752,6 +3786,7 @@ fn should_show_diff(t1: &ErrorType, t2: &ErrorType) -> bool { // If either is flex, it will unify to the other type; no diff is needed. false } + (InferenceVar, InferenceVar) => false, (FlexAbleVar(v1, _set1), FlexAbleVar(v2, _set2)) | (RigidAbleVar(v1, _set1), RigidAbleVar(v2, _set2)) => { #[cfg(debug_assertions)] @@ -3944,7 +3979,9 @@ fn should_show_diff(t1: &ErrorType, t2: &ErrorType) -> bool { | (Function(_, _, _, _), _) | (_, Function(_, _, _, _)) | (EffectfulFunc, _) - | (_, EffectfulFunc) => true, + | (_, EffectfulFunc) + | (InferenceVar, _) + | (_, InferenceVar) => true, } } @@ -4985,7 +5022,7 @@ fn type_problem_to_pretty<'b>( }; match tipe { - Infinite | Error | FlexVar(_) | EffectfulFunc => alloc.nil(), + Infinite | Error | FlexVar(_) | InferenceVar | EffectfulFunc => alloc.nil(), FlexAbleVar(_, other_abilities) => { rigid_able_vs_different_flex_able(x, abilities, other_abilities) } @@ -5058,7 +5095,7 @@ fn type_problem_to_pretty<'b>( }; match tipe { - Infinite | Error | FlexVar(_) => alloc.nil(), + Infinite | Error | FlexVar(_) | InferenceVar => alloc.nil(), FlexAbleVar(_, abilities) => { let mut abilities = abilities.into_sorted_iter(); let msg = if abilities.len() == 1 { From 89700b491c19d0a7419dfb9af9452a7ebcf9e7be Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 10 Jan 2025 20:40:22 -0500 Subject: [PATCH 07/10] Fix some tests --- .cargo/config.toml | 1 - crates/cli/tests/benchmarks/AStar.roc | 4 +-- crates/compiler/solve/tests/solve_expr.rs | 34 +++++-------------- .../compiler/test_gen/src/gen_primitives.rs | 32 ++++++++--------- crates/compiler/test_gen/src/gen_result.rs | 2 +- crates/compiler/test_gen/src/gen_tags.rs | 2 +- 6 files changed, 28 insertions(+), 47 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index f57d96eaf7..0844763970 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -39,7 +39,6 @@ ROC_TRACE_COMPACTION = "0" ROC_PRINT_UNIFICATIONS_DERIVED = "0" ROC_PRINT_MISMATCHES = "0" ROC_PRINT_FIXPOINT_FIXING = "0" -ROC_VERIFY_RIGID_LET_GENERALIZED = "0" ROC_VERIFY_OCCURS_ONE_RECURSION = "0" ROC_CHECK_MONO_IR = "0" ROC_PRINT_IR_AFTER_SPECIALIZATION = "0" diff --git a/crates/cli/tests/benchmarks/AStar.roc b/crates/cli/tests/benchmarks/AStar.roc index 20856b8d7f..6808d908c0 100644 --- a/crates/cli/tests/benchmarks/AStar.roc +++ b/crates/cli/tests/benchmarks/AStar.roc @@ -90,12 +90,12 @@ astar = \cost_fn, move_fn, goal, model -> new_neighbors = Set.difference(neighbors, model_popped.evaluated) - model_with_neighbors : Model position + model_with_neighbors : Model _ model_with_neighbors = model_popped |> &open_set(Set.union(model_popped.open_set, new_neighbors)) - walker : Model position, position -> Model position + walker : Model _, _ -> Model _ walker = \amodel, n -> update_cost(current, n, amodel) model_with_costs = diff --git a/crates/compiler/solve/tests/solve_expr.rs b/crates/compiler/solve/tests/solve_expr.rs index 8c6e02c6e5..3655d4b11f 100644 --- a/crates/compiler/solve/tests/solve_expr.rs +++ b/crates/compiler/solve/tests/solve_expr.rs @@ -2510,13 +2510,15 @@ mod solve_expr { infer_eq_without_problem( indoc!( r" - empty : [Cons a (ConsList a), Nil] as ConsList a - empty = Nil + ConsList a : [Cons a (ConsList a), Nil] - empty - " + empty : ConsList _ + empty = Nil + + empty + " ), - "ConsList a", + "ConsList *", ); } @@ -3742,7 +3744,7 @@ mod solve_expr { indoc!( r" \rec -> - { x, y } : { x : I64, y ? Bool }* + { x, y } : { x : I64, y ? Bool }_ { x, y ? Bool.false } = rec { x, y } @@ -3909,26 +3911,6 @@ mod solve_expr { ); } - #[test] - fn double_named_rigids() { - infer_eq_without_problem( - indoc!( - r#" - app "test" provides [main] to "./platform" - - - main : List x - main = - empty : List x - empty = [] - - empty - "# - ), - "List x", - ); - } - #[test] fn double_tag_application() { infer_eq_without_problem( diff --git a/crates/compiler/test_gen/src/gen_primitives.rs b/crates/compiler/test_gen/src/gen_primitives.rs index 24e21ca97c..f5ce8cbb45 100644 --- a/crates/compiler/test_gen/src/gen_primitives.rs +++ b/crates/compiler/test_gen/src/gen_primitives.rs @@ -662,7 +662,7 @@ fn linked_list_len_1() { LinkedList a : [Nil, Cons a (LinkedList a)] - one : LinkedList (Int *) + one : LinkedList (Int _) one = Cons 1 Nil length : LinkedList a -> Int * @@ -690,7 +690,7 @@ fn linked_list_len_twice_1() { LinkedList a : [Nil, Cons a (LinkedList a)] - one : LinkedList (Int *) + one : LinkedList (Int _) one = Cons 1 Nil length : LinkedList a -> Int * @@ -718,7 +718,7 @@ fn linked_list_len_3() { LinkedList a : [Nil, Cons a (LinkedList a)] - three : LinkedList (Int *) + three : LinkedList (Int _) three = Cons 3 (Cons 2 (Cons 1 Nil)) length : LinkedList a -> Int * @@ -747,7 +747,7 @@ fn linked_list_sum_num_a() { LinkedList a : [Nil, Cons a (LinkedList a)] - three : LinkedList (Int *) + three : LinkedList (Int _) three = Cons 3 (Cons 2 (Cons 1 Nil)) @@ -776,7 +776,7 @@ fn linked_list_sum_int() { LinkedList a : [Nil, Cons a (LinkedList a)] - zero : LinkedList (Int *) + zero : LinkedList (Int _) zero = Nil sum : LinkedList (Int a) -> Int a @@ -804,7 +804,7 @@ fn linked_list_map() { LinkedList a : [Nil, Cons a (LinkedList a)] - three : LinkedList (Int *) + three : LinkedList (Int _) three = Cons 3 (Cons 2 (Cons 1 Nil)) sum : LinkedList (Num a) -> Num a @@ -836,7 +836,7 @@ fn when_nested_maybe() { r" Maybe a : [Nothing, Just a] - x : Maybe (Maybe (Int a)) + x : Maybe (Maybe (Int _)) x = Just (Just 41) when x is @@ -853,7 +853,7 @@ fn when_nested_maybe() { r" Maybe a : [Nothing, Just a] - x : Maybe (Maybe (Int *)) + x : Maybe (Maybe (Int _)) x = Just Nothing when x is @@ -871,7 +871,7 @@ fn when_nested_maybe() { r" Maybe a : [Nothing, Just a] - x : Maybe (Maybe (Int *)) + x : Maybe (Maybe (Int _)) x = Nothing when x is @@ -1402,7 +1402,7 @@ fn recursive_function_with_rigid() { else 1 + foo { count: state.count - 1, x: state.x } - main : Int * + main : Int _ main = foo { count: 3, x: {} } "# @@ -1517,7 +1517,7 @@ fn rbtree_balance_3() { balance = \key, left -> Node key left Empty - main : RedBlackTree (Int *) + main : RedBlackTree (Int _) main = balance 0 Empty "# @@ -1696,7 +1696,7 @@ fn nested_pattern_match_two_ways() { _ -> 3 _ -> 3 - main : Int * + main : Int _ main = when balance Nil is _ -> 3 @@ -1719,7 +1719,7 @@ fn nested_pattern_match_two_ways() { Cons 1 (Cons 1 _) -> 3 _ -> 3 - main : Int * + main : Int _ main = when balance Nil is _ -> 3 @@ -1751,7 +1751,7 @@ fn linked_list_guarded_double_pattern_match() { _ -> 3 _ -> 3 - main : Int * + main : Int _ main = when balance Nil is _ -> 3 @@ -1778,7 +1778,7 @@ fn linked_list_double_pattern_match() { Cons _ (Cons x _) -> x _ -> 0 - main : Int * + main : Int _ main = foo (Cons 1 (Cons 32 Nil)) "# @@ -1886,7 +1886,7 @@ fn wildcard_rigid() { @Effect inner - main : MyTask {} (Frac *) + main : MyTask {} (Frac _) main = always {} "# ), diff --git a/crates/compiler/test_gen/src/gen_result.rs b/crates/compiler/test_gen/src/gen_result.rs index e46045dff6..2ce87f14eb 100644 --- a/crates/compiler/test_gen/src/gen_result.rs +++ b/crates/compiler/test_gen/src/gen_result.rs @@ -135,7 +135,7 @@ fn err_type_var_annotation() { assert_evals_to!( indoc!( r" - ok : Result I64 * + ok : Result I64 _ ok = Ok 3 Result.map ok (\x -> x + 1) diff --git a/crates/compiler/test_gen/src/gen_tags.rs b/crates/compiler/test_gen/src/gen_tags.rs index 71611fb1df..5562dcada4 100644 --- a/crates/compiler/test_gen/src/gen_tags.rs +++ b/crates/compiler/test_gen/src/gen_tags.rs @@ -1075,7 +1075,7 @@ fn applied_tag_function_result() { assert_evals_to!( indoc!( r#" - x : List (Result Str *) + x : List (Result Str _) x = List.map ["a", "b"] Ok List.keep_oks x (\y -> y) From ec23cc56ea21898c9bb14508dd1e568322932188 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 10 Jan 2025 21:31:43 -0500 Subject: [PATCH 08/10] Fix more --- crates/compiler/load/tests/test_reporting.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/compiler/load/tests/test_reporting.rs b/crates/compiler/load/tests/test_reporting.rs index f38d72898f..0ff56a2f13 100644 --- a/crates/compiler/load/tests/test_reporting.rs +++ b/crates/compiler/load/tests/test_reporting.rs @@ -1547,7 +1547,7 @@ mod test_reporting { Tip: You can convert between integers and fractions using functions like `Num.to_frac` and `Num.round`. - " + "### ); test_report!( @@ -1582,7 +1582,7 @@ mod test_reporting { Tip: You can convert between integers and fractions using functions like `Num.to_frac` and `Num.round`. - " + "### ); test_report!( From 3a6c622727eb0755e24ed51524a569fc68095cc2 Mon Sep 17 00:00:00 2001 From: Sam Mohr Date: Sun, 19 Jan 2025 15:33:01 -0800 Subject: [PATCH 09/10] Fix failing tests, remove unnecessary NONE bitflag --- .../fixtures/build/app_with_deps/Primary.roc | 2 +- .../build/module_with_deps/Primary.roc | 2 +- .../compiler/load_internal/tests/test_load.rs | 24 +++++++++++-------- crates/compiler/types/src/subs.rs | 5 ++-- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/crates/compiler/load_internal/tests/fixtures/build/app_with_deps/Primary.roc b/crates/compiler/load_internal/tests/fixtures/build/app_with_deps/Primary.roc index 6512dab3f3..c8a85af353 100644 --- a/crates/compiler/load_internal/tests/fixtures/build/app_with_deps/Primary.roc +++ b/crates/compiler/load_internal/tests/fixtures/build/app_with_deps/Primary.roc @@ -24,7 +24,7 @@ succeed = \x -> Identity(x) with_default = Res.with_default -yay : Res.Res {} err +yay : Res.Res {} _ yay = ok = Ok("foo") diff --git a/crates/compiler/load_internal/tests/fixtures/build/module_with_deps/Primary.roc b/crates/compiler/load_internal/tests/fixtures/build/module_with_deps/Primary.roc index f47f972474..509c07cea3 100644 --- a/crates/compiler/load_internal/tests/fixtures/build/module_with_deps/Primary.roc +++ b/crates/compiler/load_internal/tests/fixtures/build/module_with_deps/Primary.roc @@ -24,7 +24,7 @@ succeed = \x -> Identity(x) with_default = Res.with_default -yay : Res.Res {} err +yay : Res.Res {} _ yay = ok = Ok("foo") diff --git a/crates/compiler/load_internal/tests/test_load.rs b/crates/compiler/load_internal/tests/test_load.rs index deef83c187..14384e1c7a 100644 --- a/crates/compiler/load_internal/tests/test_load.rs +++ b/crates/compiler/load_internal/tests/test_load.rs @@ -271,11 +271,13 @@ fn load_fixture( ); } - assert!(loaded_module - .type_problems - .remove(&home) - .unwrap_or_default() - .is_empty()); + assert_eq!( + loaded_module + .type_problems + .remove(&home) + .unwrap_or_default(), + Vec::new() + ); let expected_name = loaded_module .interns @@ -433,11 +435,13 @@ fn module_with_deps() { loaded_module.can_problems.remove(&home).unwrap_or_default(), Vec::new() ); - assert!(loaded_module - .type_problems - .remove(&home) - .unwrap_or_default() - .is_empty(),); + assert_eq!( + loaded_module + .type_problems + .remove(&home) + .unwrap_or_default(), + Vec::new() + ); let mut def_count = 0; let declarations = loaded_module.declarations_by_id.remove(&home).unwrap(); diff --git a/crates/compiler/types/src/subs.rs b/crates/compiler/types/src/subs.rs index 3a84d7fcc0..e93428a295 100644 --- a/crates/compiler/types/src/subs.rs +++ b/crates/compiler/types/src/subs.rs @@ -53,11 +53,10 @@ impl fmt::Debug for Mark { bitflags! { pub struct ErrorTypeContext : u8 { - const NONE = 1 << 0; /// List all number types that satisfy number range constraints. - const EXPAND_RANGES = 1 << 1; + const EXPAND_RANGES = 1 << 0; /// Re-write non-generalized types like to inference variables. - const NON_GENERALIZED_AS_INFERRED = 1 << 2; + const NON_GENERALIZED_AS_INFERRED = 1 << 1; } } From 4fa5fd62224c0305307897346942ca1e6efdd4ad Mon Sep 17 00:00:00 2001 From: Sam Mohr Date: Sun, 19 Jan 2025 15:39:39 -0800 Subject: [PATCH 10/10] Remove vestigial uitest --- .../uitest/tests/solve/export_rigid_to_lower_rank.txt | 9 --------- 1 file changed, 9 deletions(-) delete mode 100644 crates/compiler/uitest/tests/solve/export_rigid_to_lower_rank.txt diff --git a/crates/compiler/uitest/tests/solve/export_rigid_to_lower_rank.txt b/crates/compiler/uitest/tests/solve/export_rigid_to_lower_rank.txt deleted file mode 100644 index d444bce244..0000000000 --- a/crates/compiler/uitest/tests/solve/export_rigid_to_lower_rank.txt +++ /dev/null @@ -1,9 +0,0 @@ -app "test" provides [foo] to "./platform" - -F a : { foo : a } - -foo = \arg -> -#^^^{-1} F b -[[foo(0)]]-> b - x : F b - x = arg - x.foo