From ecba687243eaeb6b403188c2f38dd7ec637252e7 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 22 Aug 2021 16:29:57 +0200 Subject: [PATCH 1/5] Unify variables directly when possible instead of going through a solved type --- compiler/mono/src/ir.rs | 140 ++++++++++++++++++------ compiler/mono/src/layout.rs | 4 +- compiler/test_gen/src/gen_primitives.rs | 46 ++++++-- 3 files changed, 141 insertions(+), 49 deletions(-) diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index a62c24cb1f..7529792fc1 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -694,10 +694,8 @@ impl<'a> Procs<'a> { layout: ProcLayout<'a>, layout_cache: &mut LayoutCache<'a>, ) { - let tuple = (name, layout); - // If we've already specialized this one, no further work is needed. - if self.specialized.contains_key(&tuple) { + if self.specialized.contains_key(&(name, layout)) { return; } @@ -707,15 +705,12 @@ impl<'a> Procs<'a> { return; } - // We're done with that tuple, so move layout back out to avoid cloning it. - let (name, layout) = tuple; - - let pending = PendingSpecialization::from_var(env.arena, env.subs, fn_var); - // This should only be called when pending_specializations is Some. // Otherwise, it's being called in the wrong pass! match &mut self.pending_specializations { Some(pending_specializations) => { + let pending = PendingSpecialization::from_var(env.arena, env.subs, fn_var); + // register the pending specialization, so this gets code genned later if self.module_thunks.contains(&name) { debug_assert!(layout.arguments.is_empty()); @@ -736,7 +731,26 @@ impl<'a> Procs<'a> { // (We had a bug around this before this system existed!) self.specialized.insert((symbol, layout), InProgress); - match specialize(env, self, symbol, layout_cache, pending, partial_proc) { + // See https://github.com/rtfeldman/roc/issues/1600 + // + // The annotation variable is the generic/lifted/top-level annotation. + // It is connected to the variables of the function's body + // + // fn_var is the variable representing the type that we actually need for the + // function right here. + // + // For some reason, it matters that we unify with the original variable. Extracting + // that variable into a SolvedType and then introducing it again severs some + // connection that turns out to be important + match specialize_variable( + env, + self, + symbol, + layout_cache, + fn_var, + Default::default(), + partial_proc, + ) { 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 @@ -2448,13 +2462,57 @@ fn specialize_solved_type<'a>( host_exposed_aliases: BumpMap, partial_proc: PartialProc<'a>, ) -> Result, SpecializeFailure<'a>> { + specialize_variable_help( + env, + procs, + proc_name, + layout_cache, + |env| introduce_solved_type_to_subs(env, &solved_type), + host_exposed_aliases, + partial_proc, + ) +} + +fn specialize_variable<'a>( + env: &mut Env<'a, '_>, + procs: &mut Procs<'a>, + proc_name: Symbol, + layout_cache: &mut LayoutCache<'a>, + fn_var: Variable, + host_exposed_aliases: BumpMap, + partial_proc: PartialProc<'a>, +) -> Result, SpecializeFailure<'a>> { + specialize_variable_help( + env, + procs, + proc_name, + layout_cache, + |_| fn_var, + host_exposed_aliases, + partial_proc, + ) +} + +fn specialize_variable_help<'a, F>( + env: &mut Env<'a, '_>, + procs: &mut Procs<'a>, + proc_name: Symbol, + layout_cache: &mut LayoutCache<'a>, + fn_var_thunk: F, + host_exposed_aliases: BumpMap, + partial_proc: PartialProc<'a>, +) -> Result, SpecializeFailure<'a>> +where + F: FnOnce(&mut Env<'a, '_>) -> Variable, +{ // add the specializations that other modules require of us use roc_solve::solve::instantiate_rigids; let snapshot = env.subs.snapshot(); let cache_snapshot = layout_cache.snapshot(); - let fn_var = introduce_solved_type_to_subs(env, &solved_type); + // important: evaluate after the snapshot has been created! + let fn_var = fn_var_thunk(env); // for debugging only let raw = layout_cache @@ -2723,32 +2781,36 @@ pub fn with_hole<'a>( hole, ), - Num(var, num) => match num_argument_to_int_or_float(env.subs, env.ptr_bytes, var, false) { - IntOrFloat::SignedIntType(precision) => Stmt::Let( - assigned, - Expr::Literal(Literal::Int(num.into())), - Layout::Builtin(int_precision_to_builtin(precision)), - hole, - ), - IntOrFloat::UnsignedIntType(precision) => Stmt::Let( - assigned, - Expr::Literal(Literal::Int(num.into())), - Layout::Builtin(int_precision_to_builtin(precision)), - hole, - ), - IntOrFloat::BinaryFloatType(precision) => Stmt::Let( - assigned, - Expr::Literal(Literal::Float(num as f64)), - Layout::Builtin(float_precision_to_builtin(precision)), - hole, - ), - IntOrFloat::DecimalFloatType => Stmt::Let( - assigned, - Expr::Literal(Literal::Float(num as f64)), - Layout::Builtin(Builtin::Decimal), - hole, - ), - }, + Num(var, num) => { + // first figure out what kind of number this is + + match num_argument_to_int_or_float(env.subs, env.ptr_bytes, var, false) { + IntOrFloat::SignedIntType(precision) => Stmt::Let( + assigned, + Expr::Literal(Literal::Int(num.into())), + Layout::Builtin(int_precision_to_builtin(precision)), + hole, + ), + IntOrFloat::UnsignedIntType(precision) => Stmt::Let( + assigned, + Expr::Literal(Literal::Int(num.into())), + Layout::Builtin(int_precision_to_builtin(precision)), + hole, + ), + IntOrFloat::BinaryFloatType(precision) => Stmt::Let( + assigned, + Expr::Literal(Literal::Float(num as f64)), + Layout::Builtin(float_precision_to_builtin(precision)), + hole, + ), + IntOrFloat::DecimalFloatType => Stmt::Let( + assigned, + Expr::Literal(Literal::Float(num as f64)), + Layout::Builtin(Builtin::Decimal), + hole, + ), + } + } LetNonRec(def, cont, _) => { if let roc_can::pattern::Pattern::Identifier(symbol) = &def.loc_pattern.value { if let Closure { @@ -7865,6 +7927,12 @@ fn union_lambda_set_to_switch<'a>( assigned: Symbol, hole: &'a Stmt<'a>, ) -> Stmt<'a> { + // NOTE this can happen if there is a type error somewhere. Since the lambda set is empty, + // there is really nothing we can do here, so we just proceed with the hole itself and + // hope that the type error is communicated in a clear way elsewhere. + if lambda_set.is_empty() { + return hole.clone(); + } debug_assert!(!lambda_set.is_empty()); let join_point_id = JoinPointId(env.unique_symbol()); diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index c4e4dece95..e0a96e6a32 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -554,10 +554,10 @@ impl<'a> LambdaSet<'a> { } Ok(()) | Err((_, Content::FlexVar(_))) => { - // TODO hack for builting functions. + // this can happen when there is a type error somewhere Ok(LambdaSet { set: &[], - representation: arena.alloc(Layout::Struct(&[])), + representation: arena.alloc(Layout::Union(UnionLayout::NonRecursive(&[]))), }) } _ => panic!("called LambdaSet.from_var on invalid input"), diff --git a/compiler/test_gen/src/gen_primitives.rs b/compiler/test_gen/src/gen_primitives.rs index 149e083c93..a416a852a6 100644 --- a/compiler/test_gen/src/gen_primitives.rs +++ b/compiler/test_gen/src/gen_primitives.rs @@ -2683,26 +2683,50 @@ fn list_walk_until() { } #[test] -#[ignore] -fn int_literal_not_specialized() { +fn int_literal_not_specialized_with_annotation() { // see https://github.com/rtfeldman/roc/issues/1600 assert_evals_to!( indoc!( r#" app "test" provides [ main ] to "./platform" - - satisfy : (U8 -> Bool) -> Str - satisfy = \_ -> "foo" - - - main : I64 main = - p1 = (\u -> u == 97) + satisfy : (U8 -> Str) -> Str + satisfy = \_ -> "foo" - satisfyA = satisfy p1 + myEq : a, a -> Str + myEq = \_, _ -> "bar" - when satisfyA is + p1 : Num * -> Str + p1 = (\u -> myEq u 64) + + when satisfy p1 is + _ -> 32 + "# + ), + 32, + i64 + ); +} + +#[test] +fn int_literal_not_specialized_no_annotation() { + // see https://github.com/rtfeldman/roc/issues/1600 + assert_evals_to!( + indoc!( + r#" + app "test" provides [ main ] to "./platform" + + main = + satisfy : (U8 -> Str) -> Str + satisfy = \_ -> "foo" + + myEq : a, a -> Str + myEq = \_, _ -> "bar" + + p1 = (\u -> myEq u 64) + + when satisfy p1 is _ -> 32 "# ), From b6255748b32ee44ecdbd6928f4d36bb03cbb7d93 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 22 Aug 2021 21:58:07 +0200 Subject: [PATCH 2/5] emit an error message when a symbol is not defined i.e. don't panic in this case --- compiler/mono/src/ir.rs | 12 +++++++----- compiler/test_gen/src/gen_primitives.rs | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 7529792fc1..ab967700f9 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -7927,13 +7927,15 @@ fn union_lambda_set_to_switch<'a>( assigned: Symbol, hole: &'a Stmt<'a>, ) -> Stmt<'a> { - // NOTE this can happen if there is a type error somewhere. Since the lambda set is empty, - // there is really nothing we can do here, so we just proceed with the hole itself and - // hope that the type error is communicated in a clear way elsewhere. if lambda_set.is_empty() { - return hole.clone(); + // NOTE this can happen if there is a type error somewhere. Since the lambda set is empty, + // there is really nothing we can do here. We generate a runtime error here which allows + // code gen to proceed. We then assume that we hit another (more descriptive) error before + // hitting this one + + let msg = "a Lambda Set isempty. Most likely there is a type error in your program."; + return Stmt::RuntimeError(msg); } - debug_assert!(!lambda_set.is_empty()); let join_point_id = JoinPointId(env.unique_symbol()); diff --git a/compiler/test_gen/src/gen_primitives.rs b/compiler/test_gen/src/gen_primitives.rs index a416a852a6..fcd63e1f7e 100644 --- a/compiler/test_gen/src/gen_primitives.rs +++ b/compiler/test_gen/src/gen_primitives.rs @@ -2760,3 +2760,21 @@ fn unresolved_tvar_when_capture_is_unused() { i64 ); } + +#[test] +#[should_panic(expected = "Roc failed with message: ")] +fn value_not_exposed_hits_panic() { + assert_evals_to!( + indoc!( + r#" + app "test" provides [ main ] to "./platform" + + main : I64 + main = + Str.toInt 32 + "# + ), + 32, + i64 + ); +} From 9903e14cd3293ea9ade38f0b20a42f1b675dafc4 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 23 Aug 2021 21:41:13 +0200 Subject: [PATCH 3/5] update zig to compile on nightly --- compiler/builtins/bitcode/src/dict.zig | 6 ++--- compiler/builtins/bitcode/src/hash.zig | 4 +-- compiler/builtins/bitcode/src/str.zig | 35 ++++++++++++++----------- compiler/builtins/bitcode/src/utils.zig | 8 +++--- 4 files changed, 29 insertions(+), 24 deletions(-) diff --git a/compiler/builtins/bitcode/src/dict.zig b/compiler/builtins/bitcode/src/dict.zig index ea9ea768fc..71ccf4ba2f 100644 --- a/compiler/builtins/bitcode/src/dict.zig +++ b/compiler/builtins/bitcode/src/dict.zig @@ -9,12 +9,12 @@ const RocList = @import("list.zig").RocList; const INITIAL_SEED = 0xc70f6907; -const InPlace = packed enum(u8) { +const InPlace = enum(u8) { InPlace, Clone, }; -const Slot = packed enum(u8) { +const Slot = enum(u8) { Empty, Filled, PreviouslyFilled, @@ -63,7 +63,7 @@ fn capacityOfLevel(input: usize) usize { // alignment of the key and value. The tag furthermore indicates // which has the biggest aligmnent. If both are the same, we put // the key first -const Alignment = packed enum(u8) { +const Alignment = enum(u8) { Align16KeyFirst, Align16ValueFirst, Align8KeyFirst, diff --git a/compiler/builtins/bitcode/src/hash.zig b/compiler/builtins/bitcode/src/hash.zig index 10687a3231..6fb42129ad 100644 --- a/compiler/builtins/bitcode/src/hash.zig +++ b/compiler/builtins/bitcode/src/hash.zig @@ -180,8 +180,8 @@ pub const Wyhash = struct { } pub fn final(self: *Wyhash) u64 { - const seed = self.state.seed; - const rem_len = @intCast(u5, self.buf_len); + // const seed = self.state.seed; + // const rem_len = @intCast(u5, self.buf_len); const rem_key = self.buf[0..self.buf_len]; return self.state.final(rem_key); diff --git a/compiler/builtins/bitcode/src/str.zig b/compiler/builtins/bitcode/src/str.zig index 5f21c91da5..7a349569eb 100644 --- a/compiler/builtins/bitcode/src/str.zig +++ b/compiler/builtins/bitcode/src/str.zig @@ -1,5 +1,4 @@ const utils = @import("utils.zig"); -const roc_mem = @import("mem.zig"); const RocList = @import("list.zig").RocList; const std = @import("std"); const mem = std.mem; @@ -10,7 +9,7 @@ const expectEqual = testing.expectEqual; const expectError = testing.expectError; const expect = testing.expect; -const InPlace = packed enum(u8) { +const InPlace = enum(u8) { InPlace, Clone, }; @@ -52,7 +51,7 @@ pub const RocStr = extern struct { return result; } - pub fn initBig(in_place: InPlace, number_of_chars: u64) RocStr { + pub fn initBig(_: InPlace, number_of_chars: u64) RocStr { const first_element = utils.allocateWithRefcount(number_of_chars, @sizeOf(usize)); return RocStr{ @@ -222,7 +221,7 @@ pub const RocStr = extern struct { // null-terminated strings. Otherwise, we need to allocate and copy a new // null-terminated string, which has a much higher performance cost! fn isNullTerminated(self: RocStr) bool { - const len = self.len(); + const length = self.len(); const longest_small_str = @sizeOf(RocStr) - 1; // NOTE: We want to compare length here, *NOT* check for is_small_str! @@ -231,7 +230,7 @@ pub const RocStr = extern struct { // // (The other branch dereferences the bytes pointer, which is not safe // to do for the empty string.) - if (len <= longest_small_str) { + if (length <= longest_small_str) { // If we're a small string, then usually the next byte after the // end of the string will be zero. (Small strings set all their // unused bytes to 0, so that comparison for equality can be fast.) @@ -242,7 +241,7 @@ pub const RocStr = extern struct { // Also, if we are exactly a maximum-length small string, // then the next byte is off the end of the struct; // in that case, we are also not null-terminated! - return len != 0 and len != longest_small_str; + return length != 0 and length != longest_small_str; } else { // This is a big string, and it's not empty, so we can safely // dereference the pointer. @@ -253,8 +252,8 @@ pub const RocStr = extern struct { // // If we have excess capacity, then we can safely read the next // byte after the end of the string. Maybe it happens to be zero! - if (capacity_or_refcount > @intCast(isize, len)) { - return self.str_bytes[len] == 0; + if (capacity_or_refcount > @intCast(isize, length)) { + return self.str_bytes[length] == 0; } else { // This string was refcounted or immortal; we can't safely read // the next byte, so assume the string is not null-terminated. @@ -267,10 +266,10 @@ pub const RocStr = extern struct { // Returns 0 for refcounted stirngs and immortal strings. // Returns the stored capacity value for all other strings. pub fn capacity(self: RocStr) usize { - const len = self.len(); + const length = self.len(); const longest_small_str = @sizeOf(RocStr) - 1; - if (len <= longest_small_str) { + if (length <= longest_small_str) { // Note that although empty strings technically have the full // capacity of a small string available, they aren't marked as small // strings, so if you want to make use of that capacity, you need @@ -316,7 +315,14 @@ pub const RocStr = extern struct { pub fn asU8ptr(self: RocStr) [*]u8 { // Since this conditional would be prone to branch misprediction, // make sure it will compile to a cmov. - return if (self.isSmallStr() or self.isEmpty()) (&@bitCast([@sizeOf(RocStr)]u8, self)) else (@ptrCast([*]u8, self.str_bytes)); + // return if (self.isSmallStr() or self.isEmpty()) (&@bitCast([@sizeOf(RocStr)]u8, self)) else (@ptrCast([*]u8, self.str_bytes)); + if (self.isSmallStr() or self.isEmpty()) { + const as_int = @ptrToInt(&self); + const as_ptr = @intToPtr([*]u8, as_int); + return as_ptr; + } else { + return @ptrCast([*]u8, self.str_bytes); + } } // Given a pointer to some bytes, write the first (len) bytes of this @@ -408,7 +414,7 @@ pub fn strFromIntC(int: i64) callconv(.C) RocStr { fn strFromIntHelp(comptime T: type, int: T) RocStr { // determine maximum size for this T - comptime const size = comptime blk: { + const size = comptime blk: { // the string representation of the minimum i128 value uses at most 40 characters var buf: [40]u8 = undefined; var result = std.fmt.bufPrint(&buf, "{}", .{std.math.minInt(T)}) catch unreachable; @@ -785,8 +791,6 @@ pub fn countGraphemeClusters(string: RocStr) callconv(.C) usize { return count; } -fn rocStrFromLiteral(bytes_arr: *const []u8) RocStr {} - test "countGraphemeClusters: empty string" { const count = countGraphemeClusters(RocStr.empty()); try expectEqual(count, 0); @@ -867,7 +871,6 @@ pub fn startsWith(string: RocStr, prefix: RocStr) callconv(.C) bool { // Str.startsWithCodePt pub fn startsWithCodePt(string: RocStr, prefix: u32) callconv(.C) bool { - const bytes_len = string.len(); const bytes_ptr = string.asU8ptr(); var buffer: [4]u8 = undefined; @@ -1266,7 +1269,7 @@ pub fn numberOfNextCodepointBytes(ptr: [*]u8, len: usize, index: usize) Utf8Deco // Return types for validateUtf8Bytes // Values must be in alphabetical order. That is, lowest values are the first alphabetically. -pub const Utf8ByteProblem = packed enum(u8) { +pub const Utf8ByteProblem = enum(u8) { CodepointTooLarge = 0, EncodesSurrogateHalf = 1, ExpectedContinuation = 2, diff --git a/compiler/builtins/bitcode/src/utils.zig b/compiler/builtins/bitcode/src/utils.zig index f83fbfd227..d454d7ebfa 100644 --- a/compiler/builtins/bitcode/src/utils.zig +++ b/compiler/builtins/bitcode/src/utils.zig @@ -45,7 +45,7 @@ fn testing_roc_dealloc(c_ptr: *c_void, _: u32) callconv(.C) void { std.testing.allocator.destroy(ptr); } -fn testing_roc_panic(c_ptr: *c_void, _: u32) callconv(.C) void { +fn testing_roc_panic(_: *c_void, _: u32) callconv(.C) void { @panic("Roc paniced"); } @@ -69,7 +69,9 @@ pub fn panic(c_ptr: *c_void, alignment: u32) callconv(.C) void { // indirection because otherwise zig creats an alias to the panic function which our LLVM code // does not know how to deal with pub fn test_panic(c_ptr: *c_void, alignment: u32) callconv(.C) void { - const cstr = @ptrCast([*:0]u8, c_ptr); + _ = c_ptr; + _ = alignment; + // const cstr = @ptrCast([*:0]u8, c_ptr); // const stderr = std.io.getStdErr().writer(); // stderr.print("Roc panicked: {s}!\n", .{cstr}) catch unreachable; @@ -226,7 +228,7 @@ pub const RocResult = extern struct { } }; -pub const Ordering = packed enum(u8) { +pub const Ordering = enum(u8) { EQ = 0, GT = 1, LT = 2, From 9037e57e1423cd71ff2d9381219e76e971a3fa99 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 23 Aug 2021 21:45:12 +0200 Subject: [PATCH 4/5] fix zig usize/u64 issues --- compiler/builtins/bitcode/src/dict.zig | 4 ++-- compiler/builtins/bitcode/src/str.zig | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/builtins/bitcode/src/dict.zig b/compiler/builtins/bitcode/src/dict.zig index 71ccf4ba2f..2e5ff2a662 100644 --- a/compiler/builtins/bitcode/src/dict.zig +++ b/compiler/builtins/bitcode/src/dict.zig @@ -359,7 +359,7 @@ pub const RocDict = extern struct { // hash the key, and modulo by the maximum size // (so we get an in-bounds index) const hash = hash_fn(seed, key); - const index = capacityOfLevel(current_level - 1) + (hash % current_level_size); + const index = capacityOfLevel(current_level - 1) + @intCast(usize, (hash % current_level_size)); switch (self.getSlot(index, key_width, value_width)) { Slot.Empty, Slot.PreviouslyFilled => { @@ -426,7 +426,7 @@ pub fn dictInsert(input: RocDict, alignment: Alignment, key: Opaque, key_width: } const hash = hash_fn(seed, key); - const index = capacityOfLevel(current_level - 1) + (hash % current_level_size); + const index = capacityOfLevel(current_level - 1) + @intCast(usize, (hash % current_level_size)); assert(index < result.capacity()); switch (result.getSlot(index, key_width, value_width)) { diff --git a/compiler/builtins/bitcode/src/str.zig b/compiler/builtins/bitcode/src/str.zig index 7a349569eb..0daef1484d 100644 --- a/compiler/builtins/bitcode/src/str.zig +++ b/compiler/builtins/bitcode/src/str.zig @@ -51,7 +51,7 @@ pub const RocStr = extern struct { return result; } - pub fn initBig(_: InPlace, number_of_chars: u64) RocStr { + pub fn initBig(_: InPlace, number_of_chars: usize) RocStr { const first_element = utils.allocateWithRefcount(number_of_chars, @sizeOf(usize)); return RocStr{ From 7d874e5c1589f74c196f07758dab312800e5b8c1 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 23 Aug 2021 22:27:00 +0200 Subject: [PATCH 5/5] fix zig warning --- compiler/builtins/bitcode/src/utils.zig | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/builtins/bitcode/src/utils.zig b/compiler/builtins/bitcode/src/utils.zig index d454d7ebfa..8f9473c5ff 100644 --- a/compiler/builtins/bitcode/src/utils.zig +++ b/compiler/builtins/bitcode/src/utils.zig @@ -45,7 +45,10 @@ fn testing_roc_dealloc(c_ptr: *c_void, _: u32) callconv(.C) void { std.testing.allocator.destroy(ptr); } -fn testing_roc_panic(_: *c_void, _: u32) callconv(.C) void { +fn testing_roc_panic(c_ptr: *c_void, tag_id: u32) callconv(.C) void { + _ = c_ptr; + _ = tag_id; + @panic("Roc paniced"); }