From 1803bc00358d1e9c1935e46437a27df1819b0b3c Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Fri, 19 Jun 2026 23:20:13 +0300 Subject: [PATCH] cgen: fix fixed-array `if`/`match` return mixing a call and a literal branch (#27457) (#27504) --- vlib/v/gen/c/cgen.v | 25 +++++++++- vlib/v/gen/c/if.v | 18 ++++++- vlib/v/gen/c/match.v | 14 ++++++ vlib/v/tests/return_fixed_array_test.v | 67 ++++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 3 deletions(-) diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index aacaf55c9..8dd1838be 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -196,6 +196,7 @@ mut: inside_lambda_autofree_tmp bool track_lambda_autofree_tmp_arg_vars bool outer_tmp_var string // tmp var from outer context (e.g. from stmts_with_tmp_var) to be used by nested if/match expressions + if_match_tmp_is_fn_ret_arr ?bool // set by if/match expr gen: authoritatively whether the shared tmp var is a fn-returned fixed array (wrapper struct accessed via `.ret_arr`); `none` means use the per-branch heuristic last_tmp_call_var []string last_if_option_type ast.Type // stores the expected if type on nested if expr loop_depth int @@ -4156,8 +4157,18 @@ fn (mut g Gen) stmts_with_tmp_var(stmts []ast.Stmt, tmp_var string) bool { } } ret_sym := g.table.sym(g.unwrap_generic(g.recheck_concrete_type(ret_type))) - tmp_is_return_fixed_array := ret_sym.info is ast.ArrayFixed - && ret_sym.info.is_fn_ret + // The tmp var of an if/match expr is a single shared variable, so every + // branch must agree on whether to write through `.ret_arr`, based on the + // tmp var's declared type rather than each branch's own expr type. A mix + // of a function call and a fixed array literal would otherwise disagree + // (only the function's fixed array type carries the `is_fn_ret` flag), e.g. + // `return if c { fa() } else { [9, 9, 9]! }` or its reverse. When set, the + // flag from if/match gen is authoritative; elsewhere fall back to the + // per-branch heuristic. + tmp_is_return_fixed_array := g.if_match_tmp_is_fn_ret_arr or { + ret_sym.info is ast.ArrayFixed && ret_sym.info.is_fn_ret + } + fixed_array_tmp_var := if tmp_is_return_fixed_array { '${tmp_var}.ret_arr' } else { @@ -7205,7 +7216,12 @@ fn (mut g Gen) expr(node_ ast.Expr) { pos: node.pos expr: node.right } + // This tmp var is local to this `&(...)`; don't inherit an enclosing + // if/match's `.ret_arr` decision (only if/match set this flag). + prev_ret_arr := g.if_match_tmp_is_fn_ret_arr + g.if_match_tmp_is_fn_ret_arr = none g.stmts_with_tmp_var(stmts, tmp_var) + g.if_match_tmp_is_fn_ret_arr = prev_ret_arr g.set_current_pos_as_last_stmt_pos() g.write(str) } @@ -9448,7 +9464,12 @@ fn (mut g Gen) lock_expr(node ast.LockExpr) { g.mtxs = '' } g.writeln('/*lock*/ {') + // This tmp var is local to the lock expr; don't inherit an enclosing if/match's + // `.ret_arr` decision (only if/match set this flag). + prev_ret_arr := g.if_match_tmp_is_fn_ret_arr + g.if_match_tmp_is_fn_ret_arr = none g.stmts_with_tmp_var(node.stmts, tmp_result) + g.if_match_tmp_is_fn_ret_arr = prev_ret_arr if node.is_expr { g.writeln(';') } diff --git a/vlib/v/gen/c/if.v b/vlib/v/gen/c/if.v index 5bad88431..8e8debc60 100644 --- a/vlib/v/gen/c/if.v +++ b/vlib/v/gen/c/if.v @@ -349,6 +349,16 @@ fn (mut g Gen) if_expr(node ast.IfExpr) { } resolved_sym := g.table.final_sym(resolved_typ) mut styp := g.styp(resolved_typ) + // When the tmp var is a fn-returned fixed array, it is a wrapper struct whose + // data lives in a `.ret_arr` member. Flag it so every branch writes through + // `.ret_arr`, even ones whose own expr type lacks the `is_fn_ret` flag (e.g. a + // fixed array literal mixed with a function call: `if c { fa() } else { [1]! }`). + prev_if_match_tmp_is_fn_ret_arr := g.if_match_tmp_is_fn_ret_arr + g.if_match_tmp_is_fn_ret_arr = resolved_sym.info is ast.ArrayFixed + && resolved_sym.info.is_fn_ret + defer(fn) { + g.if_match_tmp_is_fn_ret_arr = prev_if_match_tmp_is_fn_ret_arr + } if (g.inside_if_option || node_typ.has_flag(.option)) && !g.inside_or_block { raw_state = g.inside_if_option if resolved_node_typ != ast.void_type { @@ -788,6 +798,12 @@ fn (mut g Gen) if_expr(node ast.IfExpr) { g.empty_line = false g.writeln('\t${exit_label}: {};') g.set_current_pos_as_last_stmt_pos() - g.write('${cur_line}${tmp}') + // A fn-returned fixed array tmp var is a wrapper struct; read its data through + // `.ret_arr` (mirrors how match exprs emit the tmp var, see match.v). + if g.if_match_tmp_is_fn_ret_arr or { false } { + g.write('${cur_line}${tmp}.ret_arr') + } else { + g.write('${cur_line}${tmp}') + } } } diff --git a/vlib/v/gen/c/match.v b/vlib/v/gen/c/match.v index e9896ae85..98e672e59 100644 --- a/vlib/v/gen/c/match.v +++ b/vlib/v/gen/c/match.v @@ -65,6 +65,20 @@ fn (mut g Gen) match_expr(node ast.MatchExpr) { need_tmp_var := g.need_tmp_var_in_match(node) is_expr := (node.is_expr && resolved_return_type != ast.void_type) || g.inside_ternary > 0 + // When the tmp var is a fn-returned fixed array, it is a wrapper struct whose data + // lives in a `.ret_arr` member. Flag it so every branch writes through `.ret_arr`, + // even ones whose own expr type lacks the `is_fn_ret` flag (e.g. a fixed array + // literal mixed with a function call returning the same fixed array type). + prev_if_match_tmp_is_fn_ret_arr := g.if_match_tmp_is_fn_ret_arr + // Mirror the wrapper-struct check used when emitting the tmp var below, so every + // branch agrees with how the result is finally read. + ret_arr_sym := g.table.sym(g.unwrap_generic(g.recheck_concrete_type(resolved_return_type))) + g.if_match_tmp_is_fn_ret_arr = need_tmp_var && ret_arr_sym.info is ast.ArrayFixed + && ret_arr_sym.info.is_fn_ret + defer { + g.if_match_tmp_is_fn_ret_arr = prev_if_match_tmp_is_fn_ret_arr + } + mut cond_var := '' mut tmp_var := '' mut cur_line := '' diff --git a/vlib/v/tests/return_fixed_array_test.v b/vlib/v/tests/return_fixed_array_test.v index 5e01f9672..4b23b7b5f 100644 --- a/vlib/v/tests/return_fixed_array_test.v +++ b/vlib/v/tests/return_fixed_array_test.v @@ -112,3 +112,70 @@ fn test_fixed_array_return_from_large_enum_match_call_branches() { assert issue_fixed_array_literal_return_from_enum_match(.two) == [u8(5), 6, 7, 8]! assert issue_fixed_array_literal_return_from_enum_match(.six) == [u8(21), 22, 23, 24]! } + +// for issue 27457: an `if`/`match` expression returning a fixed array, where the +// branches mix a function call with a fixed array literal, generated invalid C +// (a struct passed where the `.ret_arr` member was expected, and vice versa). +fn issue_27457_fa() [3]int { + return [1, 2, 3]! +} + +fn issue_27457_ret_if(c bool) [3]int { + return if c { issue_27457_fa() } else { [9, 9, 9]! } +} + +fn issue_27457_ret_if_rev(c bool) [3]int { + return if c { [9, 9, 9]! } else { issue_27457_fa() } +} + +fn issue_27457_ret_if_chain(n int) [3]int { + return if n == 0 { + issue_27457_fa() + } else if n == 1 { + [5, 5, 5]! + } else { + [7, 7, 7]! + } +} + +fn issue_27457_ret_match(c bool) [3]int { + return match c { + true { issue_27457_fa() } + else { [9, 9, 9]! } + } +} + +fn issue_27457_ret_match_multi(n int) [3]int { + return match n { + 0 { issue_27457_fa() } + 1 { [5, 5, 5]! } + else { [7, 7, 7]! } + } +} + +fn issue_27457_assign(c bool) [3]int { + x := if c { issue_27457_fa() } else { [4, 4, 4]! } + return x +} + +fn test_fixed_array_return_from_mixed_if_match_branches() { + assert issue_27457_ret_if(true) == [1, 2, 3]! + assert issue_27457_ret_if(false) == [9, 9, 9]! + + assert issue_27457_ret_if_rev(true) == [9, 9, 9]! + assert issue_27457_ret_if_rev(false) == [1, 2, 3]! + + assert issue_27457_ret_if_chain(0) == [1, 2, 3]! + assert issue_27457_ret_if_chain(1) == [5, 5, 5]! + assert issue_27457_ret_if_chain(2) == [7, 7, 7]! + + assert issue_27457_ret_match(true) == [1, 2, 3]! + assert issue_27457_ret_match(false) == [9, 9, 9]! + + assert issue_27457_ret_match_multi(0) == [1, 2, 3]! + assert issue_27457_ret_match_multi(1) == [5, 5, 5]! + assert issue_27457_ret_match_multi(2) == [7, 7, 7]! + + assert issue_27457_assign(true) == [1, 2, 3]! + assert issue_27457_assign(false) == [4, 4, 4]! +} -- 2.39.5