From c98d45fdf9475deeed725c24cf24dfcf6bcbb29f Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Tue, 27 Oct 2026 09:41:30 +0300 Subject: [PATCH] transform: avoid folding array text after storage escapes --- doc/docs.md | 3 + vlib/v/driver/driver.v | 5 + vlib/v/gen/wasm/tour_test.v | 34 +++++ .../array_shared_slice_text_test.v | 120 ++++++++++++++++++ vlib/v/transform/comptime_string.v | 88 ++++++++++++- vlib/v/transform/comptime_string_fold_test.v | 59 +++++++++ 6 files changed, 304 insertions(+), 5 deletions(-) create mode 100644 vlib/v/tests/builtin_arrays/array_shared_slice_text_test.v diff --git a/doc/docs.md b/doc/docs.md index 2a21d13bca..45ade812df 100644 --- a/doc/docs.md +++ b/doc/docs.md @@ -1730,6 +1730,9 @@ the parent array or the resulting slice is mutable, V inserts an implicit affecting the other. If both arrays are immutable, the slice safely reuses the parent array's memory without cloning. The shared-memory examples below therefore use `unsafe {}` intentionally. +Printing or interpolating the parent array shows changes made through such a shared slice. +Formatting such a mutated array with `-b wasm` currently reports an unsupported +runtime-formatting error. ```v mut a := [0, 1, 2, 3, 4, 5] diff --git a/vlib/v/driver/driver.v b/vlib/v/driver/driver.v index 3b7b70b657..25f268ad5e 100644 --- a/vlib/v/driver/driver.v +++ b/vlib/v/driver/driver.v @@ -21907,6 +21907,11 @@ fn unsupported_backend_node_error(a &flat.FlatAst, tc &types.TypeChecker, id fla if callee.kind == .ident && callee.value == 'new_map' { unsupported_type = tc.resolve_type(id).source_type_id() } + // Runtime auto-formatters need a recursion guard with no wasm body. + // Constant array text is folded before this post-transform check. + if callee.kind == .ident && callee.value == 'autostr_addr_type_push' { + return '${fallback_location}error: the V wasm backend does not support runtime value formatting yet' + } } if unsupported_type != 0 { return '${fallback_location}error: the V wasm backend does not support type `${flat.source_type_name(unsupported_type)}` yet' diff --git a/vlib/v/gen/wasm/tour_test.v b/vlib/v/gen/wasm/tour_test.v index 09383b6e3e..0bfc673175 100644 --- a/vlib/v/gen/wasm/tour_test.v +++ b/vlib/v/gen/wasm/tour_test.v @@ -401,3 +401,37 @@ fn test_fold_print_wasm_matches_native_text() { } assert code == 0, 'the host reported a non-zero exit: ${out}' } + +fn test_shared_array_text_requires_runtime_formatting() { + mutations := [ + 'mut view := unsafe { values[..] }; view[0] = 9', + 'change(values)', + 'alias := values; change(alias)', + 'ArrayTextInts(values).change()', + ] + for i, mutation in mutations { + for j, print_expr in ['println("\${values}")', 'println(values.str())'] { + source := ' +fn change(values []int) { + mut view := unsafe { values[..] } + view[0] = 9 +} +type ArrayTextInts = []int +fn (values ArrayTextInts) change() { + change(values) +} +fn main() { + values := [1, 2] + ${mutation} + ${print_expr} +} +' + tour_compile('shared_array_text_${i}_${j}', source) or { + assert err.msg().contains('error: the V wasm backend does not support runtime value formatting yet'), err.msg() + assert !err.msg().contains('panic'), err.msg() + continue + } + assert false, 'shared array text unexpectedly compiled; check that runtime formatting supports [9, 2]' + } + } +} diff --git a/vlib/v/tests/builtin_arrays/array_shared_slice_text_test.v b/vlib/v/tests/builtin_arrays/array_shared_slice_text_test.v new file mode 100644 index 0000000000..cb484b0691 --- /dev/null +++ b/vlib/v/tests/builtin_arrays/array_shared_slice_text_test.v @@ -0,0 +1,120 @@ +fn test_array_text_reads_updates_through_an_unsafe_shared_slice() { + values := [1, 2] + // The slice deliberately shares the immutable array's element storage. + mut view := unsafe { values[..] } + view[0] = 9 + assert values[0] == 9 + assert '${values}' == '[9, 2]' + assert values.str() == '[9, 2]' +} + +fn test_split_array_text_reads_updates_through_an_unsafe_shared_slice() { + values := 'a,b'.split(',') + // The slice deliberately shares the immutable array's element storage. + mut view := unsafe { values[..] } + view[0] = 'z' + assert values[0] == 'z' + assert '${values}' == "['z', 'b']" + assert values.str() == "['z', 'b']" +} + +fn test_bool_array_text_reads_updates_through_an_unsafe_shared_slice() { + values := [true, false] + // The slice deliberately shares the immutable array's element storage. + mut view := unsafe { values[..] } + view[0] = false + assert !values[0] + assert '${values}' == '[false, false]' + assert values.str() == '[false, false]' +} + +fn test_array_text_reads_updates_through_a_data_pointer() { + values := [1, 2] + unsafe { + // The pointer deliberately aliases the immutable array's element storage. + mut data := &int(values.data) + data[0] = 9 + } + assert values[0] == 9 + assert '${values}' == '[9, 2]' + assert values.str() == '[9, 2]' +} + +fn test_array_text_reads_updates_through_a_captured_shared_slice() { + values := [1, 2] + mutate := fn [values] () { + // The captured header retains the original array's element storage. + mut view := unsafe { values[..] } + view[0] = 9 + } + mutate() + assert values[0] == 9 + assert '${values}' == '[9, 2]' + assert values.str() == '[9, 2]' +} + +fn test_array_text_reads_updates_through_a_header_pointer() { + values := [1, 2] + // The pointer deliberately retains the original array header and storage. + mut alias := unsafe { &values } + unsafe { + (*alias)[0] = 9 + } + assert values[0] == 9 + assert '${values}' == '[9, 2]' + assert values.str() == '[9, 2]' +} + +fn mutate_array_text_storage(values []int) { + // The callee deliberately changes storage shared with an immutable argument. + mut view := unsafe { values[..] } + view[0] = 9 +} + +type ArrayTextInts = []int + +fn (values ArrayTextInts) mutate_array_text_receiver() { + mutate_array_text_storage(values) +} + +fn test_array_text_reads_updates_through_an_immutable_argument() { + values := [1, 2] + mutate_array_text_storage(values) + assert values[0] == 9 + assert '${values}' == '[9, 2]' + assert values.str() == '[9, 2]' +} + +fn test_array_text_reads_updates_through_an_alias_argument() { + values := [1, 2] + alias := values + mutate_array_text_storage(alias) + assert values[0] == 9 + assert '${values}' == '[9, 2]' + assert values.str() == '[9, 2]' +} + +fn test_array_text_reads_updates_through_a_slice_argument() { + values := [1, 2] + mutate_array_text_storage(values[..]) + assert values[0] == 9 + assert '${values}' == '[9, 2]' + assert values.str() == '[9, 2]' +} + +fn test_array_text_reads_updates_through_an_immutable_receiver() { + values := [1, 2] + ArrayTextInts(values).mutate_array_text_receiver() + assert values[0] == 9 + assert '${values}' == '[9, 2]' + assert values.str() == '[9, 2]' +} + +fn test_array_text_reads_updates_through_a_slice_alias_argument() { + values := [1, 2] + view := values[..] + mutate_array_text_storage(view) + assert values[0] == 9 + assert '${values}' == '[9, 2]' + assert values.str() == '[9, 2]' +} diff --git a/vlib/v/transform/comptime_string.v b/vlib/v/transform/comptime_string.v index 0858fc0f00..5ea805b510 100644 --- a/vlib/v/transform/comptime_string.v +++ b/vlib/v/transform/comptime_string.v @@ -419,8 +419,8 @@ fn comptime_array_elem_text(element ComptimeStringScalar, elem_type u32) ?string } // later_stmts_write_array_local reports whether a statement after the declaration -// writes through the local. An element write or a mutable borrow changes the array -// a print would render, so a compile-time array is only trusted when none follows. +// writes through the local or exposes its storage. Aliases and unknown callees +// may change the elements a print renders, even when the binding is immutable. fn (t &Transformer) later_stmts_write_array_local(decl_id flat.NodeId, name string) bool { ids := t.cur_stmt_list mut first := 0 @@ -443,14 +443,63 @@ fn (t &Transformer) node_writes_array_local(id flat.NodeId, name string) bool { return false } node := t.a.nodes[int(id)] - // A function literal has its own scope, so a name declared there is another - // binding and a captured array is a copy that belongs to the closure. - if node.kind == .fn_literal { + // A captured array header can still share its element storage. Only inspect + // closures that capture this binding; other closures have their own locals. + if node.kind == .fn_literal && !t.fn_literal_captures_name(id, name) { return false } if node.kind == .ident && node.is_mut && node.value == name { return true } + // Unsafe slices can share the original array's elements. An address or data + // pointer can expose that storage too, even when the binding is immutable. + if ((node.kind == .block && node.value == 'unsafe') + || (node.kind == .prefix && node.op == .amp) + || (node.kind == .selector && node.value == 'data')) + && t.array_map_expr_references_ident(id, name) { + return true + } + if node.kind in [.decl_assign, .assign, .selector_assign, .index_assign, .return_stmt] { + first := if node.kind == .decl_assign { 1 } else { 0 } + for i in first .. node.children_count { + if t.expr_exposes_array_local(t.a.child(&node, i), name) { + return true + } + } + } + if node.kind == .infix && node.op == .left_shift && node.children_count == 2 + && t.expr_exposes_array_local(t.a.child(&node, 1), name) { + return true + } + if node.kind == .for_in_stmt && node.children_count >= 3 { + container := t.a.child(&node, 2) + // Iterating the array itself reads scalars; an aggregate may yield its header. + if t.a.node(container).kind != .ident && t.expr_exposes_array_local(container, name) { + return true + } + } + if node.kind == .call && node.children_count > 0 { + callee := t.a.child_node(&node, 0) + // These compiler intrinsics render their argument without exposing storage. + is_print := callee.kind == .ident + && callee.value in ['print', 'println', 'eprint', 'eprintln'] + if !is_print { + for i in 1 .. node.children_count { + if t.expr_exposes_array_local(t.a.child(&node, i), name) { + return true + } + } + } + if callee.kind == .selector && callee.children_count > 0 { + receiver := t.a.child(callee, 0) + receiver_node := t.a.node(receiver) + is_array_str := callee.value == 'str' && node.children_count == 1 + && receiver_node.kind == .ident && receiver_node.value == name + if !is_array_str && t.expr_exposes_array_local(receiver, name) { + return true + } + } + } if node.kind in [.index_assign, .selector_assign] { if t.node_writes_array_local(t.a.child(&node, 0), name) { return true @@ -464,6 +513,35 @@ fn (t &Transformer) node_writes_array_local(id flat.NodeId, name string) bool { return false } +// expr_exposes_array_local recognizes expressions that can retain the array's +// storage. Scalar reads and rendered strings do not carry that storage to callees. +fn (t &Transformer) expr_exposes_array_local(id flat.NodeId, name string) bool { + if int(id) < 0 || int(id) >= t.a.nodes.len { + return false + } + node := t.a.nodes[int(id)] + match node.kind { + .ident { return node.value == name } + .fn_literal { return t.fn_literal_captures_name(id, name) } + .index { + if node.value != 'range' { + return false + } + } + .paren, .cast_expr, .as_expr, .array_literal, .array_init, .struct_init, + .field_init, .map_init, .if_expr, .match_stmt, .match_branch, .or_expr, + .assoc, .block, .expr_stmt { + } + else { return false } + } + for i in 0 .. node.children_count { + if t.expr_exposes_array_local(t.a.child(&node, i), name) { + return true + } + } + return false +} + // later_stmts_assign_local reports whether a statement that follows the declaration // `decl_id` in its statement list assigns to the declared local `name`, increments it, or // passes it as a mutable argument or exposes its address. Only those statements can see the binding: the same name diff --git a/vlib/v/transform/comptime_string_fold_test.v b/vlib/v/transform/comptime_string_fold_test.v index 18a95e4c6e..0a7766780f 100644 --- a/vlib/v/transform/comptime_string_fold_test.v +++ b/vlib/v/transform/comptime_string_fold_test.v @@ -266,3 +266,62 @@ fn test_comptime_array_bindings_restore_after_a_nested_statement_scope() { } assert saw_inner } + +fn test_comptime_array_text_preserves_exposed_storage() { + for source in [ + 'fn main() { values := [1, 2]; mut view := unsafe { values[..] }; view[0] = 9 }', + 'fn main() { values := [1, 2]; address := &values }', + 'fn main() { values := [1, 2]; change(values) }', + 'fn main() { values := [1, 2]; alias := values; change(alias) }', + 'fn main() { values := [1, 2]; change(values[..]) }', + 'fn main() { values := [1, 2]; values.change() }', + 'type ArrayAlias = []int\nfn main() { values := [1, 2]; ArrayAlias(values).str() }', + 'fn main() { values := [1, 2]; holder.values = values; change(holder.values) }', + 'fn main() { values := [1, 2]; rows[0] = values; change(rows[0]) }', + 'fn main() { values := [1, 2]; alias := if cond { side_effect(); values } else { []int{} }; change(alias) }', + 'fn main() { values := [1, 2]; for alias in [values] { change(alias) } }', + 'fn main() { values := [1, 2]; mut rows := [][]int{}; rows << values; change(rows[0]) }', + 'fn main() { values := [1, 2]; data := values.data }', + 'fn main() { values := [1, 2]; f := fn [values] () { mut view := unsafe { values[..] }; view[0] = 9 }; f() }', + ] { + mut p := parser.Parser.new(pref.new_preferences()) + mut a := p.parse_text('array_storage_escape.v', source) + assert p.diagnostics.len == 0, p.diagnostics.str() + mut tc := types.TypeChecker.new(a) + mut t := new_transformer(mut a, &tc, map[string]bool{}) + mut found := false + for i, node in a.nodes { + if node.kind == .fn_decl && node.value == 'main' { + found = true + assert t.node_writes_array_local(flat.NodeId(i), 'values'), source + } + } + assert found + } +} + +fn test_comptime_array_text_can_fold_read_only_storage() { + for source in [ + 'fn main() { values := [1, 2]; println(values) }', + 'fn main() { values := [1, 2]; println(values.str()); eprintln(values) }', + 'fn main() { values := [1, 2]; consume("\${values}"); consume(values[0]) }', + 'fn main() { values := [1, 2]; text := values.str(); number := values[0] }', + 'fn main() { values := [1, 2]; for number in values { consume(number) }; println(values) }', + 'fn main() { values := [1, 2]; f := fn () { values := [3]; mut view := unsafe { values[..] }; view[0] = 9 }; f(); println(values) }', + 'fn main() { values := [1, 2]; other := [3]; mut view := unsafe { other[..] }; view[0] = 9; println(values) }', + ] { + mut p := parser.Parser.new(pref.new_preferences()) + mut a := p.parse_text('array_storage_read.v', source) + assert p.diagnostics.len == 0, p.diagnostics.str() + mut tc := types.TypeChecker.new(a) + mut t := new_transformer(mut a, &tc, map[string]bool{}) + mut found := false + for i, node in a.nodes { + if node.kind == .fn_decl && node.value == 'main' { + found = true + assert !t.node_writes_array_local(flat.NodeId(i), 'values'), source + } + } + assert found + } +}