Merge branch 'trunk' of github.com:rtfeldman/roc into http-sync

crates/cli/src/lib.rs

@@ -20,6 +20,7 @@ use std::io;
 use std::os::raw::{c_char, c_int};
 use std::path::{Path, PathBuf};
 use std::process;
+use std::time::Instant;
 use target_lexicon::BinaryFormat;
 use target_lexicon::{
     Architecture, Environment, OperatingSystem, Triple, Vendor, X86_32Architecture,
@@ -288,6 +289,7 @@ pub enum FormatMode {
 const SHM_SIZE: i64 = 1024;
 
 pub fn test(matches: &ArgMatches, triple: Triple) -> io::Result<i32> {
+    let start_time = Instant::now();
     let arena = Bump::new();
     let filename = matches.value_of_os(ROC_FILE).unwrap();
     let opt_level = match (
@@ -439,12 +441,31 @@ pub fn test(matches: &ArgMatches, triple: Triple) -> io::Result<i32> {
         }
     }
 
-    if failed > 0 {
+    let total_time = start_time.elapsed();
-        println!("test result: failed. {passed} passed; {failed} failed;");
+
-        Ok(1)
+    if failed == 0 && passed == 0 {
+        // TODO print this in a more nicely formatted way!
+        println!("No expectations were found.");
+
+        // If no tests ran, treat that as an error. This is perhaps
+        // briefly annoying at the very beginning of a project when
+        // you actually have zero tests, but it can save you from
+        // having a change to your CI script accidentally stop
+        // running tests altogether!
+        Ok(2)
     } else {
-        println!("test result: ok. {passed} passed; {failed} failed;");
+        let failed_color = if failed == 0 {
-        Ok(0)
+            32 // green
+        } else {
+            31 // red
+        };
+
+        println!(
+            "\x1B[{failed_color}m{failed}\x1B[39m failed and \x1B[32m{passed}\x1B[39m passed in {} ms.\n",
+            total_time.as_millis(),
+        );
+
+        Ok((failed > 0) as i32)
     }
 }
 
@@ -1075,14 +1096,13 @@ fn render_expect_failure<'a>(
         alloc.stack([
             alloc.text("This expectation failed:"),
             alloc.region(line_col_region),
-            alloc.text("The variables used in this expression are:"),
+            alloc.text("When it failed, these variables had these values:"),
             alloc.stack(it),
         ])
     } else {
         alloc.stack([
             alloc.text("This expectation failed:"),
             alloc.region(line_col_region),
-            alloc.text("I did not record any variables in this expression."),
         ])
     };
 

crates/compiler/alias_analysis/src/lib.rs

@@ -194,7 +194,7 @@ where
                         RawFunctionLayout::ZeroArgumentThunk(_) => {
                             let bytes = func_name_bytes_help(
                                 *symbol,
-                                [Layout::UNIT],
+                                [],
                                 CapturesNiche::no_niche(),
                                 &top_level.result,
                             );

crates/compiler/builtins/bitcode/src/str.zig

@@ -1977,23 +1977,26 @@ pub fn strTrim(string: RocStr) callconv(.C) RocStr {
         const small_or_shared = new_len <= SMALL_STR_MAX_LENGTH or !string.isRefcountOne();
         if (small_or_shared) {
             return RocStr.init(string.asU8ptr() + leading_bytes, new_len);
-        }
+        } else {
+            // nonempty, large, and unique: shift everything over in-place if necessary.
+            // Note: must use memmove over memcpy, because the bytes definitely overlap!
+            if (leading_bytes > 0) {
+                // Zig doesn't seem to have `memmove` in the stdlib anymore; this is based on:
+                // https://github.com/ziglang/zig/blob/52ba2c3a43a88a4db30cff47f2f3eff8c3d5be19/lib/std/special/c.zig#L115
+                // Copyright Andrew Kelley, MIT licensed.
+                const src = bytes_ptr + leading_bytes;
+                var index: usize = 0;
 
-        // nonempty, large, and unique:
+                while (index != new_len) : (index += 1) {
-
+                    bytes_ptr[index] = src[index];
-        if (leading_bytes > 0) {
+                }
-            var i: usize = 0;
-            while (i < new_len) : (i += 1) {
-                const dest = bytes_ptr + i;
-                const source = dest + leading_bytes;
-                @memcpy(dest, source, 1);
             }
+
+            var new_string = string;
+            new_string.str_len = new_len;
+
+            return new_string;
         }
-
-        var new_string = string;
-        new_string.str_len = new_len;
-
-        return new_string;
     }
 
     return RocStr.empty();
@@ -2014,23 +2017,26 @@ pub fn strTrimLeft(string: RocStr) callconv(.C) RocStr {
         const small_or_shared = new_len <= SMALL_STR_MAX_LENGTH or !string.isRefcountOne();
         if (small_or_shared) {
             return RocStr.init(string.asU8ptr() + leading_bytes, new_len);
-        }
+        } else {
+            // nonempty, large, and unique: shift everything over in-place if necessary.
+            // Note: must use memmove over memcpy, because the bytes definitely overlap!
+            if (leading_bytes > 0) {
+                // Zig doesn't seem to have `memmove` in the stdlib anymore; this is based on:
+                // https://github.com/ziglang/zig/blob/52ba2c3a43a88a4db30cff47f2f3eff8c3d5be19/lib/std/special/c.zig#L115
+                // Copyright Andrew Kelley, MIT licensed.
+                const src = bytes_ptr + leading_bytes;
+                var index: usize = 0;
 
-        // nonempty, large, and unique:
+                while (index != new_len) : (index += 1) {
-
+                    bytes_ptr[index] = src[index];
-        if (leading_bytes > 0) {
+                }
-            var i: usize = 0;
-            while (i < new_len) : (i += 1) {
-                const dest = bytes_ptr + i;
-                const source = dest + leading_bytes;
-                @memcpy(dest, source, 1);
             }
+
+            var new_string = string;
+            new_string.str_len = new_len;
+
+            return new_string;
         }
-
-        var new_string = string;
-        new_string.str_len = new_len;
-
-        return new_string;
     }
 
     return RocStr.empty();

crates/compiler/gen_llvm/src/llvm/build.rs

@@ -4917,8 +4917,22 @@ pub fn build_proc<'a, 'ctx, 'env>(
                         )
                     }
 
-                    RawFunctionLayout::ZeroArgumentThunk(_) => {
+                    RawFunctionLayout::ZeroArgumentThunk(result) => {
-                        // do nothing
+                        // Define only the return value size, since this is a thunk
+                        //
+                        // * roc__mainForHost_1_Update_result_size() -> i64
+                        let ident_string = proc.name.name().as_str(&env.interns);
+                        let fn_name: String = format!("{}_1", ident_string);
+
+                        let result_type = basic_type_from_layout(env, &result);
+
+                        build_host_exposed_alias_size_help(
+                            env,
+                            &fn_name,
+                            name,
+                            Some("result"),
+                            result_type,
+                        );
                     }
                 }
             }

crates/compiler/load_internal/src/file.rs

@@ -2165,6 +2165,14 @@ fn update<'a>(
                 header
                     .imported_modules
                     .insert(ModuleId::LIST, Region::zero());
+
+                header
+                    .package_qualified_imported_modules
+                    .insert(PackageQualified::Unqualified(ModuleId::ENCODE));
+
+                header
+                    .imported_modules
+                    .insert(ModuleId::ENCODE, Region::zero());
             }
 
             state

crates/compiler/mono/src/ir.rs

@@ -1086,8 +1086,6 @@ impl<'a> Procs<'a> {
                             // (We had a bug around this before this system existed!)
                             self.specialized.mark_in_progress(name.name(), layout);
 
-                            let outside_layout = layout;
-
                             let partial_proc_id = if let Some(partial_proc_id) =
                                 self.partial_procs.symbol_to_id(name.name())
                             {
@@ -1126,26 +1124,29 @@ impl<'a> Procs<'a> {
                                 &[],
                                 partial_proc_id,
                             ) {
-                                Ok((proc, layout)) => {
+                                Ok((proc, _ignore_layout)) => {
-                                    let top_level = ProcLayout::from_raw(
+                                    // the `layout` is a function pointer, while `_ignore_layout` can be a
+                                    // closure. We only specialize functions, storing this value with a closure
+                                    // layout will give trouble.
+                                    let arguments = Vec::from_iter_in(
+                                        proc.args.iter().map(|(l, _)| *l),
                                         env.arena,
-                                        layout,
+                                    )
-                                        proc.name.captures_niche(),
+                                    .into_bump_slice();
-                                    );
 
-                                    debug_assert_eq!(
+                                    let proper_layout = ProcLayout {
-                                        outside_layout, top_level,
+                                        arguments,
-                                        "different raw layouts for {:?}",
+                                        result: proc.ret_layout,
-                                        proc.name
+                                        captures_niche: proc.name.captures_niche(),
-                                    );
+                                    };
-
-                                    if self.is_module_thunk(proc.name.name()) {
-                                        debug_assert!(top_level.arguments.is_empty());
-                                    }
 
+                                    // NOTE: some functions are specialized to have a closure, but don't actually
+                                    // need any closure argument. Here is where we correct this sort of thing,
+                                    // by trusting the layout of the Proc, not of what we specialize for
+                                    self.specialized.remove_specialized(name.name(), &layout);
                                     self.specialized.insert_specialized(
                                         name.name(),
-                                        top_level,
+                                        proper_layout,
                                         proc,
                                     );
                                 }
@@ -1240,7 +1241,7 @@ impl<'a> Procs<'a> {
                             captures_niche: proc.name.captures_niche(),
                         };
 
-                        // NOTE: some function are specialized to have a closure, but don't actually
+                        // NOTE: some functions are specialized to have a closure, but don't actually
                         // need any closure argument. Here is where we correct this sort of thing,
                         // by trusting the layout of the Proc, not of what we specialize for
                         self.specialized.remove_specialized(symbol.name(), &layout);
@@ -2660,23 +2661,38 @@ fn specialize_suspended<'a>(
         };
 
         match specialize_variable(env, procs, name, layout_cache, var, &[], partial_proc) {
-            Ok((proc, layout)) => {
+            Ok((proc, _layout)) => {
-                // TODO thiscode is duplicated elsewhere
+                // TODO this code is duplicated elsewhere
-                let top_level = ProcLayout::from_raw(env.arena, layout, proc.name.captures_niche());
 
+                // the `layout` is a function pointer, while `_ignore_layout` can be a
+                // closure. We only specialize functions, storing this value with a closure
+                // layout will give trouble.
+                let arguments = Vec::from_iter_in(proc.args.iter().map(|(l, _)| *l), env.arena)
+                    .into_bump_slice();
+
+                let proper_layout = ProcLayout {
+                    arguments,
+                    result: proc.ret_layout,
+                    captures_niche: proc.name.captures_niche(),
+                };
                 if procs.is_module_thunk(proc.name.name()) {
                     debug_assert!(
-                        top_level.arguments.is_empty(),
+                        proper_layout.arguments.is_empty(),
                         "{:?} from {:?}",
                         name,
-                        layout
+                        proper_layout
                     );
                 }
 
-                debug_assert_eq!(outside_layout, top_level, " in {:?}", name);
+                // NOTE: some functions are specialized to have a closure, but don't actually
+                // need any closure argument. Here is where we correct this sort of thing,
+                // by trusting the layout of the Proc, not of what we specialize for
                 procs
                     .specialized
-                    .insert_specialized(name.name(), top_level, proc);
+                    .remove_specialized(name.name(), &outside_layout);
+                procs
+                    .specialized
+                    .insert_specialized(name.name(), proper_layout, proc);
             }
             Err(SpecializeFailure {
                 attempted_layout, ..
@@ -3005,8 +3021,35 @@ fn specialize_external<'a>(
 
                     aliases.insert(*symbol, (name, top_level, layout));
                 }
-                RawFunctionLayout::ZeroArgumentThunk(_) => {
+                RawFunctionLayout::ZeroArgumentThunk(result) => {
-                    unreachable!("so far");
+                    let assigned = env.unique_symbol();
+                    let hole = env.arena.alloc(Stmt::Ret(assigned));
+                    let forced = force_thunk(env, lambda_name.name(), result, assigned, hole);
+
+                    let proc = Proc {
+                        name: LambdaName::no_niche(name),
+                        args: &[],
+                        body: forced,
+                        closure_data_layout: None,
+                        ret_layout: result,
+                        is_self_recursive: SelfRecursive::NotSelfRecursive,
+                        must_own_arguments: false,
+                        host_exposed_layouts: HostExposedLayouts::NotHostExposed,
+                    };
+
+                    let top_level =
+                        ProcLayout::from_raw(env.arena, layout, CapturesNiche::no_niche());
+
+                    procs.specialized.insert_specialized(name, top_level, proc);
+
+                    aliases.insert(
+                        *symbol,
+                        (
+                            name,
+                            ProcLayout::new(env.arena, &[], CapturesNiche::no_niche(), result),
+                            layout,
+                        ),
+                    );
                 }
             }
         }

crates/compiler/parse/src/blankspace.rs

@@ -197,11 +197,7 @@ where
                 Err((MadeProgress, indent_problem(state.pos()), state))
             } else {
                 let comments_and_newlines = Vec::with_capacity_in(newlines, arena);
-                let mut spaces = eat_spaces(state, false, comments_and_newlines);
+                let spaces = eat_spaces(state, comments_and_newlines);
-
-                if spaces.multiline {
-                    spaces.state.indent_column = spaces.state.column();
-                }
 
                 Ok((
                     MadeProgress,
@@ -324,13 +320,11 @@ fn fast_eat_spaces(state: &State) -> FastSpaceState {
 
 struct SpaceState<'a> {
     state: State<'a>,
-    multiline: bool,
     comments_and_newlines: Vec<'a, CommentOrNewline<'a>>,
 }
 
 fn eat_spaces<'a>(
     mut state: State<'a>,
-    mut multiline: bool,
     mut comments_and_newlines: Vec<'a, CommentOrNewline<'a>>,
 ) -> SpaceState<'a> {
     for c in state.bytes() {
@@ -340,7 +334,6 @@ fn eat_spaces<'a>(
             }
             b'\n' => {
                 state = state.advance_newline();
-                multiline = true;
                 comments_and_newlines.push(CommentOrNewline::Newline);
             }
             b'\r' => {
@@ -350,7 +343,7 @@ fn eat_spaces<'a>(
 
             b'#' => {
                 state = state.advance(1);
-                return eat_line_comment(state, multiline, comments_and_newlines);
+                return eat_line_comment(state, comments_and_newlines);
             }
             _ => break,
         }
@@ -358,14 +351,12 @@ fn eat_spaces<'a>(
 
     SpaceState {
         state,
-        multiline,
         comments_and_newlines,
     }
 }
 
 fn eat_line_comment<'a>(
     mut state: State<'a>,
-    mut multiline: bool,
     mut comments_and_newlines: Vec<'a, CommentOrNewline<'a>>,
 ) -> SpaceState<'a> {
     let mut index = state.pos().offset as usize;
@@ -388,7 +379,6 @@ fn eat_line_comment<'a>(
                     index += 2;
 
                     comments_and_newlines.push(CommentOrNewline::DocComment(""));
-                    multiline = true;
 
                     for c in state.bytes() {
                         match c {
@@ -397,7 +387,6 @@ fn eat_line_comment<'a>(
                             }
                             b'\n' => {
                                 state = state.advance_newline();
-                                multiline = true;
                                 comments_and_newlines.push(CommentOrNewline::Newline);
                             }
                             b'\r' => {
@@ -417,7 +406,6 @@ fn eat_line_comment<'a>(
 
                     return SpaceState {
                         state,
-                        multiline,
                         comments_and_newlines,
                     };
                 }
@@ -427,7 +415,6 @@ fn eat_line_comment<'a>(
 
                     return SpaceState {
                         state,
-                        multiline,
                         comments_and_newlines,
                     };
                 }
@@ -483,7 +470,6 @@ fn eat_line_comment<'a>(
                                 comments_and_newlines.push(CommentOrNewline::LineComment(comment));
                             }
                             state = state.advance_newline();
-                            multiline = true;
 
                             index += 1;
                             while index < length {
@@ -493,7 +479,6 @@ fn eat_line_comment<'a>(
                                     }
                                     b'\n' => {
                                         state = state.advance_newline();
-                                        multiline = true;
                                         comments_and_newlines.push(CommentOrNewline::Newline);
                                     }
                                     b'\r' => {
@@ -513,7 +498,6 @@ fn eat_line_comment<'a>(
 
                             return SpaceState {
                                 state,
-                                multiline,
                                 comments_and_newlines,
                             };
                         }
@@ -550,7 +534,6 @@ fn eat_line_comment<'a>(
                         comments_and_newlines.push(CommentOrNewline::LineComment(comment));
                     }
                     state = state.advance_newline();
-                    multiline = true;
 
                     index += 1;
                     while index < length {
@@ -560,7 +543,6 @@ fn eat_line_comment<'a>(
                             }
                             b'\n' => {
                                 state = state.advance_newline();
-                                multiline = true;
                                 comments_and_newlines.push(CommentOrNewline::Newline);
                             }
                             b'\r' => {
@@ -580,7 +562,6 @@ fn eat_line_comment<'a>(
 
                     return SpaceState {
                         state,
-                        multiline,
                         comments_and_newlines,
                     };
                 }
@@ -606,7 +587,6 @@ fn eat_line_comment<'a>(
 
         return SpaceState {
             state,
-            multiline,
             comments_and_newlines,
         };
     }

crates/compiler/parse/src/expr.rs

@@ -1992,9 +1992,10 @@ mod when {
     /// Parsing when with indentation.
     fn when_with_indent<'a>() -> impl Parser<'a, u32, EWhen<'a>> {
         move |arena, state: State<'a>| {
+            let min_indent = state.column();
             parser::keyword_e(keyword::WHEN, EWhen::When)
                 .parse(arena, state)
-                .map(|(progress, (), state)| (progress, state.indent_column, state))
+                .map(|(progress, (), state)| (progress, min_indent, state))
         }
     }
 
@@ -2003,14 +2004,12 @@ mod when {
         options: ExprParseOptions,
     ) -> impl Parser<'a, Vec<'a, &'a WhenBranch<'a>>, EWhen<'a>> {
         move |arena, state: State<'a>| {
-            let when_indent = state.indent_column;
-
             let mut branches: Vec<'a, &'a WhenBranch<'a>> = Vec::with_capacity_in(2, arena);
 
             // 1. Parse the first branch and get its indentation level. (It must be >= min_indent.)
             // 2. Parse the other branches. Their indentation levels must be == the first branch's.
 
-            let (_, ((pattern_indent_level, loc_first_patterns), loc_first_guard), mut state): (
+            let (_, ((pattern_indent_level, loc_first_patterns), loc_first_guard), state): (
                 _,
                 ((_, _), _),
                 State<'a>,
@@ -2018,8 +2017,6 @@ mod when {
 
             let original_indent = pattern_indent_level;
 
-            state.indent_column = pattern_indent_level;
-
             // Parse the first "->" and the expression after it.
             let (_, loc_first_expr, mut state) =
                 branch_result(original_indent + 1).parse(arena, state)?;
@@ -2078,9 +2075,6 @@ mod when {
                 }
             }
 
-            let mut state = state;
-            state.indent_column = when_indent;
-
             Ok((MadeProgress, branches, state))
         }
     }
@@ -2382,7 +2376,7 @@ where
     E: 'a,
 {
     move |arena, state: State<'a>| {
-        let indent_column = state.indent_column;
+        let indent_column = state.column();
 
         let (progress, _, state) = parser.parse(arena, state)?;
 

crates/compiler/parse/src/state.rs

@@ -14,10 +14,6 @@ pub struct State<'a> {
 
     /// Position of the start of the current line
     pub(crate) line_start: Position,
-
-    /// Current indentation level, in columns
-    /// (so no indent is col 1 - this saves an arithmetic operation.)
-    pub(crate) indent_column: u32,
 }
 
 impl<'a> State<'a> {
@@ -26,7 +22,6 @@ impl<'a> State<'a> {
             original_bytes: bytes,
             offset: 0,
             line_start: Position::zero(),
-            indent_column: 0,
         }
     }
 
@@ -86,7 +81,6 @@ impl<'a> fmt::Debug for State<'a> {
         }
 
         write!(f, "\n\t(offset): {:?},", self.pos())?;
-        write!(f, "\n\tindent_column: {}", self.indent_column)?;
         write!(f, "\n}}")
     }
 }

