From 7d4e37c8652fee1885ef0b805cc4688555efe619 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 25 May 2026 19:33:58 +0300 Subject: [PATCH] checker, cgen: fix returning ?T/!T into ?Alias/!Alias and container variants (fix #27056) (#27264) --- vlib/builtin/chan_option_result.v | 11 +++ vlib/v/ast/table.v | 35 ++++++++ vlib/v/checker/check_types.v | 19 +++++ vlib/v/checker/return.v | 32 ++++++- vlib/v/gen/c/assign.v | 40 +++++++++ vlib/v/gen/c/cgen.v | 29 ++++++- vlib/v/markused/walker.v | 1 + ...rohibit_redeclaration_of_builtin_types.out | 12 +-- .../v/tests/option_result_alias_fn_ret_test.v | 83 +++++++++++++++++++ 9 files changed, 249 insertions(+), 13 deletions(-) create mode 100644 vlib/v/tests/option_result_alias_fn_ret_test.v diff --git a/vlib/builtin/chan_option_result.v b/vlib/builtin/chan_option_result.v index 35c66b7d5..7661aec05 100644 --- a/vlib/builtin/chan_option_result.v +++ b/vlib/builtin/chan_option_result.v @@ -54,6 +54,17 @@ fn _result_ok(data voidptr, mut res _result, size int) { } } +fn _result_clone(current &_result, mut res _result, size int) { + unsafe { + *res = _result{ + is_error: current.is_error + err: current.err + } + // use err to get the end of ResultBase and then memcpy into it + vmemcpy(&u8(&res.err) + sizeof(IError), &u8(¤t.err) + sizeof(IError), size) + } +} + // str returns the message of IError. pub fn (err IError) str() string { if err is None__ { diff --git a/vlib/v/ast/table.v b/vlib/v/ast/table.v index 10e3fbbab..4699cc3cd 100644 --- a/vlib/v/ast/table.v +++ b/vlib/v/ast/table.v @@ -1075,6 +1075,41 @@ pub fn (t &Table) unaliased_type(typ Type) Type { return typ } +// are_payloads_alias_compatible reports whether two types describe the same +// in-memory payload after fully unaliasing both sides, including recursively +// through arrays/fixed arrays/maps. It does NOT permit numeric promotions +// or any other shape change — meant for checks that need true layout +// equivalence (e.g. gating result/option memcpy-based clones, or checking +// that `!Alias <- !T` is valid when `type Alias = T`). +pub fn (t &Table) are_payloads_alias_compatible(a Type, b Type) bool { + if a == b { + return true + } + a_unaliased := t.fully_unaliased_type(a) + b_unaliased := t.fully_unaliased_type(b) + if a_unaliased == b_unaliased { + return true + } + a_sym := t.sym(a_unaliased) + b_sym := t.sym(b_unaliased) + if a_sym.kind != b_sym.kind { + return false + } + if a_sym.info is Array && b_sym.info is Array { + return a_sym.info.nr_dims == b_sym.info.nr_dims + && t.are_payloads_alias_compatible(a_sym.info.elem_type, b_sym.info.elem_type) + } + if a_sym.info is ArrayFixed && b_sym.info is ArrayFixed { + return a_sym.info.size == b_sym.info.size + && t.are_payloads_alias_compatible(a_sym.info.elem_type, b_sym.info.elem_type) + } + if a_sym.info is Map && b_sym.info is Map { + return t.are_payloads_alias_compatible(a_sym.info.key_type, b_sym.info.key_type) + && t.are_payloads_alias_compatible(a_sym.info.value_type, b_sym.info.value_type) + } + return false +} + // fully_unaliased_type unwraps alias chains while preserving pointer indirections and flags. @[inline] pub fn (t &Table) fully_unaliased_type(typ Type) Type { diff --git a/vlib/v/checker/check_types.v b/vlib/v/checker/check_types.v index 3b978b723..512c8b4cf 100644 --- a/vlib/v/checker/check_types.v +++ b/vlib/v/checker/check_types.v @@ -579,6 +579,25 @@ fn (mut c Checker) check_basic(got ast.Type, expected ast.Type) bool { if c.table.type_to_str(got) == c.table.type_to_str(expected).trim('&') { return true } + // `[]Alias` ↔ `[]Parent` when `Alias = Parent` (and the same for maps/fixed arrays). + // `check_basic` is otherwise blind to aliases nested inside container types. + if got_sym.kind == .array && got_sym.info is ast.Array && exp_sym.info is ast.Array { + if got_sym.info.nr_dims == exp_sym.info.nr_dims + && c.table.fully_unaliased_type(got_sym.info.elem_type) == c.table.fully_unaliased_type(exp_sym.info.elem_type) { + return true + } + } else if got_sym.kind == .array_fixed && got_sym.info is ast.ArrayFixed + && exp_sym.info is ast.ArrayFixed { + if got_sym.info.size == exp_sym.info.size + && c.table.fully_unaliased_type(got_sym.info.elem_type) == c.table.fully_unaliased_type(exp_sym.info.elem_type) { + return true + } + } else if got_sym.kind == .map && got_sym.info is ast.Map && exp_sym.info is ast.Map { + if c.table.fully_unaliased_type(got_sym.info.key_type) == c.table.fully_unaliased_type(exp_sym.info.key_type) + && c.table.fully_unaliased_type(got_sym.info.value_type) == c.table.fully_unaliased_type(exp_sym.info.value_type) { + return true + } + } } if !unalias_got.is_ptr() && got_sym.kind == .array_fixed && unalias_expected.is_any_kind_of_pointer() { diff --git a/vlib/v/checker/return.v b/vlib/v/checker/return.v index 4e0904891..ebbe47b54 100644 --- a/vlib/v/checker/return.v +++ b/vlib/v/checker/return.v @@ -23,6 +23,18 @@ fn (mut c Checker) error_unaliased_type_name(exp_type ast.Type) string { return c.error_type_name(c.table.unaliased_type(exp_type)) } +// returns_call_like_result_expr reports whether `expr` is a `CallExpr`, +// possibly wrapped in redundant `ParExpr`s. Used to decide whether the +// return path is shaped like `return foo()` / `return (foo())` — the +// shapes for which cgen can emit a `_result_*` alias clone. +fn returns_call_like_result_expr(expr ast.Expr) bool { + mut e := expr + for e is ast.ParExpr { + e = e.expr + } + return e is ast.CallExpr +} + // TODO: non deferred fn (mut c Checker) return_stmt(mut node ast.Return) { if c.table.cur_fn == unsafe { nil } { @@ -221,10 +233,22 @@ fn (mut c Checker) return_stmt(mut node ast.Return) { c.error('cannot use `${c.table.type_to_str(got_type)}` as ${c.error_type_name(exp_type)} in return argument', exprv.pos()) } - if got_type.has_flag(.result) && (!exp_type.has_flag(.result) - || c.table.type_to_str(got_type) != c.table.type_to_str(exp_type)) { - c.error('cannot use `${c.table.type_to_str(got_type)}` as ${c.error_type_name(exp_type)} in return argument', - exprv.pos()) + if got_type.has_flag(.result) { + // Accept payload-equivalent aliases only when the expression is a + // call (optionally wrapped in `()`); the cgen alias clone path only + // handles bare calls today. For anything else, fall back to a + // strict identity check so cgen never emits a struct cast between + // distinct `_result_*` types (which C rejects). + payload_compat := if returns_call_like_result_expr(exprv) { + c.table.are_payloads_alias_compatible(got_type.clear_flag(.result), + exp_type.clear_flag(.result)) + } else { + got_type.clear_flag(.result) == exp_type.clear_flag(.result) + } + if !exp_type.has_flag(.result) || !payload_compat { + c.error('cannot use `${c.table.type_to_str(got_type)}` as ${c.error_type_name(exp_type)} in return argument', + exprv.pos()) + } } if exprv is ast.ComptimeCall && exprv.kind == .tmpl && c.table.final_sym(exp_type).kind != .string { diff --git a/vlib/v/gen/c/assign.v b/vlib/v/gen/c/assign.v index 92ae7a12d..17d059f73 100644 --- a/vlib/v/gen/c/assign.v +++ b/vlib/v/gen/c/assign.v @@ -427,6 +427,46 @@ fn (mut g Gen) expr_opt_with_alias(expr ast.Expr, expr_typ ast.Type, ret_typ ast return ret_var } +// expr_result_with_alias handles conversion from different result alias type name +fn (mut g Gen) expr_result_with_alias(expr ast.Expr, expr_typ ast.Type, ret_typ ast.Type) string { + styp := g.base_type(ret_typ) + + line := g.go_before_last_stmt().trim_space() + g.empty_line = true + + ret_var := g.new_tmp_var() + ret_styp := g.styp(ret_typ).replace('*', '_ptr') + g.writeln('${ret_styp} ${ret_var} = {.is_error=false, .err=_const_none__, .data={E_STRUCT}};') + + is_result_expr := expr_typ.has_flag(.result) + if is_result_expr { + g.write('builtin___result_clone((${result_name}*)') + } else { + g.write('builtin___result_ok(&(${styp}[]){ ') + } + has_addr := is_result_expr && expr !in [ast.Ident, ast.SelectorExpr] + if has_addr { + expr_styp := g.styp(expr_typ).replace('*', '_ptr') + g.write('ADDR(${expr_styp}, ') + } else if is_result_expr { + g.write('&') + } + g.expr(expr) + if has_addr { + g.write(')') + } + if !is_result_expr { + g.write(' }') + } + g.writeln(', (${result_name}*)&${ret_var}, sizeof(${styp}));') + g.write(line) + if g.inside_return { + g.write(' ') + } + g.write(ret_var) + return ret_var +} + // expr_opt_with_cast is used in cast expr when converting compatible option types // e.g. ?int(?u8(0)) fn (mut g Gen) expr_opt_with_cast(expr ast.Expr, expr_typ ast.Type, ret_typ ast.Type) string { diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index 20e43a986..d8a539160 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -10698,14 +10698,26 @@ fn (mut g Gen) return_stmt(node ast.Return) { } } else { if ret_type.has_flag(.option) { - expr0_is_alias_fn_ret := expr0 is ast.CallExpr && type0.has_flag(.option) + inner_expr := unwrap_paren_call_expr(expr0) + expr0_is_alias_fn_ret := inner_expr is ast.CallExpr && type0.has_flag(.option) && g.table.type_kind(type0) in [.placeholder, .alias] - // return foo() where foo() returns different option alias than current fn + // return foo() (or `return (foo())`) where foo() returns a + // different option alias than the current fn. if expr0_is_alias_fn_ret { - g.expr_opt_with_cast(expr0, type0, ret_type) + g.expr_opt_with_cast(inner_expr, type0, ret_type) } else { g.expr_with_opt(expr0, type0, ret_type) } + } else if ret_type.has_flag(.result) && type0.has_flag(.result) + && type0 != ret_type + && unwrap_paren_call_expr(expr0) is ast.CallExpr + && g.table.are_payloads_alias_compatible(type0.clear_flag(.result), + ret_type.clear_flag(.result)) { + // return foo() (or `return (foo())`) where foo() returns a different + // but layout-equivalent result alias (e.g. `!Aa` vs `!Bb` with + // `type Aa = Bb`, or `![]Aa` vs `![]Bb`). The two C structs are + // distinct, so emit a memcpy-based clone. + g.expr_result_with_alias(unwrap_paren_call_expr(expr0), type0, ret_type) } else { if fn_return_is_fixed_array && !type0.has_option_or_result() { if node.exprs[0] is ast.Ident { @@ -10768,6 +10780,17 @@ fn (mut g Gen) return_stmt(node ast.Return) { } } +// unwrap_paren_call_expr strips redundant `ParExpr` wrappers from `expr` and +// returns the inner expression. Used by the result-alias return path to +// recognize `return (foo())` as equivalent to `return foo()`. +fn unwrap_paren_call_expr(expr ast.Expr) ast.Expr { + mut e := expr + for e is ast.ParExpr { + e = e.expr + } + return e +} + // check_expr_is_const checks if the expr is eligible to be used as const initializer on C global scope fn (mut g Gen) check_expr_is_const(expr ast.Expr) bool { match expr { diff --git a/vlib/v/markused/walker.v b/vlib/v/markused/walker.v index 9627c96d1..aa5d63a99 100644 --- a/vlib/v/markused/walker.v +++ b/vlib/v/markused/walker.v @@ -3941,6 +3941,7 @@ pub fn (mut w Walker) finalize(include_panic_deps bool) { } if w.used_result > 0 { w.fn_by_name('_result_ok') + w.fn_by_name('_result_clone') w.mark_by_sym_name('_result') } if (w.used_option + w.used_result + w.used_none) > 0 { diff --git a/vlib/v/parser/tests/prohibit_redeclaration_of_builtin_types.out b/vlib/v/parser/tests/prohibit_redeclaration_of_builtin_types.out index 2800d404b..00b5f376e 100644 --- a/vlib/v/parser/tests/prohibit_redeclaration_of_builtin_types.out +++ b/vlib/v/parser/tests/prohibit_redeclaration_of_builtin_types.out @@ -1,10 +1,10 @@ vlib/v/parser/tests/prohibit_redeclaration_of_builtin_types.vv:1:8: error: cannot register struct `Option`, another type with this name exists 1 | struct Option {} | ~~~~~~ -Details: vlib/builtin/chan_option_result.v:137:8: details: another declaration was found here - 135 | - 136 | // Option is the base of V's internal option return system. - 137 | struct Option { +Details: vlib/builtin/chan_option_result.v:148:8: details: another declaration was found here + 146 | + 147 | // Option is the base of V's internal option return system. + 148 | struct Option { | ~~~~~~ - 138 | state u8 // 0 - ok; 2 - none; 1 - ? - 139 | err IError = none__ + 149 | state u8 // 0 - ok; 2 - none; 1 - ? + 150 | err IError = none__ diff --git a/vlib/v/tests/option_result_alias_fn_ret_test.v b/vlib/v/tests/option_result_alias_fn_ret_test.v new file mode 100644 index 000000000..aa8a83ccd --- /dev/null +++ b/vlib/v/tests/option_result_alias_fn_ret_test.v @@ -0,0 +1,83 @@ +// Regression test for https://github.com/vlang/v/issues/27056 and +// https://github.com/vlang/v/issues/27006: returning `?T` / `!T` from a +// function whose return type is `?Alias` / `!Alias` (where `type Alias = T`) +// must compile, both for the direct alias case and for containers (`?[]Alias`, +// `![]Alias`, etc.). + +struct Bb { + s string +} + +type Aa = Bb + +fn returns_b_option() ?Bb { + return Bb{s: 'b'} +} + +fn returns_b_result() !Bb { + return Bb{s: 'b'} +} + +fn returns_bs_option() ?[]Bb { + return [Bb{s: 'b'}] +} + +fn returns_bs_result() ![]Bb { + return [Bb{s: 'b'}] +} + +fn case_a() !Aa { + return returns_b_result() +} + +fn case_b() ?Aa { + return returns_b_option() +} + +fn case_c() ?[]Aa { + return returns_bs_option() +} + +fn case_d() ![]Aa { + return returns_bs_result() +} + +fn case_e() !Aa { + return (returns_b_result()) +} + +fn case_f() ?Aa { + return (returns_b_option()) +} + +fn test_result_alias_direct() { + a := case_a() or { panic(err) } + assert a.s == 'b' +} + +fn test_option_alias_direct() { + b := case_b() or { panic(err) } + assert b.s == 'b' +} + +fn test_option_alias_array() { + c := case_c() or { panic(err) } + assert c.len == 1 + assert c[0].s == 'b' +} + +fn test_result_alias_array() { + d := case_d() or { panic(err) } + assert d.len == 1 + assert d[0].s == 'b' +} + +fn test_result_alias_paren_wrapped() { + e := case_e() or { panic(err) } + assert e.s == 'b' +} + +fn test_option_alias_paren_wrapped() { + f := case_f() or { panic('none') } + assert f.s == 'b' +} -- 2.39.5