Provide warning for defs that are used only in (mutual) recursion

This patch provides errors for defs that are used only in
possibly-mutual recursion, and are not reachable outside of their
recursive closures. For example:

```
test_report!(
    mutual_recursion_not_reached_nested,
    indoc!(
        r#"
        app "test" provides [main] to "./platform"

        main =
            f = \{} -> if Bool.true then "" else g {}
            g = \{} -> if Bool.true then "" else f {}
            ""
        "#
    ),
@r###"
── DEFINITIONs ONLY USED IN RECURSION ──────────────────── /code/proj/Main.roc ─

These 2 definitions are only used in mutual recursion with themselves:

4│>      f = \{} -> if Bool.true then "" else g {}
5│>      g = \{} -> if Bool.true then "" else f {}

If you don't intend to use or export any of them, they should all be
removed!
"###
);
```

crates/compiler/can/src/def.rs

@@ -301,7 +301,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()
 }
@@ -1546,10 +1546,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,
@@ -1610,7 +1612,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
@@ -1635,6 +1637,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(
@@ -1719,6 +1725,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 {
@@ -1733,6 +1742,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(
@@ -1754,6 +1769,13 @@ pub(crate) fn sort_can_defs_new(
                         }
                     }
                 }
+
+                if group_is_initial {
+                    env.problem(Problem::DefsOnlyUsedInRecursion(
+                        group_length,
+                        whole_region.unwrap(),
+                    ));
+                }
             }
         }
     }
@@ -1802,7 +1824,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();
@@ -1816,6 +1838,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 {
@@ -1861,11 +1893,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))
             };
 
@@ -2311,10 +2358,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)
@@ -2323,8 +2375,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() {
@@ -2735,12 +2785,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

@@ -1411,6 +1411,13 @@ fn canonicalize_closure_body<'a>(
         }
     }
 
+    // // Discount the def name from the referenced arguments as well. This catches recursive
+    // // functions that only reference themselves.
+    // // Note that this will not catch mutually recursive functions whose reference closure is the
+    // // recursive closure. Doing so requires us to know what defs are in a cycle, which is only
+    // // calculated after enumeration of references.
+    // output.references.remove_value_lookup(&symbol);
+
     // store the references of this function in the Env. This information is used
     // when we canonicalize a surrounding def (if it exists)
     env.closures.insert(symbol, output.references.clone());

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) =
+    let (mut declarations, mut output) = crate::def::sort_can_defs_new(
-        crate::def::sort_can_defs_new(&mut scope, var_store, defs, new_output);
+        &mut env,
+        &mut scope,
+        var_store,
+        defs,
+        new_output,
+        exposed_symbols,
+    );
 
     debug_assert!(
         output.pending_derives.is_empty(),

crates/compiler/collections/src/reference_matrix.rs

@@ -200,6 +200,7 @@ impl Params {
             scc: Sccs {
                 matrix: ReferenceMatrix::new(length),
                 components: 0,
+                not_initial: BitVec::repeat(false, length),
             },
             scca: BitVec::repeat(false, length),
         }
@@ -253,6 +254,10 @@ fn recurse_onto(length: usize, bitvec: &BitVec, v: usize, params: &mut Params) {
             }
         }
 
+        if !params.s.is_empty() {
+            params.scc.not_initial.set(params.scc.components, true);
+        }
+
         params.scc.components += 1;
     }
 }
