v3: address PR review batch 25 — escape-mark only returned pointers, optional fixed-array returns through defers

The escape prepass (mark_escaping_amp_ptrs) collected every ident in a
return expression, so a pointer that merely appears in a consuming
expression was treated as escaping. For `return p == p && v == 1`, that
heap-moved `v` even though only a bool is returned, and later codegen
read `v` as an int* in the integer comparison (wrong result). Replace
collect_subtree_idents with collect_return_escape_idents, which stops at
operators that consume their operands into a fresh value (infix, postfix,
is/in, and any non-& prefix such as deref *p), so only pointers in a real
escape position (the returned value or a member of a returned aggregate)
mark their source local. This also stops over-marking `return *p`.

An optional fixed-array function (?[N]T) with a pending defer routes
through gen_return_with_defers -> return_expr_string, which lacked the
fixed-array special case the direct path has. The optional's .value is a
fixed-array member that can't be set via a compound literal, so the
deferred path emitted {.ok = false}, dropping the array. Mirror the
direct path's temp + memcpy form (new fixed_array_copy_source_string
capture helper).

Adds regression coverage (batch twentysix).

Author: medvednikov

vlib/v3/gen/c/cleanc.v

@@ -905,6 +905,21 @@ fn (mut g FlatGen) interface_value_to_string(id flat.NodeId, expected types.Type
 	return result
 }
 
+// fixed_array_copy_source_string captures gen_fixed_array_copy_source as a string, so a deferred
+// optional/fixed-array return can embed the memcpy source when saving the value into a temp.
+fn (mut g FlatGen) fixed_array_copy_source_string(value_id flat.NodeId, field_type types.Type) string {
+	orig := g.sb
+	orig_line_start := g.line_start
+	g.sb = strings.new_builder(64)
+	// Emit mid-statement (no leading indent), matching the direct return path.
+	g.line_start = false
+	g.gen_fixed_array_copy_source(value_id, field_type)
+	result := g.sb.str()
+	g.sb = orig
+	g.line_start = orig_line_start
+	return result
+}
+
 // expr_to_string_with_expected_type converts expr to string with expected type data for c.
 fn (mut g FlatGen) expr_to_string_with_expected_type(id flat.NodeId, expected types.Type) string {
 	orig := g.sb

vlib/v3/gen/c/stmt.v

@@ -564,6 +564,13 @@ fn (mut g FlatGen) return_expr_string(node flat.Node, ret_id flat.NodeId, ret_no
 		if base is types.Void {
 			return '(${ct}){.ok = false}'
 		}
+		if base is types.ArrayFixed {
+			// The optional's `.value` is a fixed-array member, which can't be set in a compound
+			// literal; build via a temp + memcpy (mirrors the direct return path) so a deferred
+			// return saves the array value instead of dropping it to `{.ok = false}`.
+			src := g.fixed_array_copy_source_string(ret_id, base)
+			return '({ ${ct} __opt = {.ok = true}; memcpy(__opt.value, ${src}, sizeof(__opt.value)); __opt; })'
+		}
 		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)

vlib/v3/tests/type_checker_errors_test.v

@@ -949,3 +949,27 @@ fn test_pr_review_codegen_batch_twentyfive() {
 		"fn compound() int {\n\tmut v := 1\n\tp := &v\n\tv += 1\n\treturn *p\n}\nfn postfix() int {\n\tmut v := 5\n\tp := &v\n\tv++\n\tv++\n\treturn *p\n}\nfn main() {\n\tprintln(int_str(compound()) + ',' + int_str(postfix()))\n}\n")
 	assert heap_compound == '2,7'
 }
