Merge remote-tracking branch 'origin/main' into expect-print-values

crates/compiler/can/src/constraint.rs

@@ -488,17 +488,19 @@ impl Constraints {
     /// then need to find their way to the pool, and a convenient approach turned out to be to
     /// tag them onto the `Let` that we used to add the imported values.
     #[inline(always)]
-    pub fn let_import_constraint<I1, I2>(
+    pub fn let_import_constraint<I1, I2, I3>(
         &mut self,
         rigid_vars: I1,
-        def_types: I2,
+        flex_vars: I2,
+        def_types: I3,
         module_constraint: Constraint,
         pool_variables: &[Variable],
     ) -> Constraint
     where
         I1: IntoIterator<Item = Variable>,
-        I2: IntoIterator<Item = (Symbol, Loc<TypeOrVar>)>,
-        I2::IntoIter: ExactSizeIterator,
+        I2: IntoIterator<Item = Variable>,
+        I3: IntoIterator<Item = (Symbol, Loc<TypeOrVar>)>,
+        I3::IntoIter: ExactSizeIterator,
     {
         // defs and ret constraint are stored consequtively, so we only need to store one index
         let defs_and_ret_constraint = Index::new(self.constraints.len() as _);
@@ -508,7 +510,7 @@ impl Constraints {
 
         let let_contraint = LetConstraint {
             rigid_vars: self.variable_slice(rigid_vars),
-            flex_vars: Slice::default(),
+            flex_vars: self.variable_slice(flex_vars),
             def_types: self.def_types_slice(def_types),
             defs_and_ret_constraint,
         };

crates/compiler/can/src/def.rs

@@ -302,7 +302,7 @@ fn sort_type_defs_before_introduction(
     matrix
         .strongly_connected_components_all()
         .groups()
-        .flat_map(|group| group.iter_ones())
+        .flat_map(|(group, _)| group.iter_ones())
         .map(|index| symbols[index])
         .collect()
 }
@@ -1547,10 +1547,12 @@ impl DefOrdering {
 
 #[inline(always)]
 pub(crate) fn sort_can_defs_new(
+    env: &mut Env<'_>,
     scope: &mut Scope,
     var_store: &mut VarStore,
     defs: CanDefs,
     mut output: Output,
+    exposed_symbols: &VecSet<Symbol>,
 ) -> (Declarations, Output) {
     let CanDefs {
         defs,
@@ -1611,7 +1613,7 @@ pub(crate) fn sort_can_defs_new(
 
     sccs.reorder(&mut defs);
 
-    for group in sccs.groups().rev() {
+    for (group, is_initial) in sccs.groups().rev() {
         match group.count_ones() {
             1 => {
                 // a group with a single Def, nice and simple
@@ -1636,6 +1638,10 @@ pub(crate) fn sort_can_defs_new(
                         }
                     };
 
+                    if is_initial && !exposed_symbols.contains(&symbol) {
+                        env.problem(Problem::DefsOnlyUsedInRecursion(1, def.region()));
+                    }
+
                     match def.loc_expr.value {
                         Closure(closure_data) => {
                             declarations.push_recursive_def(
@@ -1720,6 +1726,9 @@ pub(crate) fn sort_can_defs_new(
                 let cycle_mark = IllegalCycleMark::new(var_store);
                 declarations.push_recursive_group(group_length as u16, cycle_mark);
 
+                let mut group_is_initial = is_initial;
+                let mut whole_region = None;
+
                 // then push the definitions of this group
                 for def in group_defs {
                     let (symbol, specializes) = match def.loc_pattern.value {
@@ -1734,6 +1743,12 @@ pub(crate) fn sort_can_defs_new(
                         }
                     };
 
+                    group_is_initial = group_is_initial && !exposed_symbols.contains(&symbol);
+                    whole_region = match whole_region {
+                        None => Some(def.region()),
+                        Some(r) => Some(Region::span_across(&r, &def.region())),
+                    };
+
                     match def.loc_expr.value {
                         Closure(closure_data) => {
                             declarations.push_recursive_def(
@@ -1755,6 +1770,13 @@ pub(crate) fn sort_can_defs_new(
                         }
                     }
                 }
+
+                if group_is_initial {
+                    env.problem(Problem::DefsOnlyUsedInRecursion(
+                        group_length,
+                        whole_region.unwrap(),
+                    ));
+                }
             }
         }
     }
@@ -1803,7 +1825,7 @@ pub(crate) fn sort_can_defs(
 
     let mut declarations = Vec::with_capacity(defs.len());
 
-    for group in sccs.groups() {
+    for (group, is_initial) in sccs.groups() {
         if group.count_ones() == 1 {
             // a group with a single Def, nice and simple
             let index = group.iter_ones().next().unwrap();
@@ -1817,6 +1839,16 @@ pub(crate) fn sort_can_defs(
             let declaration = if def_ordering.references.get_row_col(index, index) {
                 debug_assert!(!is_specialization, "Self-recursive specializations can only be determined during solving - but it was determined for {:?} now, that's a bug!", def);
 
+                if is_initial
+                    && !def
+                        .pattern_vars
+                        .keys()
+                        .any(|sym| output.references.has_value_lookup(*sym))
+                {
+                    // This defs is only used in recursion with itself.
+                    env.problem(Problem::DefsOnlyUsedInRecursion(1, def.region()));
+                }
+
                 // this function calls itself, and must be typechecked as a recursive def
                 Declaration::DeclareRec(vec![mark_def_recursive(def)], IllegalCycleMark::empty())
             } else {
@@ -1862,11 +1894,26 @@ pub(crate) fn sort_can_defs(
 
                 Declaration::InvalidCycle(entries)
             } else {
-                let rec_defs = group
+                let rec_defs: Vec<Def> = group
                     .iter_ones()
                     .map(|index| mark_def_recursive(take_def!(index)))
                     .collect();
 
+                if is_initial
+                    && !rec_defs.iter().any(|def| {
+                        def.pattern_vars
+                            .keys()
+                            .any(|sym| output.references.has_value_lookup(*sym))
+                    })
+                {
+                    // These defs are only used in mutual recursion with themselves.
+                    let region = Region::span_across(
+                        &rec_defs.first().unwrap().region(),
+                        &rec_defs.last().unwrap().region(),
+                    );
+                    env.problem(Problem::DefsOnlyUsedInRecursion(rec_defs.len(), region));
+                }
+
                 Declaration::DeclareRec(rec_defs, IllegalCycleMark::new(var_store))
             };
 
@@ -2312,10 +2359,15 @@ pub fn can_defs_with_return<'a>(
     output
         .introduced_variables
         .union(&defs_output.introduced_variables);
+
+    // Sort the defs with the output of the return expression - we'll use this to catch unused defs
+    // due only to recursion.
+    let (declarations, mut output) = sort_can_defs(env, var_store, unsorted, output);
+
     output.references.union_mut(&defs_output.references);
 
     // Now that we've collected all the references, check to see if any of the new idents
-    // we defined went unused by the return expression. If any were unused, report it.
+    // we defined went unused by the return expression or any other def.
     for (symbol, region) in symbols_introduced {
         if !output.references.has_type_or_value_lookup(symbol)
             && !scope.abilities_store.is_specialization_name(symbol)
@@ -2324,8 +2376,6 @@ pub fn can_defs_with_return<'a>(
         }
     }
 
-    let (declarations, output) = sort_can_defs(env, var_store, unsorted, output);
-
     let mut loc_expr: Loc<Expr> = ret_expr;
 
     for declaration in declarations.into_iter().rev() {
@@ -2738,12 +2788,12 @@ fn correct_mutual_recursive_type_alias<'a>(
     // this is needed.
     let scratchpad_capacity = sccs
         .groups()
-        .map(|r| r.count_ones())
+        .map(|(r, _)| r.count_ones())
         .max()
         .unwrap_or_default();
     let mut scratchpad = Vec::with_capacity(scratchpad_capacity);
 
-    for cycle in sccs.groups() {
+    for (cycle, _is_initial) in sccs.groups() {
         debug_assert!(cycle.count_ones() > 0);
 
         // We need to instantiate the alias with any symbols in the currrent module it

crates/compiler/can/src/expr.rs

@@ -3003,7 +3003,7 @@ fn toplevel_expect_to_inline_expect_help(mut loc_expr: Loc<Expr>, has_effects: b
 
     let mut loc_expr = Loc::at(expect_region, expect);
 
-    for stored in stack {
+    for stored in stack.into_iter().rev() {
         match stored {
             StoredDef::NonRecursive(region, boxed_def) => {
                 loc_expr = Loc::at(region, Expr::LetNonRec(boxed_def, Box::new(loc_expr)));

crates/compiler/can/src/module.rs

@@ -376,6 +376,9 @@ pub fn canonicalize_module_defs<'a>(
 
     // See if any of the new idents we defined went unused.
     // If any were unused and also not exposed, report it.
+    //
+    // We'll catch symbols that are only referenced due to (mutual) recursion later,
+    // when sorting the defs.
     for (symbol, region) in symbols_introduced {
         if !output.references.has_type_or_value_lookup(symbol)
             && !exposed_symbols.contains(&symbol)
@@ -427,8 +430,14 @@ pub fn canonicalize_module_defs<'a>(
         ..Default::default()
     };
 
-    let (mut declarations, mut output) =
-        crate::def::sort_can_defs_new(&mut scope, var_store, defs, new_output);
+    let (mut declarations, mut output) = crate::def::sort_can_defs_new(
+        &mut env,
+        &mut scope,
+        var_store,
+        defs,
+        new_output,
+        exposed_symbols,
+    );
 
     debug_assert!(
         output.pending_derives.is_empty(),