crates/compiler/parse/tests/snapshots/pass/when_in_assignment.expr.result-ast

@@ -0,0 +1,57 @@
+Defs(
+    Defs {
+        tags: [
+            Index(2147483648),
+        ],
+        regions: [
+            @0-25,
+        ],
+        space_before: [
+            Slice(start = 0, length = 0),
+        ],
+        space_after: [
+            Slice(start = 0, length = 0),
+        ],
+        spaces: [],
+        type_defs: [],
+        value_defs: [
+            Body(
+                @0-1 Identifier(
+                    "x",
+                ),
+                @4-25 When(
+                    @9-10 Var {
+                        module_name: "",
+                        ident: "n",
+                    },
+                    [
+                        WhenBranch {
+                            patterns: [
+                                @19-20 SpaceBefore(
+                                    NumLiteral(
+                                        "0",
+                                    ),
+                                    [
+                                        Newline,
+                                    ],
+                                ),
+                            ],
+                            value: @24-25 Num(
+                                "0",
+                            ),
+                            guard: None,
+                        },
+                    ],
+                ),
+            ),
+        ],
+    },
+    @26-28 SpaceBefore(
+        Num(
+            "42",
+        ),
+        [
+            Newline,
+        ],
+    ),
+)

crates/compiler/parse/tests/snapshots/pass/when_in_assignment.expr.roc