@@ -261,6 +266,7 @@ fn recurse_onto(length: usize, bitvec: &BitVec, v: usize, params: &mut Params) {
 pub struct Sccs {
     components: usize,
     matrix: ReferenceMatrix,
+    not_initial: BitVec,
 }
 
 impl Sccs {
@@ -271,7 +277,7 @@ impl Sccs {
     ///
     /// It is guaranteed that a group is non-empty, and that flattening the groups gives a valid
     /// topological ordering.
-    pub fn groups(&self) -> std::iter::Take<bitvec::slice::ChunksExact<'_, Element, Order>> {
+    pub fn groups(&self) -> impl DoubleEndedIterator<Item = (&'_ BitSlice, bool)> {
         // work around a panic when requesting a chunk size of 0
         let length = if self.matrix.length == 0 {
             // the `.take(self.components)` ensures the resulting iterator will be empty
@@ -286,13 +292,15 @@ impl Sccs {
             .bitvec
             .chunks_exact(length)
             .take(self.components)
+            .enumerate()
+            .map(|(c, slice)| (slice, !self.not_initial[c]))
     }
 
     /// Reorder the input slice based on the SCCs. This produces a topological sort
     pub fn reorder<T>(&self, slice: &mut [T]) {
         debug_assert_eq!(self.matrix.length, slice.len());
 
-        let mut indices: Vec<_> = self.groups().flat_map(|s| s.iter_ones()).collect();
+        let mut indices: Vec<_> = self.groups().flat_map(|(s, _)| s.iter_ones()).collect();
 
         for i in 0..slice.len() {
             let mut index = indices[i];

crates/compiler/mono/src/borrow.rs

@@ -68,7 +68,7 @@ pub fn infer_borrow<'a>(
 
     let sccs = matrix.strongly_connected_components_all();
 
-    for group in sccs.groups() {
+    for (group, _) in sccs.groups() {
         // This is a fixed-point analysis
         //
         // all functions initiall own all their parameters

crates/compiler/problem/src/can.rs

@@ -40,6 +40,7 @@ pub enum Problem {
     /// Second symbol is the name of the argument that is unused
     UnusedArgument(Symbol, bool, Symbol, Region),
     UnusedBranchDef(Symbol, Region),
+    DefsOnlyUsedInRecursion(usize, Region),
     PrecedenceProblem(PrecedenceProblem),
     // Example: (5 = 1 + 2) is an unsupported pattern in an assignment; Int patterns aren't allowed in assignments!
     UnsupportedPattern(BadPattern, Region),
@@ -333,7 +334,8 @@ impl Problem {
             | Problem::BadTypeArguments { region, .. }
             | Problem::UnnecessaryOutputWildcard { region }
             | Problem::OverAppliedCrash { region }
-            | Problem::UnappliedCrash { region } => Some(*region),
+            | Problem::UnappliedCrash { region }
+            | Problem::DefsOnlyUsedInRecursion(_, region) => Some(*region),
             Problem::RuntimeError(RuntimeError::CircularDef(cycle_entries))
             | Problem::BadRecursion(cycle_entries) => {
                 cycle_entries.first().map(|entry| entry.expr_region)

crates/reporting/src/error/canonicalize.rs

@@ -123,6 +123,34 @@ pub fn can_problem<'b>(
             title = UNUSED_IMPORT.to_string();
             severity = Severity::Warning;
         }
+        Problem::DefsOnlyUsedInRecursion(1, region) => {
+            doc = alloc.stack([
+                alloc.reflow("This definition is only used in recursion with itself:"),
+                alloc.region(lines.convert_region(region)),
+                alloc.reflow(
+                    "If you don't intend to use or export this definition, it should be removed!",
+                ),
+            ]);
+
+            title = "DEFINITION ONLY USED IN RECURSION".to_string();
+            severity = Severity::Warning;
+        }
+        Problem::DefsOnlyUsedInRecursion(n, region) => {
+            doc = alloc.stack([
+                alloc.concat([
+                    alloc.reflow("These "),
+                    alloc.string(n.to_string()),
+                    alloc.reflow(" definitions are only used in mutual recursion with themselves:"),
+                ]),
+                alloc.region(lines.convert_region(region)),
+                alloc.reflow(
+                    "If you don't intend to use or export any of them, they should all be removed!",
+                ),
+            ]);
+
+            title = "DEFINITIONs ONLY USED IN RECURSION".to_string();
+            severity = Severity::Warning;
+        }
         Problem::ExposedButNotDefined(symbol) => {
             doc = alloc.stack([
                 alloc.symbol_unqualified(symbol).append(

crates/reporting/tests/test_reporting.rs

@@ -12628,4 +12628,147 @@ I recommend using camelCase. It's the standard style in Roc code!
             "#
         )
     );
+
+    test_report!(
+        self_recursive_not_reached,
+        indoc!(
+            r#"
+            app "test" provides [f] to "./platform"
+            f = h {}
+            h = \{} -> 1
+            g = \{} -> if Bool.true then "" else g {}
+            "#
+        ),
+    @r###"
+    ── DEFINITION ONLY USED IN RECURSION ───────────────────── /code/proj/Main.roc ─
+
+    This definition is only used in recursion with itself:
+
+    4│  g = \{} -> if Bool.true then "" else g {}
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+    If you don't intend to use or export this definition, it should be
+    removed!
+    "###
+    );
+
+    test_no_problem!(
+        self_recursive_not_reached_but_exposed,
+        indoc!(
+            r#"
+            app "test" provides [g] to "./platform"
+            g = \{} -> if Bool.true then "" else g {}
+            "#
+        )
+    );
+
+    test_report!(
+        mutual_recursion_not_reached,
+        indoc!(
+            r#"
+            app "test" provides [h] to "./platform"
+            h = ""
+            f = \{} -> if Bool.true then "" else g {}
+            g = \{} -> if Bool.true then "" else f {}
+            "#
+        ),
+    @r###"
+    ── DEFINITIONs ONLY USED IN RECURSION ──────────────────── /code/proj/Main.roc ─
+
+    These 2 definitions are only used in mutual recursion with themselves:
+
+    3│>  f = \{} -> if Bool.true then "" else g {}
+    4│>  g = \{} -> if Bool.true then "" else f {}
+
+    If you don't intend to use or export any of them, they should all be
+    removed!
+    "###
+    );
+
+    test_report!(
+        mutual_recursion_not_reached_but_exposed,
+        indoc!(
+            r#"
+            app "test" provides [f] to "./platform"
+            f = \{} -> if Bool.true then "" else g {}
+            g = \{} -> if Bool.true then "" else f {}
+            "#
+        ),
+    @r###"
+    "###
+    );
+
+    test_report!(
+        self_recursive_not_reached_nested,
+        indoc!(
+            r#"
+            app "test" provides [main] to "./platform"
+            main =
+                g = \{} -> if Bool.true then "" else g {}
+                ""
+            "#
+        ),
+    @r###"
+    ── DEFINITION ONLY USED IN RECURSION ───────────────────── /code/proj/Main.roc ─
+
+    This definition is only used in recursion with itself:
+
+    3│      g = \{} -> if Bool.true then "" else g {}
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+    If you don't intend to use or export this definition, it should be
+    removed!
+    "###
+    );
+
+    test_no_problem!(
+        self_recursive_not_reached_but_exposed_nested,
+        indoc!(
+            r#"
+            app "test" provides [main] to "./platform"
+            main =
+                g = \{} -> if Bool.true then "" else g {}
+                g
+            "#
+        )
+    );
+
+    test_report!(
+        mutual_recursion_not_reached_nested,
+        indoc!(
+            r#"
+            app "test" provides [main] to "./platform"
+            main =
+                f = \{} -> if Bool.true then "" else g {}
+                g = \{} -> if Bool.true then "" else f {}
+                ""
+            "#
+        ),
+    @r###"
+    ── DEFINITIONs ONLY USED IN RECURSION ──────────────────── /code/proj/Main.roc ─
+
+    These 2 definitions are only used in mutual recursion with themselves:
+
+    3│>      f = \{} -> if Bool.true then "" else g {}
+    4│>      g = \{} -> if Bool.true then "" else f {}
+
+    If you don't intend to use or export any of them, they should all be
+    removed!
+    "###
+    );
+
+    test_report!(
+        mutual_recursion_not_reached_but_exposed_nested,
+        indoc!(
+            r#"
+            app "test" provides [main] to "./platform"
+            main =
+                f = \{} -> if Bool.true then "" else g {}
+                g = \{} -> if Bool.true then "" else f {}
+                f
+            "#
+        ),
+    @r###"
+    "###
+    );
 }