+
+fn test_pr_review_codegen_batch_twentysix() {
+	v3_bin := build_v3()
+	// The escape prepass must only heap-move a local whose address is *actually returned*. A
+	// pointer that merely appears in a consuming expression (comparison/boolean here, or a deref)
+	// does not escape, so its source local must stay a plain value — otherwise codegen reads it as
+	// an `int*` in the integer comparison and the result is wrong. `return p == p && v == 1` and
+	// `return *p` both keep `v` on the stack.
+	no_escape := run_good(v3_bin, 'good_escape_only_returned_pointer',
+		"fn check() bool {\n\tmut v := 1\n\tp := &v\n\treturn p == p && v == 1\n}\nfn deref() int {\n\tmut v := 3\n\tp := &v\n\tv = 8\n\treturn *p\n}\nfn main() {\n\tprintln(check().str() + ',' + int_str(deref()))\n}\n")
+	assert no_escape == 'true,8'
+	// A pointer that *is* returned (directly, and inside a returned aggregate) must still heap-move
+	// its source local, so the returned pointer observes mutations made after the address was taken.
+	escapes := run_good(v3_bin, 'good_escape_returned_pointer_and_aggregate',
+		"fn direct() &int {\n\tmut v := 10\n\tp := &v\n\tv += 5\n\treturn p\n}\nfn aggregate() []&int {\n\tmut v := 7\n\tp := &v\n\tv = 9\n\treturn [p]\n}\nfn main() {\n\tarr := aggregate()\n\tprintln(int_str(*direct()) + ',' + int_str(*arr[0]))\n}\n")
+	assert escapes == '15,9'
+	// An optional fixed-array return (`?[N]T`) with a pending `defer` routes through the deferred
+	// return path; that path must apply the same temp + memcpy handling as the direct path (the
+	// `.value` member is a fixed array and cannot be set via a compound literal), so the array value
+	// is preserved instead of being dropped to `{.ok = false}`.
+	defer_opt_arr := run_good(v3_bin, 'good_optional_fixed_array_return_with_defer',
+		"fn f() ?[2]int {\n\tdefer {\n\t\t_ := 0\n\t}\n\treturn [1, 2]!\n}\nfn main() {\n\ta := f() or { [0, 0]! }\n\tprintln(int_str(a[0]) + ',' + int_str(a[1]))\n}\n")
+	assert defer_opt_arr == '1,2'
+}

vlib/v3/transform/transform.v

@@ -1178,7 +1178,7 @@ fn (mut t Transformer) scan_escape_pass(id flat.NodeId, mut amp_ptrs map[string]
 	}
 	if node.kind == .return_stmt {
 		for i in 0 .. node.children_count {
-			t.collect_subtree_idents(t.a.child(&node, i), mut returned)
+			t.collect_return_escape_idents(t.a.child(&node, i), mut returned)
 		}
 	}
 	for i in 0 .. node.children_count {
@@ -1187,18 +1187,42 @@ fn (mut t Transformer) scan_escape_pass(id flat.NodeId, mut amp_ptrs map[string]
 	}
 }
 
-// collect_subtree_idents gathers every ident name in a subtree (used to find which
-// names flow into a return statement).
-fn (mut t Transformer) collect_subtree_idents(id flat.NodeId, mut names map[string]bool) {
+// collect_return_escape_idents gathers the idents in a return-expression subtree that occupy an
+// actual escape position — the returned value itself, or a member of a returned aggregate
+// (struct/array/map literal, multi-return). It deliberately stops at operators that consume their
+// operands into a fresh value: infix (`==`, `&&`, arithmetic, …), postfix, `is`/`in`, and any
+// non-`&` prefix (deref `*p`, `!x`, `-x`). That way a pointer that is merely compared or
+// dereferenced in the return expression — e.g. `return p == p && v == 1` — is not mistaken for a
+// pointer that escapes, so its source local is not needlessly heap-moved (which would also make
+// later non-pointer uses of that local read through an `int*`).
+fn (mut t Transformer) collect_return_escape_idents(id flat.NodeId, mut names map[string]bool) {
 	if int(id) < 0 || int(id) >= t.a.nodes.len {
 		return
 	}
 	node := t.a.nodes[int(id)]
-	if node.kind == .ident && node.value.len > 0 {
-		names[node.value] = true
+	match node.kind {
+		.ident {
+			if node.value.len > 0 {
+				names[node.value] = true
+			}
+			return
+		}
+		.infix, .postfix, .is_expr, .in_expr {
+			// These yield a new scalar/bool; their operands do not escape through the return.
+			return
+		}
+		.prefix {
+			// `&x` propagates an address (which may escape); any other prefix (`*x`, `!x`, `-x`)
+			// produces a fresh value, so its operand does not escape.
+			if node.op != .amp {
+				return
+			}
+		}
+		else {}
 	}
+
 	for i in 0 .. node.children_count {
-		t.collect_subtree_idents(t.a.child(&node, i), mut names)
+		t.collect_return_escape_idents(t.a.child(&node, i), mut names)
 	}
 }