diff --git a/src/build/builtin_compiler/main.zig b/src/build/builtin_compiler/main.zig index 59018a7ad2..d936351bdd 100644 --- a/src/build/builtin_compiler/main.zig +++ b/src/build/builtin_compiler/main.zig @@ -172,6 +172,9 @@ fn replaceStrIsEmptyWithLowLevel(env: *ModuleEnv) !std.ArrayList(CIR.Def.Idx) { if (env.common.findIdent("list_get_unsafe")) |list_get_unsafe_ident| { try low_level_map.put(list_get_unsafe_ident, .list_get_unsafe); } + if (env.common.findIdent("list_append_unsafe")) |list_append_unsafe_ident| { + try low_level_map.put(list_append_unsafe_ident, .list_append_unsafe); + } if (env.common.findIdent("Builtin.List.drop_at")) |list_drop_at_ident| { try low_level_map.put(list_drop_at_ident, .list_drop_at); } diff --git a/src/build/roc/Builtin.roc b/src/build/roc/Builtin.roc index 0c5cf096b7..9736a29981 100644 --- a/src/build/roc/Builtin.roc +++ b/src/build/roc/Builtin.roc @@ -90,10 +90,14 @@ Builtin :: [].{ } map : List(a), (a -> b) -> List(b) - map = |list, transform| - # Implement using fold + concat for now - # TODO: Optimize with in-place update when list is unique and element sizes match - List.fold(list, [], |acc, item| List.concat(acc, [transform(item)])) + map = |list, transform| { + # TODO: Optimize with in-place update when list is unique and element sizes match + var $new_list = List.with_capacity(list.len()) + for item in list { + $new_list = list_append_unsafe($new_list, transform(item)) + } + $new_list + } keep_if : List(a), (a -> Bool) -> List(a) keep_if = |list, predicate| @@ -289,6 +293,18 @@ Builtin :: [].{ Ok(_) => fallback } + map_ok : Try(a, err), (a -> b) -> Try(b, err) + map_ok = |try, transform| match try { + Err(err) => Err(err) + Ok(a) => Ok(transform(a)) + } + + map_err : Try(ok, a), (a -> b) -> Try(ok, b) + map_err = |try, transform| match try { + Err(a) => Err(transform(a)) + Ok(ok) => Ok(ok) + } + is_eq : Try(ok, err), Try(ok, err) -> Bool where [ ok.is_eq : ok, ok -> Bool, @@ -1091,6 +1107,9 @@ range_until = |var $current, end| { # Implemented by the compiler, does not perform bounds checks list_get_unsafe : List(item), U64 -> item +# Implemented by the compiler, does not perform bounds checks +list_append_unsafe : List(item), item -> List(item) + # Unsafe conversion functions - these return simple records instead of Try types # They are low-level operations that get replaced by the compiler # Note: success is U8 (0 = false, 1 = true) since Bool is not available at top level diff --git a/src/canonicalize/Expression.zig b/src/canonicalize/Expression.zig index ce3b2fed54..1499e501fe 100644 --- a/src/canonicalize/Expression.zig +++ b/src/canonicalize/Expression.zig @@ -509,6 +509,7 @@ pub const Expr = union(enum) { list_len, list_is_empty, list_get_unsafe, + list_append_unsafe, list_concat, list_with_capacity, list_sort_with, @@ -882,6 +883,7 @@ pub const Expr = union(enum) { .list_concat => &.{ .consume, .consume }, .list_with_capacity => &.{.borrow}, // capacity is value type .list_sort_with => &.{.consume}, + .list_append_unsafe => &.{.consume}, .list_append => &.{ .consume, .borrow }, // list consumed, element borrowed .list_drop_at => &.{ .consume, .borrow }, // list consumed, index is value type .list_sublist => &.{ .consume, .borrow }, // list consumed, {start, len} record is value type diff --git a/src/eval/interpreter.zig b/src/eval/interpreter.zig index 028a88b7ed..e20041cfee 100644 --- a/src/eval/interpreter.zig +++ b/src/eval/interpreter.zig @@ -3056,6 +3056,224 @@ pub const Interpreter = struct { out.is_initialized = true; return out; }, + .list_append_unsafe => { + // List.append: List(a), a -> List(a) + std.debug.assert(args.len == 2); // low-level .list_append expects 2 arguments + + const roc_list_arg = args[0]; + const elt_arg = args[1]; + + std.debug.assert(roc_list_arg.ptr != null); // low-level .list_append expects non-null list pointer + std.debug.assert(elt_arg.ptr != null); // low-level .list_append expects non-null 2nd argument + + // Extract element layout from List(a) + std.debug.assert(roc_list_arg.layout.tag == .list or roc_list_arg.layout.tag == .list_of_zst); // low-level .list_append expects list layout + + // Handle ZST lists: appending to a list of ZSTs doesn't actually store anything + // The list header tracks the length but elements are zero-sized. + if (roc_list_arg.layout.tag == .list_of_zst) { + const roc_list: *const builtins.list.RocList = @ptrCast(@alignCast(roc_list_arg.ptr.?)); + + // If the element is also ZST, just bump the length + if (elt_arg.layout.tag == .zst) { + var result_list = roc_list.*; + result_list.length += 1; + var out = try self.pushRaw(roc_list_arg.layout, 0, roc_list_arg.rt_var); + out.is_initialized = false; + const result_ptr: *builtins.list.RocList = @ptrCast(@alignCast(out.ptr.?)); + result_ptr.* = result_list; + out.is_initialized = true; + return out; + } + + // The list was inferred as list_of_zst (e.g., from List.with_capacity with unknown element type) + // but we're appending a non-ZST element. We need to "upgrade" to a proper list layout. + // The original list_of_zst should be empty (or contain only ZST elements that we can discard). + // Create a new list with the element's layout and append to it. + const elem_layout = elt_arg.layout; + const elem_layout_idx = try self.runtime_layout_store.insertLayout(elem_layout); + var new_list_layout = roc_list_arg.layout; + new_list_layout.tag = .list; + new_list_layout.data = .{ .list = elem_layout_idx }; + + // Create new empty list with correct element layout + const non_null_bytes: [*]u8 = @ptrCast(elt_arg.ptr.?); + const append_elt: builtins.list.Opaque = non_null_bytes; + const elem_size: u32 = self.runtime_layout_store.layoutSize(elem_layout); + const elem_alignment = elem_layout.alignment(self.runtime_layout_store.targetUsize()).toByteUnits(); + const elem_alignment_u32: u32 = @intCast(elem_alignment); + + // Determine if elements contain refcounted data + const elements_refcounted = layoutContainsRefcounted(elem_layout, &self.runtime_layout_store); + + // Set up context for refcount callbacks + const elem_rt_var = try self.runtime_types.fresh(); + var refcount_context = RefcountContext{ + .layout_store = &self.runtime_layout_store, + .elem_layout = elem_layout, + .elem_rt_var = elem_rt_var, + .roc_ops = roc_ops, + }; + + const copy_fn: builtins.list.CopyFallbackFn = copy: switch (elem_layout.tag) { + .scalar => { + switch (elem_layout.data.scalar.tag) { + .str => break :copy &builtins.list.copy_str, + .int => { + switch (elem_layout.data.scalar.data.int) { + .u8 => break :copy &builtins.list.copy_u8, + .u16 => break :copy &builtins.list.copy_u16, + .u32 => break :copy &builtins.list.copy_u32, + .u64 => break :copy &builtins.list.copy_u64, + .u128 => break :copy &builtins.list.copy_u128, + .i8 => break :copy &builtins.list.copy_i8, + .i16 => break :copy &builtins.list.copy_i16, + .i32 => break :copy &builtins.list.copy_i32, + .i64 => break :copy &builtins.list.copy_i64, + .i128 => break :copy &builtins.list.copy_i128, + } + }, + else => break :copy &builtins.list.copy_fallback, + } + }, + .box => break :copy &builtins.list.copy_box, + .box_of_zst => break :copy &builtins.list.copy_box_zst, + .list => break :copy &builtins.list.copy_list, + .list_of_zst => break :copy &builtins.list.copy_list_zst, + else => break :copy &builtins.list.copy_fallback, + }; + + // Increment refcount of the element being appended + if (elements_refcounted) { + elt_arg.incref(&self.runtime_layout_store, roc_ops); + } + + // Append to an empty list (ignoring the old list_of_zst content) + const empty_list = builtins.list.RocList.empty(); + const result_list = builtins.list.listAppend( + empty_list, + elem_alignment_u32, + append_elt, + elem_size, + elements_refcounted, + if (elements_refcounted) @ptrCast(&refcount_context) else null, + if (elements_refcounted) &listElementInc else &builtins.list.rcNone, + builtins.utils.UpdateMode.Immutable, + copy_fn, + roc_ops, + ); + + // Decref the original list_of_zst (it may have capacity allocated) + roc_list_arg.decref(&self.runtime_layout_store, roc_ops); + + // Push result with upgraded layout + var out = try self.pushRaw(new_list_layout, 0, roc_list_arg.rt_var); + out.is_initialized = false; + const result_ptr: *builtins.list.RocList = @ptrCast(@alignCast(out.ptr.?)); + result_ptr.* = result_list; + out.is_initialized = true; + return out; + } + + // Format arguments into proper types + const roc_list: *const builtins.list.RocList = @ptrCast(@alignCast(roc_list_arg.ptr.?)); + const non_null_bytes: [*]u8 = @ptrCast(elt_arg.ptr.?); + const append_elt: builtins.list.Opaque = non_null_bytes; + + // Get element layout from the list's stored layout + const stored_elem_layout_idx = roc_list_arg.layout.data.list; + const stored_elem_layout = self.runtime_layout_store.getLayout(stored_elem_layout_idx); + + // Check if the stored element layout needs to be upgraded. + // This handles the case where the list was created with an unknown element type + // (e.g., List(List(?)) where the inner list type was inferred as list_of_zst), + // but we're now appending an element with a more specific layout. + // We should use the element's actual layout to ensure correct behavior. + const needs_element_layout_upgrade = stored_elem_layout.tag == .list_of_zst and + elt_arg.layout.tag != .zst and elt_arg.layout.tag != .list_of_zst; + + const elem_layout: Layout = if (needs_element_layout_upgrade) elt_arg.layout else stored_elem_layout; + const elem_layout_idx = if (needs_element_layout_upgrade) + try self.runtime_layout_store.insertLayout(elt_arg.layout) + else + stored_elem_layout_idx; + + const elem_size: u32 = self.runtime_layout_store.layoutSize(elem_layout); + const elem_alignment = elem_layout.alignment(self.runtime_layout_store.targetUsize()).toByteUnits(); + const elem_alignment_u32: u32 = @intCast(elem_alignment); + + // Determine if elements contain refcounted data (directly or transitively). + // This is more comprehensive than isRefcounted() - it also catches tuples/records + // containing strings, which need proper refcounting (fixes issue #8650). + const elements_refcounted = layoutContainsRefcounted(elem_layout, &self.runtime_layout_store); + + // Determine if list can be mutated in place + const update_mode = if (roc_list.isUnique(roc_ops)) builtins.utils.UpdateMode.InPlace else builtins.utils.UpdateMode.Immutable; + + // Set up context for refcount callbacks + const elem_rt_var = try self.runtime_types.fresh(); + var refcount_context = RefcountContext{ + .layout_store = &self.runtime_layout_store, + .elem_layout = elem_layout, + .elem_rt_var = elem_rt_var, + .roc_ops = roc_ops, + }; + + const copy_fn: builtins.list.CopyFallbackFn = copy: switch (elem_layout.tag) { + .scalar => { + switch (elem_layout.data.scalar.tag) { + .str => break :copy &builtins.list.copy_str, + .int => { + switch (elem_layout.data.scalar.data.int) { + .u8 => break :copy &builtins.list.copy_u8, + .u16 => break :copy &builtins.list.copy_u16, + .u32 => break :copy &builtins.list.copy_u32, + .u64 => break :copy &builtins.list.copy_u64, + .u128 => break :copy &builtins.list.copy_u128, + .i8 => break :copy &builtins.list.copy_i8, + .i16 => break :copy &builtins.list.copy_i16, + .i32 => break :copy &builtins.list.copy_i32, + .i64 => break :copy &builtins.list.copy_i64, + .i128 => break :copy &builtins.list.copy_i128, + } + }, + else => break :copy &builtins.list.copy_fallback, + } + }, + .box => break :copy &builtins.list.copy_box, + .box_of_zst => break :copy &builtins.list.copy_box_zst, + .list => break :copy &builtins.list.copy_list, + .list_of_zst => break :copy &builtins.list.copy_list_zst, + else => break :copy &builtins.list.copy_fallback, + }; + + // Increment refcount of the element being appended. + // The element is copied into the list, creating a second reference, + // so we need to increment its refcount before the copy. + // Without this, when the original element is freed, the list would + // hold a dangling reference (use-after-free bug). + if (elements_refcounted) { + elt_arg.incref(&self.runtime_layout_store, roc_ops); + } + + const result_list = builtins.list.listAppend(roc_list.*, elem_alignment_u32, append_elt, elem_size, elements_refcounted, if (elements_refcounted) @ptrCast(&refcount_context) else null, if (elements_refcounted) &listElementInc else &builtins.list.rcNone, update_mode, copy_fn, roc_ops); + + // Allocate space for the result list + // If we upgraded the element layout, create a new list layout with the upgraded element + const result_layout: Layout = if (needs_element_layout_upgrade) + Layout{ .tag = .list, .data = .{ .list = elem_layout_idx } } + else + roc_list_arg.layout; // Same layout as input + var out = try self.pushRaw(result_layout, 0, roc_list_arg.rt_var); + out.is_initialized = false; + + // Copy the result list structure to the output + const result_ptr: *builtins.list.RocList = @ptrCast(@alignCast(out.ptr.?)); + result_ptr.* = result_list; + + out.is_initialized = true; + return out; + }, .list_drop_at => { // List.drop_at : List(a), U64 -> List(a) std.debug.assert(args.len == 2); // low-level .list_drop_at expects 2 argument diff --git a/test/snapshots/list_map.md b/test/snapshots/list_map.md new file mode 100644 index 0000000000..c7b0fdf69d --- /dev/null +++ b/test/snapshots/list_map.md @@ -0,0 +1,13 @@ +# META +~~~ini +description=List.map +type=repl +~~~ +# SOURCE +~~~roc +» List.map([2, 4, 6], |val| val * 2) +~~~ +# OUTPUT +[4, 8, 12] +# PROBLEMS +NIL diff --git a/test/snapshots/plume_package/Color.md b/test/snapshots/plume_package/Color.md index 05dcaef9f1..309df3d382 100644 --- a/test/snapshots/plume_package/Color.md +++ b/test/snapshots/plume_package/Color.md @@ -98,7 +98,6 @@ MISSING METHOD - Color.md:40:23:40:43 MISSING METHOD - Color.md:62:12:62:26 MISSING METHOD - Color.md:56:26:56:32 MISSING METHOD - Color.md:57:32:57:38 -MISSING METHOD - Color.md:58:23:58:29 # PROBLEMS **MODULE HEADER DEPRECATED** The `module` header is deprecated. @@ -354,20 +353,6 @@ The value's type, which does not have a method named **to_str**, is: **Hint:** For this to work, the type would need to have a method named **to_str** associated with it in the type's declaration. -**MISSING METHOD** -This **map_ok** method is being called on a value whose type doesn't have that method: -**Color.md:58:23:58:29:** -```roc -expect hex("#ff00ff").map_ok(to_str) == Ok("#ff00ff") -``` - ^^^^^^ - -The value's type, which does not have a method named **map_ok**, is: - - _Try(Color, [InvalidHex(Str)])_ - -**Hint:** For this to work, the type would need to have a method named **map_ok** associated with it in the type's declaration. - # TOKENS ~~~zig KwModule,OpenSquare, diff --git a/test/snapshots/try_map_err.md b/test/snapshots/try_map_err.md new file mode 100644 index 0000000000..c5e306be0d --- /dev/null +++ b/test/snapshots/try_map_err.md @@ -0,0 +1,16 @@ +# META +~~~ini +description=Try.map_err with Ok and Err variants +type=repl +~~~ +# SOURCE +~~~roc +» Try.map_err(Try.Err(50), |err_code| err_code + 1) +» Try.map_err(Try.Ok("hello"), |_| "world") +~~~ +# OUTPUT +Err(51) +--- +Ok("hello") +# PROBLEMS +NIL diff --git a/test/snapshots/try_map_ok.md b/test/snapshots/try_map_ok.md new file mode 100644 index 0000000000..0b358d910d --- /dev/null +++ b/test/snapshots/try_map_ok.md @@ -0,0 +1,16 @@ +# META +~~~ini +description=Try.map_ok with Ok and Err variants +type=repl +~~~ +# SOURCE +~~~roc +» Try.map_ok(Try.Err("failed"), |val| val + 1) +» Try.map_ok(Try.Ok(100), |val| val - 50) +~~~ +# OUTPUT +Err("failed") +--- +Ok(50) +# PROBLEMS +NIL