@@ -0,0 +1,3 @@
+x = when n is
+     0 -> 0
+42

crates/compiler/parse/tests/snapshots/pass/when_in_function.expr.result-ast

@@ -0,0 +1,64 @@
+Defs(
+    Defs {
+        tags: [
+            Index(2147483648),
+        ],
+        regions: [
+            @0-43,
+        ],
+        space_before: [
+            Slice(start = 0, length = 0),
+        ],
+        space_after: [
+            Slice(start = 0, length = 0),
+        ],
+        spaces: [],
+        type_defs: [],
+        value_defs: [
+            Body(
+                @0-4 Identifier(
+                    "func",
+                ),
+                @7-43 Closure(
+                    [
+                        @8-9 Identifier(
+                            "x",
+                        ),
+                    ],
+                    @13-43 When(
+                        @18-19 Var {
+                            module_name: "",
+                            ident: "n",
+                        },
+                        [
+                            WhenBranch {
+                                patterns: [
+                                    @37-38 SpaceBefore(
+                                        NumLiteral(
+                                            "0",
+                                        ),
+                                        [
+                                            Newline,
+                                        ],
+                                    ),
+                                ],
+                                value: @42-43 Num(
+                                    "0",
+                                ),
+                                guard: None,
+                            },
+                        ],
+                    ),
+                ),
+            ),
+        ],
+    },
+    @44-46 SpaceBefore(
+        Num(
+            "42",
+        ),
+        [
+            Newline,
+        ],
+    ),
+)

