diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index 1b3d82d9d2..ac26c074c7 100644 --- a/crates/cli/src/lib.rs +++ b/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 { + 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 { } } - if failed > 0 { - println!("test result: failed. {passed} passed; {failed} failed;"); - Ok(1) + let total_time = start_time.elapsed(); + + 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;"); - Ok(0) + let failed_color = if failed == 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."), ]) }; diff --git a/crates/compiler/alias_analysis/src/lib.rs b/crates/compiler/alias_analysis/src/lib.rs index 53c1649864..bf1d42ab95 100644 --- a/crates/compiler/alias_analysis/src/lib.rs +++ b/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, ); diff --git a/crates/compiler/builtins/bitcode/src/str.zig b/crates/compiler/builtins/bitcode/src/str.zig index b785094871..a8c2899bf8 100644 --- a/crates/compiler/builtins/bitcode/src/str.zig +++ b/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: - - 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); + while (index != new_len) : (index += 1) { + bytes_ptr[index] = src[index]; + } } + + 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: - - 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); + while (index != new_len) : (index += 1) { + bytes_ptr[index] = src[index]; + } } + + 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(); diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index 4ebb95b2c3..82184696a6 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -4917,8 +4917,22 @@ pub fn build_proc<'a, 'ctx, 'env>( ) } - RawFunctionLayout::ZeroArgumentThunk(_) => { - // do nothing + RawFunctionLayout::ZeroArgumentThunk(result) => { + // 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, + ); } } } diff --git a/crates/compiler/load_internal/src/file.rs b/crates/compiler/load_internal/src/file.rs index 11b75f8a20..cba581fe32 100644 --- a/crates/compiler/load_internal/src/file.rs +++ b/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 diff --git a/crates/compiler/mono/src/ir.rs b/crates/compiler/mono/src/ir.rs index 6ad3c450d3..b3583064cc 100644 --- a/crates/compiler/mono/src/ir.rs +++ b/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)) => { - let top_level = ProcLayout::from_raw( + Ok((proc, _ignore_layout)) => { + // 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!( - outside_layout, top_level, - "different raw layouts for {:?}", - proc.name - ); - - if self.is_module_thunk(proc.name.name()) { - debug_assert!(top_level.arguments.is_empty()); - } + let proper_layout = ProcLayout { + arguments, + result: proc.ret_layout, + captures_niche: proc.name.captures_niche(), + }; + // 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)) => { - // TODO thiscode is duplicated elsewhere - let top_level = ProcLayout::from_raw(env.arena, layout, proc.name.captures_niche()); + Ok((proc, _layout)) => { + // TODO this code is duplicated elsewhere + // 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(_) => { - unreachable!("so far"); + RawFunctionLayout::ZeroArgumentThunk(result) => { + 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, + ), + ); } } } diff --git a/crates/compiler/parse/src/blankspace.rs b/crates/compiler/parse/src/blankspace.rs index 577457baad..3e56027cd6 100644 --- a/crates/compiler/parse/src/blankspace.rs +++ b/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); - - if spaces.multiline { - spaces.state.indent_column = spaces.state.column(); - } + let spaces = eat_spaces(state, comments_and_newlines); 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, }; } diff --git a/crates/compiler/parse/src/expr.rs b/crates/compiler/parse/src/expr.rs index 2ef870a964..c7835cf8d8 100644 --- a/crates/compiler/parse/src/expr.rs +++ b/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)?; diff --git a/crates/compiler/parse/src/state.rs b/crates/compiler/parse/src/state.rs index 4e936d7589..617909bf7d 100644 --- a/crates/compiler/parse/src/state.rs +++ b/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}}") } } diff --git a/crates/compiler/parse/tests/snapshots/pass/when_in_assignment.expr.result-ast b/crates/compiler/parse/tests/snapshots/pass/when_in_assignment.expr.result-ast new file mode 100644 index 0000000000..d1f7841add --- /dev/null +++ b/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, + ], + ), +) diff --git a/crates/compiler/parse/tests/snapshots/pass/when_in_assignment.expr.roc b/crates/compiler/parse/tests/snapshots/pass/when_in_assignment.expr.roc new file mode 100644 index 0000000000..ffdb01e237 --- /dev/null +++ b/crates/compiler/parse/tests/snapshots/pass/when_in_assignment.expr.roc @@ -0,0 +1,3 @@ +x = when n is + 0 -> 0 +42 \ No newline at end of file diff --git a/crates/compiler/parse/tests/snapshots/pass/when_in_function.expr.result-ast b/crates/compiler/parse/tests/snapshots/pass/when_in_function.expr.result-ast new file mode 100644 index 0000000000..0b2dd8c5b1 --- /dev/null +++ b/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, + ], + ), +) diff --git a/crates/compiler/parse/tests/snapshots/pass/when_in_function.expr.roc b/crates/compiler/parse/tests/snapshots/pass/when_in_function.expr.roc new file mode 100644 index 0000000000..3d3a0a6bce --- /dev/null +++ b/crates/compiler/parse/tests/snapshots/pass/when_in_function.expr.roc @@ -0,0 +1,3 @@ +func = \x -> when n is + 0 -> 0 +42 \ No newline at end of file diff --git a/crates/compiler/parse/tests/test_parse.rs b/crates/compiler/parse/tests/test_parse.rs index c55773c28a..5b78ca1756 100644 --- a/crates/compiler/parse/tests/test_parse.rs +++ b/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, diff --git a/crates/compiler/solve/src/solve.rs b/crates/compiler/solve/src/solve.rs index 4ab0e6d928..dbea87d931 100644 --- a/crates/compiler/solve/src/solve.rs +++ b/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, diff --git a/crates/compiler/types/src/subs.rs b/crates/compiler/types/src/subs.rs index fc2715758f..5a527251c5 100644 --- a/crates/compiler/types/src/subs.rs +++ b/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 - /// need to check for recursion var occurrence. + /// already priced in and expected to occur. pub fn occurs(&self, var: Variable) -> Result<(), (Variable, Vec)> { - occurs(self, &[], var, false) - } - - /// Like [Subs::occurs], but also errors when recursion vars occur. - pub fn occurs_including_recursion_vars( - &self, - var: Variable, - ) -> Result<(), (Variable, Vec)> { - occurs(self, &[], var, true) + occurs(self, &[], var) } 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)> { use self::Content::*; use self::FlatType::*; @@ -3096,53 +3086,34 @@ fn occurs( new_seen.push(root_var); match flat_type { - Apply(_, args) => short_circuit( - subs, - root_var, - &new_seen, - subs.get_subs_slice(*args).iter(), - include_recursion_var, - ), + Apply(_, args) => { + short_circuit(subs, root_var, &new_seen, subs.get_subs_slice(*args).iter()) + } 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( - subs, - root_var, - &new_seen, - *ext_var, - include_recursion_var, - ) + short_circuit_help(subs, root_var, &new_seen, *ext_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) => { - if include_recursion_var { - new_seen.push(subs.get_root_key_without_compacting(*rec_var)); - } - occurs_union(subs, root_var, &new_seen, include_recursion_var, tags)?; + RecursiveTagUnion(_, tags, ext_var) => { + occurs_union(subs, root_var, &new_seen, tags)?; - short_circuit_help( - subs, - root_var, - &new_seen, - *ext_var, - include_recursion_var, - ) + short_circuit_help(subs, root_var, &new_seen, *ext_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( subs: &Subs, root_var: Variable, seen: &[Variable], - include_recursion_var: bool, tags: &UnionLabels, ) -> Result<(), (Variable, Vec)> { 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)> where T: Iterator, { 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)> { - 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)); } diff --git a/crates/compiler/unify/src/unify.rs b/crates/compiler/unify/src/unify.rs index ecdb540f8d..f186398413 100644 --- a/crates/compiler/unify/src/unify.rs +++ b/crates/compiler/unify/src/unify.rs @@ -890,15 +890,7 @@ fn unify_structure( 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); - - if outcome.mismatches.is_empty() { - outcome.union(fix_tag_union_recursion_variable( - subs, ctx, ctx.first, other, - )); - } - - outcome + unify_pool(subs, pool, ctx.first, *structure, ctx.mode) } FlatType::RecursiveTagUnion(rec, _, _) => { debug_assert!(is_recursion_var(subs, *rec)); @@ -907,15 +899,7 @@ fn unify_structure( } FlatType::FunctionOrTagUnion(_, _, _) => { // unify the structure with this unrecursive tag union - let mut outcome = 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 + unify_pool(subs, pool, ctx.first, *structure, ctx.mode) } // Only tag unions can be recursive; everything else is an error. _ => mismatch!( @@ -1376,57 +1360,6 @@ fn unify_lambda_set_help( 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 ]] as `. -// After unifying the tag application annotated "region-b" with the recursion variable ``, -// the tentative total-type of the application annotated "region-a" would be -// ` = [Nil, Cons [Indirect ]] as `. 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( - subs: &mut Subs, - ctx: &Context, - tag_union_promoted_to_recursive: Variable, - recursion_var: &Content, -) -> Outcome { - 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( subs: &mut Subs, pool: &mut Pool, @@ -2097,14 +2030,47 @@ fn unify_shared_tags_new( outcome.union(unify_pool(subs, pool, actual, expected, ctx.mode)); - // clearly, this is very suspicious: these variables have just been unified. And yet, - // not doing this leads to stack overflows - if let Rec::Right(_) = recursion_var { - if outcome.mismatches.is_empty() { - matching_vars.push(expected); - } - } else if outcome.mismatches.is_empty() { - matching_vars.push(actual); + if outcome.mismatches.is_empty() { + // If one of the variables is a recursion var, keep that one, so that we avoid inlining + // a recursive tag union type content where we should have a recursion var instead. + // + // When might this happen? 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 ]] as `. + // After unifying the tag application annotated "region-b" with the recursion variable ``, + // we might have that e.g. `actual` is `` 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 ``, 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);