Merge pull request #4507 from roc-lang/alias-analysis-rc-recursion

fix stack overflow in alias analysis

crates/cli/tests/cli_run.rs

@@ -147,13 +147,17 @@ mod cli_run {
             std::env::set_var("NO_AVX512", "1");
         }
 
-        let cli_commands = if test_many_cli_commands {
-            vec![CliMode::RocBuild, CliMode::RocRun, CliMode::Roc]
-        } else if cfg!(windows) {
-            // TODO: expects don't currently work on windows
-            vec![CliMode::RocRun]
+        // TODO: expects don't currently work on windows
+        let cli_commands = if cfg!(windows) {
+            match test_many_cli_commands {
+                true => vec![CliMode::RocBuild, CliMode::RocRun],
+                false => vec![CliMode::RocRun],
+            }
         } else {
-            vec![CliMode::Roc]
+            match test_many_cli_commands {
+                true => vec![CliMode::RocBuild, CliMode::RocRun, CliMode::Roc],
+                false => vec![CliMode::Roc],
+            }
         };
 
         for cli_mode in cli_commands.iter() {
@@ -248,7 +252,10 @@ mod cli_run {
                 ),
             };
 
-            if !&out.stdout.ends_with(expected_ending) {
+            // strip out any carriage return characters to make the output on windows match unix
+            let stdout = out.stdout.replace('\r', "");
+
+            if !stdout.ends_with(expected_ending) {
                 panic!(
                     "expected output to end with {:?} but instead got {:#?} - stderr was: {:#?}",
                     expected_ending, out.stdout, out.stderr
@@ -575,7 +582,6 @@ mod cli_run {
     }
 
     #[test]
-    #[cfg_attr(windows, ignore = "overflows the stack on windows")]
     fn false_interpreter() {
         test_roc_app(
             "examples/cli/false-interpreter",

crates/compiler/alias_analysis/src/lib.rs

@@ -441,6 +441,36 @@ struct Env<'a> {
     type_names: MutSet<UnionLayout<'a>>,
 }
 
+fn apply_refcount_operation<'a>(
+    builder: &mut FuncDefBuilder,
+    env: &mut Env<'a>,
+    block: BlockId,
+    modify_rc: &ModifyRc,
+) -> Result<()> {
+    match modify_rc {
+        ModifyRc::Inc(symbol, _) => {
+            let argument = env.symbols[symbol];
+
+            // a recursive touch is never worse for optimizations than a normal touch
+            // and a bit more permissive in its type
+            builder.add_recursive_touch(block, argument)?;
+        }
+
+        ModifyRc::Dec(symbol) => {
+            let argument = env.symbols[symbol];
+
+            builder.add_recursive_touch(block, argument)?;
+        }
+        ModifyRc::DecRef(symbol) => {
+            let argument = env.symbols[symbol];
+
+            builder.add_recursive_touch(block, argument)?;
+        }
+    };
+
+    Ok(())
+}
+
 fn stmt_spec<'a>(
     builder: &mut FuncDefBuilder,
     interner: &STLayoutInterner<'a>,
@@ -458,12 +488,25 @@ fn stmt_spec<'a>(
 
             let mut queue = vec![symbol];
 
-            while let Let(symbol, expr, expr_layout, c) = continuation {
-                let value_id = expr_spec(builder, interner, env, block, expr_layout, expr)?;
-                env.symbols.insert(*symbol, value_id);
+            loop {
+                match continuation {
+                    Let(symbol, expr, expr_layout, c) => {
+                        let value_id = expr_spec(builder, interner, env, block, expr_layout, expr)?;
+                        env.symbols.insert(*symbol, value_id);
 
-                queue.push(symbol);
-                continuation = c;
+                        queue.push(symbol);
+                        continuation = c;
+                    }
+                    Refcounting(modify_rc, c) => {
+                        // in practice it is common to see a chain of `Let`s interspersed with
+                        // Inc/Dec. For e.g. the False interpreter, this caused stack overflows.
+                        // so we handle RC operations here to limit recursion depth
+                        apply_refcount_operation(builder, env, block, modify_rc)?;
+
+                        continuation = c;
+                    }
+                    _ => break,
+                }
             }
 
             let result = stmt_spec(builder, interner, env, block, layout, continuation)?;
@@ -499,32 +542,11 @@ fn stmt_spec<'a>(
         Expect { remainder, .. } => stmt_spec(builder, interner, env, block, layout, remainder),
         ExpectFx { remainder, .. } => stmt_spec(builder, interner, env, block, layout, remainder),
         Ret(symbol) => Ok(env.symbols[symbol]),
-        Refcounting(modify_rc, continuation) => match modify_rc {
-            ModifyRc::Inc(symbol, _) => {
-                let argument = env.symbols[symbol];
+        Refcounting(modify_rc, continuation) => {
+            apply_refcount_operation(builder, env, block, modify_rc)?;
 
-                // a recursive touch is never worse for optimizations than a normal touch
-                // and a bit more permissive in its type
-                builder.add_recursive_touch(block, argument)?;
-
-                stmt_spec(builder, interner, env, block, layout, continuation)
-            }
-
-            ModifyRc::Dec(symbol) => {
-                let argument = env.symbols[symbol];
-
-                builder.add_recursive_touch(block, argument)?;
-
-                stmt_spec(builder, interner, env, block, layout, continuation)
-            }
-            ModifyRc::DecRef(symbol) => {
-                let argument = env.symbols[symbol];
-
-                builder.add_recursive_touch(block, argument)?;
-
-                stmt_spec(builder, interner, env, block, layout, continuation)
-            }
-        },
+            stmt_spec(builder, interner, env, block, layout, continuation)
+        }
         Join {
             id,
             parameters,

examples/cli/false-interpreter/False.roc

@@ -78,7 +78,7 @@ isWhitespace = \char ->
     char
     == 0xA # new line
     || char
-    == 0xB # carriage return
+    == 0xD # carriage return
     || char
     == 0x20 # space
     || char