crates/compiler/parse/tests/snapshots/pass/when_in_function.expr.roc

@@ -0,0 +1,3 @@
+func = \x -> when n is
+              0 -> 0
+42

crates/compiler/parse/tests/test_parse.rs

@@ -122,18 +122,16 @@ mod test_parse {
     snapshot_tests! {
         fail/type_argument_no_arrow.expr,
         fail/type_double_comma.expr,
-        pass/plus_if.expr,
-        pass/list_closing_indent_not_enough.expr,
-        pass/ability_single_line.expr,
-        pass/ability_multi_line.expr,
         pass/ability_demand_signature_is_multiline.expr,
+        pass/ability_multi_line.expr,
+        pass/ability_single_line.expr,
         pass/ability_two_in_a_row.expr,
         pass/add_var_with_spaces.expr,
         pass/add_with_spaces.expr,
         pass/annotated_record_destructure.expr,
         pass/annotated_tag_destructure.expr,
-        pass/apply_tag.expr,
         pass/apply_parenthetical_tag_args.expr,
+        pass/apply_tag.expr,
         pass/apply_three_args.expr,
         pass/apply_two_args.expr,
         pass/apply_unary_negation.expr,
@@ -151,9 +149,8 @@ mod test_parse {
         pass/comment_with_non_ascii.expr,
         pass/destructure_tag_assignment.expr,
         pass/empty_app_header.header,
-        pass/empty_interface_header.header,
         pass/empty_hosted_header.header,
-        pass/nonempty_hosted_header.header,
+        pass/empty_interface_header.header,
         pass/empty_list.expr,
         pass/empty_platform_header.header,
         pass/empty_record.expr,
@@ -161,7 +158,6 @@ mod test_parse {
         pass/equals_with_spaces.expr,
         pass/equals.expr,
         pass/expect.expr,
-        pass/record_type_with_function.expr,
         pass/float_with_underscores.expr,
         pass/full_app_header_trailing_commas.header,
         pass/full_app_header.header,
@@ -171,9 +167,10 @@ mod test_parse {
         pass/if_def.expr,
         pass/int_with_underscore.expr,
         pass/interface_with_newline.header,
-        pass/lowest_float.expr,
+        pass/list_closing_indent_not_enough.expr,
         pass/list_closing_same_indent_no_trailing_comma.expr,
         pass/list_closing_same_indent_with_trailing_comma.expr,
+        pass/lowest_float.expr,
         pass/lowest_int.expr,
         pass/malformed_ident_due_to_underscore.expr,
         pass/malformed_pattern_field_access.expr, // See https://github.com/rtfeldman/roc/issues/399
@@ -202,6 +199,7 @@ mod test_parse {
         pass/newline_before_sub.expr,
         pass/newline_inside_empty_list.expr,
         pass/newline_singleton_list.expr,
+        pass/nonempty_hosted_header.header,
         pass/nonempty_platform_header.header,
         pass/not_docs.expr,
         pass/number_literal_suffixes.expr,
@@ -212,16 +210,16 @@ mod test_parse {
         pass/one_plus_two.expr,
         pass/one_spaced_def.expr,
         pass/opaque_has_abilities.expr,
+        pass/opaque_reference_expr_with_arguments.expr,
+        pass/opaque_reference_expr.expr,
+        pass/opaque_reference_pattern_with_arguments.expr,
+        pass/opaque_reference_pattern.expr,
         pass/opaque_simple.module,
         pass/opaque_with_type_arguments.module,
-        pass/opaque_reference_expr.expr,
-        pass/opaque_reference_expr_with_arguments.expr,
-        pass/opaque_reference_pattern.expr,
-        pass/opaque_reference_pattern_with_arguments.expr,
         pass/ops_with_newlines.expr,
+        pass/outdented_app_with_record.expr,
         pass/outdented_list.expr,
         pass/outdented_record.expr,
-        pass/outdented_app_with_record.expr,
         pass/packed_singleton_list.expr,
         pass/parenthetical_apply.expr,
         pass/parenthetical_basic_field.expr,
@@ -230,6 +228,7 @@ mod test_parse {
         pass/parse_alias.expr,
         pass/parse_as_ann.expr,
         pass/pattern_with_space_in_parens.expr, // https://github.com/rtfeldman/roc/issues/929
+        pass/plus_if.expr,
         pass/pos_inf_float.expr,
         pass/positive_float.expr,
         pass/positive_int.expr,
@@ -239,6 +238,7 @@ mod test_parse {
         pass/qualified_var.expr,
         pass/record_destructure_def.expr,
         pass/record_func_type_decl.expr,
+        pass/record_type_with_function.expr,
         pass/record_update.expr,
         pass/record_with_if.expr,
         pass/requires_type.header,
@@ -272,6 +272,8 @@ mod test_parse {
         pass/var_minus_two.expr,
         pass/var_then.expr,
         pass/var_when.expr,
+        pass/when_in_assignment.expr,
+        pass/when_in_function.expr,
         pass/when_if_guard.expr,
         pass/when_in_parens_indented.expr,
         pass/when_in_parens.expr,
@@ -281,9 +283,9 @@ mod test_parse {
         pass/when_with_numbers.expr,
         pass/when_with_records.expr,
         pass/where_clause_function.expr,
-        pass/where_clause_non_function.expr,
-        pass/where_clause_multiple_has.expr,
         pass/where_clause_multiple_has_across_newlines.expr,
+        pass/where_clause_multiple_has.expr,
+        pass/where_clause_non_function.expr,
         pass/where_clause_on_newline.expr,
         pass/zero_float.expr,
         pass/zero_int.expr,

crates/compiler/solve/src/solve.rs

@@ -2984,6 +2984,15 @@ fn type_to_variable<'a>(
                         let copy_var = helper!(arg_type);
                         subs.variables[target_index] = copy_var;
                     }
+                    let it = (new_variables.indices().skip(type_arguments.len()))
+                        .zip(lambda_set_variables);
+                    for (target_index, ls) in it {
+                        // We MUST do this now, otherwise when linking the ambient function during
+                        // instantiation of the real var, there will be nothing to link against.
+                        let copy_var =
+                            type_to_variable(subs, rank, pools, arena, aliases, &ls.0, true);
+                        subs.variables[target_index] = copy_var;
+                    }
 
                     AliasVariables {
                         variables_start: new_variables.start,

crates/compiler/types/src/subs.rs

@@ -1905,18 +1905,9 @@ impl Subs {
     /// reference chain r -> t1 -> t2 -> r, [occurs] will return `Err(r, [t2, t1, r])`.
     ///
     /// This ignores [Content::RecursionVar]s that occur recursively, because those are
-    /// already priced in and expected to occur. Use [Subs::occurs_including_recursion_vars] if you
+    /// already priced in and expected to occur.
-    /// need to check for recursion var occurrence.
     pub fn occurs(&self, var: Variable) -> Result<(), (Variable, Vec<Variable>)> {
-        occurs(self, &[], var, false)
+        occurs(self, &[], var)
-    }
-
-    /// Like [Subs::occurs], but also errors when recursion vars occur.
-    pub fn occurs_including_recursion_vars(
-        &self,
-        var: Variable,
-    ) -> Result<(), (Variable, Vec<Variable>)> {
-        occurs(self, &[], var, true)
     }
 
     pub fn mark_tag_union_recursive(
@@ -3072,7 +3063,6 @@ fn occurs(
     subs: &Subs,
     seen: &[Variable],
     input_var: Variable,
-    include_recursion_var: bool,
 ) -> Result<(), (Variable, Vec<Variable>)> {
     use self::Content::*;
     use self::FlatType::*;
@@ -3096,53 +3086,34 @@ fn occurs(
                 new_seen.push(root_var);
 
                 match flat_type {
-                    Apply(_, args) => short_circuit(
+                    Apply(_, args) => {
-                        subs,
+                        short_circuit(subs, root_var, &new_seen, subs.get_subs_slice(*args).iter())
-                        root_var,
+                    }
-                        &new_seen,
-                        subs.get_subs_slice(*args).iter(),
-                        include_recursion_var,
-                    ),
                     Func(arg_vars, closure_var, ret_var) => {
                         let it = once(ret_var)
                             .chain(once(closure_var))
                             .chain(subs.get_subs_slice(*arg_vars).iter());
-                        short_circuit(subs, root_var, &new_seen, it, include_recursion_var)
+                        short_circuit(subs, root_var, &new_seen, it)
                     }
                     Record(vars_by_field, ext_var) => {
                         let slice =
                             SubsSlice::new(vars_by_field.variables_start, vars_by_field.length);
                         let it = once(ext_var).chain(subs.get_subs_slice(slice).iter());
-                        short_circuit(subs, root_var, &new_seen, it, include_recursion_var)
+                        short_circuit(subs, root_var, &new_seen, it)
                     }
                     TagUnion(tags, ext_var) => {
-                        occurs_union(subs, root_var, &new_seen, include_recursion_var, tags)?;
+                        occurs_union(subs, root_var, &new_seen, tags)?;
 
-                        short_circuit_help(
+                        short_circuit_help(subs, root_var, &new_seen, *ext_var)
-                            subs,
-                            root_var,
-                            &new_seen,
-                            *ext_var,
-                            include_recursion_var,
-                        )
                     }
                     FunctionOrTagUnion(_, _, ext_var) => {
                         let it = once(ext_var);
-                        short_circuit(subs, root_var, &new_seen, it, include_recursion_var)
+                        short_circuit(subs, root_var, &new_seen, it)
                     }
-                    RecursiveTagUnion(rec_var, tags, ext_var) => {
+                    RecursiveTagUnion(_, tags, ext_var) => {
-                        if include_recursion_var {
+                        occurs_union(subs, root_var, &new_seen, tags)?;
-                            new_seen.push(subs.get_root_key_without_compacting(*rec_var));
-                        }
-                        occurs_union(subs, root_var, &new_seen, include_recursion_var, tags)?;
 
-                        short_circuit_help(
+                        short_circuit_help(subs, root_var, &new_seen, *ext_var)
-                            subs,
-                            root_var,
-                            &new_seen,
-                            *ext_var,
-                            include_recursion_var,
-                        )
                     }
                     EmptyRecord | EmptyTagUnion | Erroneous(_) => Ok(()),
                 }
@@ -3153,30 +3124,24 @@ fn occurs(
 
                 for var_index in args.into_iter() {
                     let var = subs[var_index];
-                    short_circuit_help(subs, root_var, &new_seen, var, include_recursion_var)?;
+                    short_circuit_help(subs, root_var, &new_seen, var)?;
                 }
 
                 Ok(())
             }
             LambdaSet(self::LambdaSet {
                 solved,
-                recursion_var,
+                recursion_var: _,
                 unspecialized: _,
                 ambient_function: _,
             }) => {
                 let mut new_seen = seen.to_owned();
                 new_seen.push(root_var);
 
-                if include_recursion_var {
-                    if let Some(v) = recursion_var.into_variable() {
-                        new_seen.push(subs.get_root_key_without_compacting(v));
-                    }
-                }
-
                 // unspecialized lambda vars excluded because they are not explicitly part of the
                 // type (they only matter after being resolved).
 
-                occurs_union(subs, root_var, &new_seen, include_recursion_var, solved)
+                occurs_union(subs, root_var, &new_seen, solved)
             }
             RangedNumber(_range_vars) => Ok(()),
         }
@@ -3188,14 +3153,13 @@ fn occurs_union<L: Label>(
     subs: &Subs,
     root_var: Variable,
     seen: &[Variable],
-    include_recursion_var: bool,
     tags: &UnionLabels<L>,
 ) -> Result<(), (Variable, Vec<Variable>)> {
     for slice_index in tags.variables() {
         let slice = subs[slice_index];
         for var_index in slice {
             let var = subs[var_index];
-            short_circuit_help(subs, root_var, seen, var, include_recursion_var)?;
+            short_circuit_help(subs, root_var, seen, var)?;
         }
     }
     Ok(())
@@ -3207,13 +3171,12 @@ fn short_circuit<'a, T>(
     root_key: Variable,
     seen: &[Variable],
     iter: T,
-    include_recursion_var: bool,
 ) -> Result<(), (Variable, Vec<Variable>)>
 where
     T: Iterator<Item = &'a Variable>,
 {
     for var in iter {
-        short_circuit_help(subs, root_key, seen, *var, include_recursion_var)?;
+        short_circuit_help(subs, root_key, seen, *var)?;
     }
 
     Ok(())
@@ -3225,9 +3188,8 @@ fn short_circuit_help(
     root_key: Variable,
     seen: &[Variable],
     var: Variable,
-    include_recursion_var: bool,
 ) -> Result<(), (Variable, Vec<Variable>)> {
-    if let Err((v, mut vec)) = occurs(subs, seen, var, include_recursion_var) {
+    if let Err((v, mut vec)) = occurs(subs, seen, var) {
         vec.push(root_key);
         return Err((v, vec));
     }

crates/compiler/unify/src/unify.rs

@@ -890,15 +890,7 @@ fn unify_structure<M: MetaCollector>(
         RecursionVar { structure, .. } => match flat_type {
             FlatType::TagUnion(_, _) => {
                 // unify the structure with this unrecursive tag union
-                let mut outcome = unify_pool(subs, pool, ctx.first, *structure, ctx.mode);
+                unify_pool(subs, pool, ctx.first, *structure, ctx.mode)
-
-                if outcome.mismatches.is_empty() {
-                    outcome.union(fix_tag_union_recursion_variable(
-                        subs, ctx, ctx.first, other,
-                    ));
-                }
-
-                outcome
             }
             FlatType::RecursiveTagUnion(rec, _, _) => {
                 debug_assert!(is_recursion_var(subs, *rec));
@@ -907,15 +899,7 @@ fn unify_structure<M: MetaCollector>(
             }
             FlatType::FunctionOrTagUnion(_, _, _) => {
                 // unify the structure with this unrecursive tag union
-                let mut outcome = unify_pool(subs, pool, ctx.first, *structure, ctx.mode);
+                unify_pool(subs, pool, ctx.first, *structure, ctx.mode)
-
-                if outcome.mismatches.is_empty() {
-                    outcome.union(fix_tag_union_recursion_variable(
-                        subs, ctx, ctx.first, other,
-                    ));
-                }
-
-                outcome
             }
             // Only tag unions can be recursive; everything else is an error.
             _ => mismatch!(
@@ -1376,57 +1360,6 @@ fn unify_lambda_set_help<M: MetaCollector>(
     whole_outcome
 }
 
-/// Ensures that a non-recursive tag union, when unified with a recursion var to become a recursive
-/// tag union, properly contains a recursion variable that recurses on itself.
-//
-// When might this not be the case? For example, in the code
-//
-//   Indirect : [Indirect ConsList]
-//
-//   ConsList : [Nil, Cons Indirect]
-//
-//   l : ConsList
-//   l = Cons (Indirect (Cons (Indirect Nil)))
-//   #   ^^^^^^^^^^^^^^^~~~~~~~~~~~~~~~~~~~~~^ region-a
-//   #                  ~~~~~~~~~~~~~~~~~~~~~  region-b
-//   l
-//
-// Suppose `ConsList` has the expanded type `[Nil, Cons [Indirect <rec>]] as <rec>`.
-// After unifying the tag application annotated "region-b" with the recursion variable `<rec>`,
-// the tentative total-type of the application annotated "region-a" would be
-// `<v> = [Nil, Cons [Indirect <v>]] as <rec>`. That is, the type of the recursive tag union
-// would be inlined at the site "v", rather than passing through the correct recursion variable
-// "rec" first.
-//
-// This is not incorrect from a type perspective, but causes problems later on for e.g. layout
-// determination, which expects recursion variables to be placed correctly. Attempting to detect
-// this during layout generation does not work so well because it may be that there *are* recursive
-// tag unions that should be inlined, and not pass through recursion variables. So instead, try to
-// resolve these cases here.
-//
-// See tests labeled "issue_2810" for more examples.
-fn fix_tag_union_recursion_variable<M: MetaCollector>(
-    subs: &mut Subs,
-    ctx: &Context,
-    tag_union_promoted_to_recursive: Variable,
-    recursion_var: &Content,
-) -> Outcome<M> {
-    debug_assert!(matches!(
-        subs.get_content_without_compacting(tag_union_promoted_to_recursive),
-        Structure(FlatType::RecursiveTagUnion(..))
-    ));
-
-    let has_recursing_recursive_variable = subs
-        .occurs_including_recursion_vars(tag_union_promoted_to_recursive)
-        .is_err();
-
-    if !has_recursing_recursive_variable {
-        merge(subs, ctx, *recursion_var)
-    } else {
-        Outcome::default()
-    }
-}
-
 fn unify_record<M: MetaCollector>(
     subs: &mut Subs,
     pool: &mut Pool,
@@ -2097,14 +2030,47 @@ fn unify_shared_tags_new<M: MetaCollector>(
 
             outcome.union(unify_pool(subs, pool, actual, expected, ctx.mode));
 
-            // clearly, this is very suspicious: these variables have just been unified. And yet,
+            if outcome.mismatches.is_empty() {
-            // not doing this leads to stack overflows
+                // If one of the variables is a recursion var, keep that one, so that we avoid inlining
-            if let Rec::Right(_) = recursion_var {
+                // a recursive tag union type content where we should have a recursion var instead.
-                if outcome.mismatches.is_empty() {
+                //
-                    matching_vars.push(expected);
+                // When might this happen? For example, in the code
-                }
+                //
-            } else if outcome.mismatches.is_empty() {
+                //   Indirect : [Indirect ConsList]
-                matching_vars.push(actual);
+                //
+                //   ConsList : [Nil, Cons Indirect]
+                //
+                //   l : ConsList
+                //   l = Cons (Indirect (Cons (Indirect Nil)))
+                //   #   ^^^^^^^^^^^^^^^~~~~~~~~~~~~~~~~~~~~~^ region-a
+                //   #                  ~~~~~~~~~~~~~~~~~~~~~  region-b
+                //   l
+                //
+                // Suppose `ConsList` has the expanded type `[Nil, Cons [Indirect <rec>]] as <rec>`.
+                // After unifying the tag application annotated "region-b" with the recursion variable `<rec>`,
+                // we might have that e.g. `actual` is `<rec>` and `expected` is `[Cons (Indirect ...)]`.
+                //
+                // Now, we need to be careful to set the type we choose to represent the merged type
+                // here to be `<rec>`, not the tag union content of `expected`! Otherwise, we will
+                // have lost a recursion variable in the recursive tag union.
+                //
+                // This would not be incorrect from a type perspective, but causes problems later on for e.g.
+                // layout generation, which expects recursion variables to be placed correctly. Attempting to detect
+                // this during layout generation does not work so well because it may be that there *are* recursive
+                // tag unions that should be inlined, and not pass through recursion variables. So instead, resolve
+                // these cases here.
+                //
+                // See tests labeled "issue_2810" for more examples.
+                let merged_var = match (
+                    (actual, subs.get_content_unchecked(actual)),
+                    (expected, subs.get_content_unchecked(expected)),
+                ) {
+                    ((var, Content::RecursionVar { .. }), _)
+                    | (_, (var, Content::RecursionVar { .. })) => var,
+                    _ => actual,
+                };
+
+                matching_vars.push(merged_var);
             }
 
             total_outcome.union(outcome);