v3: address PR review batch 19 — heap escaped pointer-alias locals, unwrap fixed-array returns from static/import-qualified calls

Two PR-review fixes (three earlier comments were already resolved in batch 18;
GitHub just re-anchored their line numbers):

- transform.v: a value local whose address escapes (`p := &v` with `p`
  returned) was heap-copied eagerly at the alias, so a mutation between the
  alias and the return (`mut v := Box{x: 1}; p := &v; v.x = 2; return p`) was
  lost — the caller saw stale data (x == 1). Move the local itself to the heap
  at its declaration (its type becomes `&T`; a struct literal becomes `&T{..}`,
  any other init is copied into a temp and memdup'd) and make the alias `p := v`,
  so writes through `v` are visible to the caller, matching V's auto-heap
  semantics (V1 returns 2). cgen already handles pointer-typed locals.

- stmt.v/cleanc.v: the fixed-array-return unwrap (`(call()).ret_arr`) relied on
  declared_call_return_type, whose selector path only handled receiver methods.
  A static method (`Type.make()`) or import-qualified function (`mod.make()`)
  returning `[N]T` emits a `_v_ret_*` wrapper that was never unwrapped, so the
  assignment/index saw the wrapper struct (invalid C). Resolve such selector
  calls the same way gen_call does (static_method_fn_name / import_alias_module)
  and read the declared return type. declared_call_return_type now takes the
  call node id.

Adds regression tests for both.

Author: medvednikov

vlib/v3/gen/c/cleanc.v

@@ -1768,7 +1768,7 @@ fn (mut g FlatGen) gen_expr(id flat.NodeId) {
 			// A call to a fixed-array-returning function yields the wrapper struct;
 			// unwrap `.ret_arr` so the result behaves as the array value everywhere
 			// (indexing, arg passing, memcpy into a destination).
-			ret_t := g.declared_call_return_type(node)
+			ret_t := g.declared_call_return_type(id)
 			if ret_t is types.ArrayFixed && g.tc.c_type(ret_t) in g.fixed_array_ret_wrappers {
 				g.write('(')
 				g.gen_call(id, node)

vlib/v3/gen/c/stmt.v

@@ -261,7 +261,7 @@ fn (mut g FlatGen) gen_node(id flat.NodeId) {
 						raw_expr_type := g.tc.resolve_type(ret_id)
 						expr_type := g.usable_expr_type(ret_id)
 						call_ret_type := g.local_fn_call_return_type(ret_id, ret_node)
-						decl_ret_type := g.declared_call_return_type(ret_node)
+						decl_ret_type := g.declared_call_return_type(ret_id)
 						if g.optional_result_matches_base(raw_expr_type, base)
 							|| g.optional_result_matches_base(expr_type, base)
 							|| g.optional_result_matches_base(call_ret_type, base)
@@ -567,7 +567,7 @@ fn (mut g FlatGen) return_expr_string(node flat.Node, ret_id flat.NodeId, ret_no
 		raw_expr_type := g.tc.resolve_type(ret_id)
 		expr_type := g.usable_expr_type(ret_id)
 		call_ret_type := g.local_fn_call_return_type(ret_id, ret_node)
-		decl_ret_type := g.declared_call_return_type(ret_node)
+		decl_ret_type := g.declared_call_return_type(ret_id)
 		if g.optional_result_matches_base(raw_expr_type, base)
 			|| g.optional_result_matches_base(expr_type, base)
 			|| g.optional_result_matches_base(call_ret_type, base)
@@ -673,7 +673,11 @@ fn (g &FlatGen) local_fn_call_return_type(call_id flat.NodeId, call_node flat.No
 // becomes `?int`), which makes the optional C type name appear to differ from
 // the callee's signature. The declared type read from `fn_ret_types`/the fn decl
 // keeps the alias, so propagating `return call()` is recognised as valid.
-fn (g &FlatGen) declared_call_return_type(call_node flat.Node) types.Type {
+fn (g &FlatGen) declared_call_return_type(call_id flat.NodeId) types.Type {
+	if int(call_id) < 0 {
+		return types.Type(types.void_)
+	}
+	call_node := g.a.nodes[int(call_id)]
 	if call_node.kind != .call || call_node.children_count == 0 {
 		return types.Type(types.void_)
 	}
@@ -716,6 +720,26 @@ fn (g &FlatGen) selector_call_return_type(fn_node flat.Node) ?types.Type {
 		return none
 	}
 	base_id := g.a.child(&fn_node, 0)
+	base_node := g.a.nodes[int(base_id)]
+	// A selector whose base names a type or an imported module is not a receiver method but a
+	// static method (`Type.make()`) or import-qualified function (`mod.make()`); the base ident
+	// has no value type, so resolve it the same way gen_call does and read the declared return
+	// type. Without this a fixed-array such call's `_v_ret_*` wrapper is never unwrapped.
+	if base_node.kind == .ident && base_node.value.len > 0 {
+		base_is_local := g.tc.cur_scope.lookup(base_node.value) or { types.Type(types.void_) } !is types.Void
+		if !base_is_local {
+			if static_fn := g.static_method_fn_name(base_node.value, fn_node.value) {
+				if ret := g.tc.fn_ret_types[static_fn] {
+					return ret
+				}
+			}
+			if mod := g.import_alias_module(base_node.value) {
+				if ret := g.tc.fn_ret_types['${mod}.${fn_node.value}'] {
+					return ret
+				}
+			}
+		}
+	}
 	base_type := g.tc.resolve_type(base_id)
 	clean_type := types.unwrap_pointer(base_type)
 	mut receiver_name := clean_type.name()

vlib/v3/tests/type_checker_errors_test.v

@@ -778,3 +778,19 @@ fn test_pr_review_codegen_batch_eighteen() {
 		"fn work() [0x10]u8 {\n\tmut r := [0x10]u8{}\n\tr[0] = 42\n\tr[15] = 7\n\treturn r\n}\nfn main() {\n\tmut threads := []thread [0x10]u8{}\n\tthreads << spawn work()\n\tresults := threads.wait()\n\tprintln(int_str(results[0][0]) + ',' + int_str(results[0][15]))\n}\n")
 	assert thread_fixed == '42,7'
 }
+
+fn test_pr_review_codegen_batch_nineteen() {
+	v3_bin := build_v3()
+	// A value local whose address escapes (`p := &v` with `p` returned) is moved to the heap at
+	// its declaration, so a mutation between the alias and the return is observed by the caller —
+	// matching V's auto-heap semantics. Copying eagerly at `p := &v` returned stale data (x == 1).
+	escaped := run_good(v3_bin, 'good_escaped_pointer_alias_mutation',
+		'struct Box {\nmut:\n\tx int\n}\nfn make() &Box {\n\tmut v := Box{\n\t\tx: 1\n\t}\n\tp := &v\n\tv.x = 2\n\treturn p\n}\nfn main() {\n\tb := make()\n\tprintln(int_str(b.x))\n}\n')
+	assert escaped == '2'
+	// A static-method call returning a fixed array (`Type.make() [N]T`) is lowered to a selector
+	// whose base is a type, not a receiver value; its `_v_ret_*` wrapper must still be unwrapped to
+	// `.ret_arr`, so the assignment/indexing below sees the array, not the wrapper struct.
+	static_fixed := run_good(v3_bin, 'good_static_method_fixed_array_return',
+		'struct Maker {}\nfn Maker.make() [3]int {\n\treturn [10, 20, 30]!\n}\nfn main() {\n\ta := Maker.make()\n\tprintln(int_str(a[0] + a[1] + a[2]))\n}\n')
+	assert static_fixed == '60'
+}

vlib/v3/transform/transform.v

@@ -96,6 +96,14 @@ mut:
 	// on return. Recomputed per function (structural pre-pass in transform_fn_body),
 	// consumed when the `p := &v` decl is transformed (RHS rewritten to a heap copy).
 	escaping_amp_ptrs map[string]bool
+	// escaping_amp_sources holds the source locals `v` of such `p := &v` escapes — the
+	// values whose address leaves the frame. The local itself is moved to the heap at its
+	// declaration (its type becomes `&T`) so a mutation between `p := &v` and `return p`
+	// is observed by the caller; copying eagerly at the alias would return stale data.
+	escaping_amp_sources map[string]bool
+	// heaped_amp_locals records which of those sources were actually moved to the heap, so
+	// the `p := &v` alias emits `p = v` (the heap pointer) instead of a fresh memdup copy.
+	heaped_amp_locals map[string]bool
 }
 
 // AliasCache memoizes normalize_type_alias results. It lives on the heap so the
@@ -1029,24 +1037,72 @@ fn (t &Transformer) try_heap_escaping_amp(node flat.Node, rhs_id flat.NodeId) bo
 	if amp_node.kind != .ident {
 		return false
 	}
+	// The source local was moved to the heap at its declaration: the alias is now just that
+	// `&T` pointer (handled below), regardless of its rewritten pointer type.
+	if amp_node.value in t.heaped_amp_locals {
+		return true
+	}
 	local_type := t.node_type(amp_child)
 	return local_type.len > 0 && !local_type.starts_with('&') && !local_type.starts_with('[]')
 		&& !local_type.starts_with('map[') && !local_type.starts_with('?')
 		&& !local_type.starts_with('!')
 }
 
 // heap_escaping_amp_rhs rewrites `&v` into `(&T)memdup(&v, sizeof(T))`, a heap copy
-// of the value local `v` so the escaping pointer outlives the stack frame.
+// of the value local `v` so the escaping pointer outlives the stack frame. When `v` was
+// itself moved to the heap at its declaration, the alias is simply that pointer — copying
+// would resurrect the stale-mutation bug the move avoids.
 fn (mut t Transformer) heap_escaping_amp_rhs(rhs_id flat.NodeId) flat.NodeId {
 	rhs := t.a.nodes[int(rhs_id)]
 	amp_child := t.a.child(&rhs, 0)
+	amp_node := t.a.nodes[int(amp_child)]
+	if amp_node.kind == .ident && amp_node.value in t.heaped_amp_locals {
+		return t.transform_expr(amp_child)
+	}
 	local_type := t.node_type(amp_child)
 	addr := t.make_prefix(.amp, t.transform_expr(amp_child))
 	size := t.make_sizeof_type(local_type)
 	dup := t.make_call_typed('memdup', arr2(addr, size), 'voidptr')
 	return t.make_cast('&${local_type}', dup, '&${local_type}')
 }
 
+// heapable_value_type reports whether a local of this declared type can be moved to the heap
+// as a `&T` — a plain value type, not an already-reference / container / optional type (those
+// either carry their own indirection or are not addressable as a single `T`).
+fn (t &Transformer) heapable_value_type(typ string) bool {
+	return typ.len > 0 && !typ.starts_with('&') && !typ.starts_with('[]')
+		&& !typ.starts_with('map[') && !typ.starts_with('?') && !typ.starts_with('!')
+		&& !typ.starts_with('[') && typ != 'unknown' && typ != 'void'
+}
+
+// heap_escaping_source_decl rewrites `mut v := <init>` (where `&v` escapes) into a heap
+// allocation so `v` is a `&T` to a heap object. A struct literal becomes `&T{..}` (the cgen
+// memdup's it); any other initializer is copied into a stack temp and memdup'd. Subsequent
+// `v.field = ..` writes then mutate the heap object the returned pointer alias also sees.
+fn (mut t Transformer) heap_escaping_source_decl(node flat.Node, var_name string, elem_typ string) []flat.NodeId {
+	rhs_id := t.a.child(&node, 1)
+	rhs := t.a.nodes[int(rhs_id)]
+	ptr_typ := '&${elem_typ}'
+	mut stmts := []flat.NodeId{}
+	transformed_init := t.transform_expr(rhs_id)
+	// Statements lifted out while transforming the initializer must precede the heap decl.
+	t.drain_pending(mut stmts)
+	mut heap_rhs := flat.NodeId(0)
+	if rhs.kind == .struct_init {
+		heap_rhs = t.make_prefix(.amp, transformed_init)
+	} else {
+		tmp := t.new_temp('esc')
+		stmts << t.make_decl_assign_typed(tmp, transformed_init, elem_typ)
+		addr := t.make_prefix(.amp, t.make_ident(tmp))
+		size := t.make_sizeof_type(elem_typ)
+		dup := t.make_call_typed('memdup', arr2(addr, size), 'voidptr')
+		heap_rhs = t.make_cast(ptr_typ, dup, ptr_typ)
+	}
+	t.heaped_amp_locals[var_name] = true
+	stmts << t.make_decl_assign_typed(var_name, heap_rhs, ptr_typ)
+	return stmts
+}
+
 // mark_escaping_amp_ptrs runs a structural pre-pass over a function body to find
 // `p := &v` declarations whose pointer `p` is later returned. Such a `v` is a local
 // value whose address escapes, so it must be heap-copied (V auto-heaps it); the
@@ -1055,22 +1111,29 @@ fn (mut t Transformer) heap_escaping_amp_rhs(rhs_id flat.NodeId) flat.NodeId {
 // at rewrite time when `v`'s type is known.
 fn (mut t Transformer) mark_escaping_amp_ptrs(body_ids []flat.NodeId) {
 	t.escaping_amp_ptrs = map[string]bool{}
+	t.escaping_amp_sources = map[string]bool{}
+	t.heaped_amp_locals = map[string]bool{}
 	mut amp_ptrs := map[string]bool{}
+	mut amp_sources := map[string]string{} // pointer `p` -> source local `v`
 	mut returned := map[string]bool{}
 	for id in body_ids {
-		t.scan_escape_pass(id, mut amp_ptrs, mut returned)
+		t.scan_escape_pass(id, mut amp_ptrs, mut amp_sources, mut returned)
 	}
 	for name, _ in amp_ptrs {
 		if name in returned {
 			t.escaping_amp_ptrs[name] = true
+			if src := amp_sources[name] {
+				t.escaping_amp_sources[src] = true
+			}
 		}
 	}
 }
 
 // scan_escape_pass recursively collects, in a function-body subtree, (a) the LHS
-// names of `p := &ident` declarations into `amp_ptrs`, and (b) every ident name
-// appearing inside a return statement into `returned`.
-fn (mut t Transformer) scan_escape_pass(id flat.NodeId, mut amp_ptrs map[string]bool, mut returned map[string]bool) {
+// names of `p := &ident` declarations into `amp_ptrs` (and the source `ident` into
+// `amp_sources[p]`), and (b) every ident name appearing inside a return statement
+// into `returned`.
+fn (mut t Transformer) scan_escape_pass(id flat.NodeId, mut amp_ptrs map[string]bool, mut amp_sources map[string]string, mut returned map[string]bool) {
 	if int(id) < 0 || int(id) >= t.a.nodes.len {
 		return
 	}
@@ -1083,6 +1146,7 @@ fn (mut t Transformer) scan_escape_pass(id flat.NodeId, mut amp_ptrs map[string]
 			amp_child := t.a.nodes[int(t.a.child(&rhs, 0))]
 			if amp_child.kind == .ident {
 				amp_ptrs[lhs.value] = true
+				amp_sources[lhs.value] = amp_child.value
 			}
 		}
 	}
@@ -1092,7 +1156,7 @@ fn (mut t Transformer) scan_escape_pass(id flat.NodeId, mut amp_ptrs map[string]
 		}
 	}
 	for i in 0 .. node.children_count {
-		t.scan_escape_pass(t.a.child(&node, i), mut amp_ptrs, mut returned)
+		t.scan_escape_pass(t.a.child(&node, i), mut amp_ptrs, mut amp_sources, mut returned)
 	}
 }
 
@@ -2687,13 +2751,34 @@ fn (mut t Transformer) transform_decl_assign_stmt(id flat.NodeId, node flat.Node
 			}
 		}
 	}
+	// A value local whose address escapes (`p := &v` with `p` returned) is moved to the heap
+	// at its own declaration so writes after the alias are visible to the caller. Must run
+	// before the `p := &v` alias is transformed (the source is declared first).
+	if node.children_count == 2 {
+		src := t.a.child_node(&node, 0)
+		if src.kind == .ident && src.value in t.escaping_amp_sources
+			&& src.value !in t.heaped_amp_locals && t.heapable_value_type(inferred_typ) {
+			return t.heap_escaping_source_decl(node, src.value, inferred_typ)
+		}
+	}
 	mut new_children := []flat.NodeId{cap: int(node.children_count)}
 	for i in 0 .. node.children_count {
 		child_id := t.a.child(&node, i)
 		if i == 0 || (node.children_count > 2 && i > 1) {
 			new_children << t.transform_lvalue(child_id)
 		} else if node.children_count == 2 && t.try_heap_escaping_amp(node, child_id) {
 			new_children << t.heap_escaping_amp_rhs(child_id)
+			// When `v` was heap-moved it is already a `&T`, so `p := &v` is really `p := v`
+			// (a `&T`), not `&&T` as the literal `&v` would infer. Adopt the source's pointer
+			// type for `p` so its declaration and later uses are consistent.
+			amp := t.a.nodes[int(child_id)]
+			if amp.children_count > 0 {
+				amp_src := t.a.nodes[int(t.a.child(&amp, 0))]
+				if amp_src.kind == .ident && amp_src.value in t.heaped_amp_locals {
+					inferred_typ = t.var_type(amp_src.value)
+					t.set_var_type(t.a.nodes[int(t.a.child(&node, 0))].value, inferred_typ)
+				}
+			}
 		} else {
 			lhs_id := t.a.child(&node, 0)
 			lhs_type := if inferred_typ.len > 0 {