diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 61a7c92af6f671..b667186241b80c 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -108,6 +108,7 @@ pub mut: inside_ct_attr bool // true inside `[if expr]` inside_x_is_type bool // true inside the Type expression of `if x is Type {` inside_x_matches_type bool // true inside the match branch of `match x.type { Type {} }` + force_value_match_or_if bool // force a value `match`/`if` in a value-required void context (array element, map value, prefix operand) to be an expression, e.g. `[match x {..}]`, `{'k': match x {..}}`, `-(match x {..})` anon_struct_should_be_mut bool // true when `mut var := struct { ... }` is used inside_generic_struct_init bool inside_integer_literal_cast bool // true inside `int(123)` @@ -3080,7 +3081,19 @@ fn (mut c Checker) selector_expr(mut node ast.SelectorExpr) ast.Type { node.is_field_typ = node.is_field_typ || c.comptime.is_comptime_selector_type(node) old_selector_expr := c.inside_selector_expr c.inside_selector_expr = true + // A value `match`/`if` selector receiver, e.g. `(match x { ... }).field`, in a + // void context (nested in an if-branch) must be checked as an expression so its + // arms produce values, instead of being typed `void` ("does not return a value"). + mut restore_force_value := false + if c.expected_type == ast.void_type && !c.force_value_match_or_if + && operand_is_value_match_or_if(node.expr) { + c.force_value_match_or_if = true + restore_force_value = true + } mut typ := c.expr(mut node.expr) + if restore_force_value { + c.force_value_match_or_if = false + } expr_is_auto_deref_var := node.expr.is_auto_deref_var() receiver_uses_wrapped_smartcast := typ.has_option_or_result() || c.table.sym(c.unwrap_generic(typ)).kind in [.interface, .sum_type, .any] @@ -4867,7 +4880,18 @@ pub fn (mut c Checker) expr(mut node ast.Expr) ast.Type { return c.array_init(mut node) } ast.AsCast { - node.expr_type = c.expr(mut node.expr) + if c.expected_type == ast.void_type && operand_is_value_match_or_if(node.expr) { + // A `match`/`if` operand of an `as` cast is a value expression, e.g. + // `(match x { ... }) as Variant`. Give it a non-void expected type so + // it is checked as an expression (`is_expr`) even when nested in a + // void context (e.g. an if-branch), instead of being typed as `void`. + old_expected_type := c.expected_type + c.expected_type = node.typ + node.expr_type = c.expr(mut node.expr) + c.expected_type = old_expected_type + } else { + node.expr_type = c.expr(mut node.expr) + } expr_type_sym := c.table.sym(node.expr_type) type_sym := c.table.sym(c.unwrap_generic(node.typ)) if mut node.expr is ast.Ident { @@ -5281,7 +5305,19 @@ pub fn (mut c Checker) expr(mut node ast.Expr) ast.Type { return c.unsafe_expr(mut node) } ast.Likely { + // A value `match`/`if` operand, e.g. `_likely_(match x { ... })`, in a + // void context (nested in an if-branch) must be checked as an expression + // so its arms produce values, instead of being typed `void`. + mut restore_force_value := false + if c.expected_type == ast.void_type && !c.force_value_match_or_if + && operand_is_value_match_or_if(node.expr) { + c.force_value_match_or_if = true + restore_force_value = true + } ltype := c.expr(mut node.expr) + if restore_force_value { + c.force_value_match_or_if = false + } if !c.check_types(ltype, ast.bool_type) { ltype_sym := c.table.sym(ltype) lname := if node.is_likely { '_likely_' } else { '_unlikely_' } @@ -5414,6 +5450,23 @@ fn integer_literal_from_pointer_cast_expr(expr ast.Expr) ?ast.IntegerLiteral { } } +// operand_is_value_match_or_if reports whether an expression is a `match`/`if` +// expression used as a value, looking through transparent `(...)` and +// `unsafe { }` wrappers (including compositions like `unsafe { match ... }`). +// Such an operand of a cast or infix expression must be checked with a non-void +// expected type so it is treated as `is_expr`, even when the surrounding expected +// type is void (e.g. nested inside an if-branch) — otherwise it is mistyped as +// `void`. +fn operand_is_value_match_or_if(expr ast.Expr) bool { + if expr is ast.ParExpr { + return operand_is_value_match_or_if(expr.expr) + } + if expr is ast.UnsafeExpr { + return operand_is_value_match_or_if(expr.expr) + } + return expr is ast.MatchExpr || expr is ast.IfExpr +} + fn (mut c Checker) cast_expr(mut node ast.CastExpr) ast.Type { // Given: `Outside( Inside(xyz) )`, // node.expr_type: `Inside` @@ -5454,6 +5507,13 @@ fn (mut c Checker) cast_expr(mut node ast.CastExpr) ast.Type { c.expected_type = base_to_type } else if node.expr is ast.IndexExpr && to_type.has_flag(.option) { c.expected_type = to_type + } else if c.expected_type == ast.void_type && operand_is_value_match_or_if(node.expr) { + // A `match`/`if` operand of a cast is a value expression, e.g. + // `i64(match x { ... })`. Propagate the cast target as its expected type + // so it is checked as an expression (`is_expr`) even in contexts where + // the surrounding expected type is void (e.g. nested inside an if-branch), + // instead of being mistyped as `void` ("does not return a value"). + c.expected_type = base_to_type } expr_is_ident_or_cast := node.expr is ast.Ident || node.expr is ast.CastExpr node.expr_type = c.expr(mut node.expr) // type to be casted @@ -6932,7 +6992,19 @@ fn (mut c Checker) check_known_struct_name(ident ast.Ident) ? { fn (mut c Checker) concat_expr(mut node ast.ConcatExpr) ast.Type { mut mr_types := []ast.Type{} for mut expr in node.vals { + // A value `match`/`if` multi-return value, e.g. `match x { ... }, 9`, in a + // void context (nested in an if-branch) must be checked as an expression so + // its arms produce values, instead of being typed `void`. + mut restore_force_value := false + if c.expected_type == ast.void_type && !c.force_value_match_or_if + && operand_is_value_match_or_if(expr) { + c.force_value_match_or_if = true + restore_force_value = true + } mut typ := c.expr(mut expr) + if restore_force_value { + c.force_value_match_or_if = false + } if typ == ast.nil_type { // nil and voidptr produces the same struct type name typ = ast.voidptr_type @@ -7903,7 +7975,19 @@ fn (mut c Checker) get_base_name(node &ast.Expr) string { fn (mut c Checker) prefix_expr(mut node ast.PrefixExpr) ast.Type { old_inside_ref_lit := c.inside_ref_lit c.inside_ref_lit = c.inside_ref_lit || node.op == .amp + // A value `match`/`if` prefix operand, e.g. `-(match x { ... })`, in a void + // context (nested in an if-branch) must be checked as an expression so its arms + // produce values, instead of being typed `void` ("value after `-` is void"). + mut restore_force_value := false + if c.expected_type == ast.void_type && !c.force_value_match_or_if + && operand_is_value_match_or_if(node.right) { + c.force_value_match_or_if = true + restore_force_value = true + } right_type := c.expr(mut node.right) + if restore_force_value { + c.force_value_match_or_if = false + } c.inside_ref_lit = old_inside_ref_lit node.right_type = right_type mut expr := node.right @@ -8459,12 +8543,33 @@ fn (mut c Checker) index_expr(mut node ast.IndexExpr) ast.Type { } } if mut node.index is ast.RangeExpr { // [1..2] + // A value `match`/`if` range bound, e.g. `values[(match x { ... })..]`, in a + // void context (nested in an if-branch) must be checked as an expression so + // its arms produce values, instead of being typed `void`. if node.index.has_low { + mut restore_low := false + if c.expected_type == ast.void_type && !c.force_value_match_or_if + && operand_is_value_match_or_if(node.index.low) { + c.force_value_match_or_if = true + restore_low = true + } index_type := c.expr(mut node.index.low) + if restore_low { + c.force_value_match_or_if = false + } c.check_index(typ_sym, node.index.low, index_type, true, node.is_gated) } if node.index.has_high { + mut restore_high := false + if c.expected_type == ast.void_type && !c.force_value_match_or_if + && operand_is_value_match_or_if(node.index.high) { + c.force_value_match_or_if = true + restore_high = true + } index_type := c.expr(mut node.index.high) + if restore_high { + c.force_value_match_or_if = false + } c.check_index(typ_sym, node.index.high, index_type, true, node.is_gated) } // array[1..2] => array @@ -8509,7 +8614,20 @@ fn (mut c Checker) index_expr(mut node ast.IndexExpr) ast.Type { c.warn('`or {}` block required when indexing a map with sum type value', node.pos) } } else { + // A value `match`/`if` index, e.g. `values[match x { ... }]`, in a void + // context (nested in an if-branch) must be checked as an expression so + // its arms produce values, instead of being typed `void` ("non-integer + // index `void`"). + mut restore_force_value := false + if c.expected_type == ast.void_type && !c.force_value_match_or_if + && operand_is_value_match_or_if(node.index) { + c.force_value_match_or_if = true + restore_force_value = true + } index_type := c.expr(mut node.index) + if restore_force_value { + c.force_value_match_or_if = false + } if node.is_gated && (typ.is_ptr() || typ.is_pointer() || typ_sym.kind !in [.array, .array_fixed, .string]) { c.error('`#[]` negative indexing is only supported for arrays, fixed arrays, and strings', diff --git a/vlib/v/checker/containers.v b/vlib/v/checker/containers.v index 44ea73e9a0e64d..842eaf35109f0e 100644 --- a/vlib/v/checker/containers.v +++ b/vlib/v/checker/containers.v @@ -409,8 +409,20 @@ fn (mut c Checker) array_init(mut node ast.ArrayInit) ast.Type { } if node.has_update_expr { - // `[...base, e1, e2]` — array update/spread literal + // `[...base, e1, e2]` — array update/spread literal. + // A value `match`/`if` spread operand, e.g. `[...(match x { ... })]`, in a + // void context (nested in an if-branch) must be checked as an expression so + // its arms produce values, instead of being typed `void`. + mut restore_force_value := false + if c.expected_type == ast.void_type && !c.force_value_match_or_if + && operand_is_value_match_or_if(node.update_expr) { + c.force_value_match_or_if = true + restore_force_value = true + } update_typ := c.expr(mut node.update_expr) + if restore_force_value { + c.force_value_match_or_if = false + } // Resolve through type aliases so `type Ints = []int; [...Ints(...)]` // is accepted; use final_sym to look past aliases of arrays. update_sym := c.table.final_sym(update_typ) @@ -507,7 +519,22 @@ fn (mut c Checker) array_init(mut node ast.ArrayInit) ast.Type { expr_pos) continue } + // A value `match`/`if` array element, e.g. `[match x { ... }]`, in a + // void context (e.g. nested in an if-branch) must be checked as an + // expression (`is_expr`) so its arms produce values and infer the + // element type, instead of being lowered as void statements. Signal + // that via a flag rather than a forced expected type, which would + // mistype the arms. + mut restore_array_elem_flag := false + if c.expected_type == ast.void_type && !c.force_value_match_or_if + && operand_is_value_match_or_if(expr) { + c.force_value_match_or_if = true + restore_array_elem_flag = true + } typ = c.check_expr_option_or_result_call(expr, c.expr(mut expr)) + if restore_array_elem_flag { + c.force_value_match_or_if = false + } sym := c.table.sym(expected_value_type) if sym.kind == .interface { c.type_implements(typ, expected_value_type, expr.pos()) @@ -911,7 +938,19 @@ fn (mut c Checker) map_init(mut node ast.MapInit) ast.Type { map_type = c.expected_type } if node.has_update_expr { + // A value `match`/`if` map update operand, e.g. `{ ...(match x { .. }), k: v }`, + // in a void context must be checked as an expression so its arms produce + // values, instead of being typed `void` ("non-map type"). + mut restore_force_value := false + if map_type == ast.void_type && c.expected_type == ast.void_type + && !c.force_value_match_or_if && operand_is_value_match_or_if(node.update_expr) { + c.force_value_match_or_if = true + restore_force_value = true + } update_type := c.expr(mut node.update_expr) + if restore_force_value { + c.force_value_match_or_if = false + } if map_type != ast.void_type { if update_type != map_type { msg := c.expected_msg(update_type, map_type) @@ -934,12 +973,39 @@ fn (mut c Checker) map_init(mut node ast.MapInit) ast.Type { } else if node.keys.len > 0 { // `{'age': 20}` mut key_ := node.keys[0] + // A value `match`/`if` map key, e.g. `{(match x { ... }): v}`, in a void + // context determines the map key type and must be checked as an + // expression (like the value below). Same flag mechanism. + mut restore_key_flag := false + if c.expected_type == ast.void_type && !c.force_value_match_or_if + && operand_is_value_match_or_if(key_) { + c.force_value_match_or_if = true + restore_key_flag = true + } map_key_type = ast.mktyp(c.expr(mut key_)) + if restore_key_flag { + c.force_value_match_or_if = false + } if node.keys[0].is_auto_deref_var() { map_key_type = map_key_type.deref() } mut val_ := node.vals[0] + // A value `match`/`if` map value, e.g. `{'k': match x { ... }}`, in a + // void context (e.g. nested in an if-branch) determines the map value + // type and must be checked as an expression (`is_expr`) so its arms + // produce values, instead of being lowered as void statements. Signal + // that via a flag (leaving the expected type void so the arms infer + // their own type), consumed by `match_expr`/`if_expr`. + mut restore_container_flag := false + if c.expected_type == ast.void_type && !c.force_value_match_or_if + && operand_is_value_match_or_if(val_) { + c.force_value_match_or_if = true + restore_container_flag = true + } map_val_type = ast.mktyp(c.expr(mut val_)) + if restore_container_flag { + c.force_value_match_or_if = false + } if node.vals[0].is_auto_deref_var() { map_val_type = map_val_type.deref() } diff --git a/vlib/v/checker/if.v b/vlib/v/checker/if.v index e3ad0bd4edb452..1d6f1ab020aa4b 100644 --- a/vlib/v/checker/if.v +++ b/vlib/v/checker/if.v @@ -115,6 +115,12 @@ fn (mut c Checker) gen_branch_context_string() string { fn (mut c Checker) if_expr(mut node ast.IfExpr) ast.Type { if_kind := if node.is_comptime { '\$if' } else { 'if' } + // Consume the value-required flag so it applies only to this outer node, not to + // nested statement-level match/if inside the branches. A value `if` in a + // value-required void context (`[if ...]`, `{'k': if ...}`, `-(if ...)`) must + // still be treated as an expression. + force_value := c.force_value_match_or_if + c.force_value_match_or_if = false mut node_is_expr := false if node.branches.len > 0 && node.has_else { stmts := node.branches[0].stmts @@ -122,6 +128,8 @@ fn (mut c Checker) if_expr(mut node ast.IfExpr) ast.Type { node_is_expr = true } else if node.is_expr { node_is_expr = true + } else if force_value { + node_is_expr = true } } if c.expected_type == ast.void_type && node_is_expr { diff --git a/vlib/v/checker/infix.v b/vlib/v/checker/infix.v index 3395f9cbedbc45..4172053c2409dc 100644 --- a/vlib/v/checker/infix.v +++ b/vlib/v/checker/infix.v @@ -113,20 +113,20 @@ fn (mut c Checker) infix_expr(mut node ast.InfixExpr) ast.Type { } // In bool contexts like `assert` and `return`, short enum literals on the left // need the right operand type first, so `.a == x` resolves `.a` correctly. - mut check_right_type_first_for_left_short_enum := false + mut check_right_type_first := false if node.op in [.eq, .ne] && node.left is ast.EnumVal { left_enum := node.left as ast.EnumVal if left_enum.enum_name.len == 0 { if node.right is ast.EnumVal { right_enum := node.right as ast.EnumVal - check_right_type_first_for_left_short_enum = right_enum.enum_name.len > 0 + check_right_type_first = right_enum.enum_name.len > 0 } else { - check_right_type_first_for_left_short_enum = true + check_right_type_first = true } } } mut right_type := ast.void_type - if check_right_type_first_for_left_short_enum { + if check_right_type_first { right_type = c.expr(mut node.right) if right_type == ast.no_type { node.right_type = right_type @@ -135,7 +135,22 @@ fn (mut c Checker) infix_expr(mut node ast.InfixExpr) ast.Type { node.right_type = right_type c.expected_type = right_type } + // A `match`/`if` value operand on the left, e.g. `(match x { ... }) + 1` or + // `(match x { ... }) in [1, 2]`, would otherwise be checked with the (void) + // surrounding expected type and mistyped as a statement (e.g. when nested inside + // an if-branch). Force it to be checked as a value expression so its arms infer + // their own type, without imposing the right operand's type (which for a + // membership operator is a container, not the element/key type). + mut restore_force_value := false + if !check_right_type_first && c.expected_type == ast.void_type && !c.force_value_match_or_if + && operand_is_value_match_or_if(node.left) { + c.force_value_match_or_if = true + restore_force_value = true + } mut left_type := c.expr(mut node.left) + if restore_force_value { + c.force_value_match_or_if = false + } if left_type == ast.no_type { node.left_type = left_type return ast.void_type @@ -229,7 +244,7 @@ fn (mut c Checker) infix_expr(mut node ast.InfixExpr) ast.Type { } } } - if !check_right_type_first_for_left_short_enum { + if !check_right_type_first { right_type = c.expr(mut node.right) if right_type == ast.no_type { node.right_type = right_type diff --git a/vlib/v/checker/match.v b/vlib/v/checker/match.v index 64fa8a376f5266..9d5dc840965eef 100644 --- a/vlib/v/checker/match.v +++ b/vlib/v/checker/match.v @@ -6,8 +6,16 @@ import v.token import strings fn (mut c Checker) match_expr(mut node ast.MatchExpr) ast.Type { + // `c.force_value_match_or_if` marks a value match used in a value-required + // position whose surrounding expected type is void (an array element + // `[match x { .. }]`, a map value `{'k': match x { .. }}`, or a prefix operand + // `-(match x { .. })`), which must be treated as an expression. Consume the + // flag here so it applies only to this outer node, not to nested + // statement-level match/if inside the arms. + force_value := c.force_value_match_or_if + c.force_value_match_or_if = false if !node.is_comptime { - node.is_expr = c.expected_type != ast.void_type + node.is_expr = c.expected_type != ast.void_type || force_value } node.expected_type = c.expected_type if mut node.cond is ast.ParExpr && !c.pref.translated && !c.file.is_translated { diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index 1a25d52da67200..387c2a5f077600 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -13976,6 +13976,9 @@ fn as_cast_operand_needs_tmp_eval(expr ast.Expr) bool { ast.ParExpr { as_cast_operand_needs_tmp_eval(expr.expr) } + ast.UnsafeExpr { + as_cast_operand_needs_tmp_eval(expr.expr) + } ast.SelectorExpr { as_cast_operand_needs_tmp_eval(expr.expr) } diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v index 4d9629d48370b8..5eb11b6c322c2e 100644 --- a/vlib/v/parser/parser.v +++ b/vlib/v/parser/parser.v @@ -578,71 +578,340 @@ fn (mut p Parser) parse_block_no_scope(is_top_level bool) []ast.Stmt { } fn (mut p Parser) mark_last_call_return_as_used(mut last_stmt ast.Stmt) { - match mut last_stmt { - ast.ExprStmt { - match mut last_stmt.expr { - ast.CallExpr { - // last stmt on block is CallExpr - last_stmt.expr.is_return_used = true - if last_stmt.expr.or_block.stmts.len > 0 { - mut or_block_last_stmt := last_stmt.expr.or_block.stmts.last() - p.mark_last_call_return_as_used(mut or_block_last_stmt) + if mut last_stmt is ast.ExprStmt { + p.mark_last_call_expr_return_as_used(mut last_stmt.expr) + } +} + +// expr_contains_value_match_or_if reports whether an expression is (or, through +// transparent `(...)`/`unsafe { }`/cast/`as`-cast wrappers and nested call +// arguments, contains) a `match`/`if` value expression. Used to decide whether a +// call argument holds a block-value match/if whose arm calls must be marked as +// return-used, including nested compositions like `wrap(wrap(match value { .. }))`. +fn (p &Parser) expr_contains_value_match_or_if(expr ast.Expr) bool { + return match expr { + ast.MatchExpr, ast.IfExpr { + true + } + ast.ParExpr { + p.expr_contains_value_match_or_if(expr.expr) + } + ast.UnsafeExpr { + p.expr_contains_value_match_or_if(expr.expr) + } + ast.CastExpr { + p.expr_contains_value_match_or_if(expr.expr) + } + ast.AsCast { + p.expr_contains_value_match_or_if(expr.expr) + } + ast.CallExpr { + mut found := expr.is_method && p.expr_contains_value_match_or_if(expr.left) + if !found { + for arg in expr.args { + if p.expr_contains_value_match_or_if(arg.expr) { + found = true + break } } - ast.ConcatExpr { - // last stmt on block is: a, b, c := ret1(), ret2(), ret3() - for mut expr in last_stmt.expr.vals { - if mut expr is ast.CallExpr { - expr.is_return_used = true - } + } + found + } + ast.InfixExpr { + // e.g. `1 + (match value { .. })` as a call argument. + + p.expr_contains_value_match_or_if(expr.left) + || p.expr_contains_value_match_or_if(expr.right) + } + ast.PrefixExpr { + // e.g. `-(match value { .. })` as a call argument. + p.expr_contains_value_match_or_if(expr.right) + } + ast.IndexExpr { + // e.g. `values[match value { .. }]` as a call argument. + + p.expr_contains_value_match_or_if(expr.left) + || p.expr_contains_value_match_or_if(expr.index) + } + ast.RangeExpr { + // e.g. `values[(match value { .. })..]` slice bound. + + (expr.has_low && p.expr_contains_value_match_or_if(expr.low)) + || (expr.has_high && p.expr_contains_value_match_or_if(expr.high)) + } + ast.SelectorExpr { + // e.g. `(match value { .. }).field` as a call argument. + p.expr_contains_value_match_or_if(expr.expr) + } + ast.ArrayInit { + // e.g. `[match value { .. }]` or `[...(match value { .. })]`. + mut found := expr.has_update_expr && p.expr_contains_value_match_or_if(expr.update_expr) + if !found { + for element in expr.exprs { + if p.expr_contains_value_match_or_if(element) { + found = true + break } } - ast.IfExpr { - // last stmt on block is: if .. { foo() } else { bar() } - for mut branch in last_stmt.expr.branches { - if branch.stmts.len > 0 { - mut last_if_stmt := branch.stmts.last() - p.mark_last_call_return_as_used(mut last_if_stmt) - } + } + found + } + ast.StructInit { + // e.g. `Holder{ value: match value { .. } }` or + // `Holder{ ...(match value { .. }), other: 1 }` as a call argument. + mut found := expr.has_update_expr && p.expr_contains_value_match_or_if(expr.update_expr) + if !found { + for field in expr.init_fields { + if p.expr_contains_value_match_or_if(field.expr) { + found = true + break } } - ast.InfixExpr { - if last_stmt.expr.or_block.stmts.len > 0 { - mut or_block_last_stmt := last_stmt.expr.or_block.stmts.last() - p.mark_last_call_return_as_used(mut or_block_last_stmt) - } - // last stmt has infix expr with CallExpr: foo()? + 'a' - mut left_expr := last_stmt.expr.left - for { - mut next_left_expr := ast.Expr(ast.EmptyExpr{}) - if mut left_expr is ast.InfixExpr { - if left_expr.or_block.stmts.len > 0 { - mut or_block_last_stmt := left_expr.or_block.stmts.last() - p.mark_last_call_return_as_used(mut or_block_last_stmt) - } - next_left_expr = left_expr.left - } else if mut left_expr is ast.CallExpr { - left_expr.is_return_used = true - if left_expr.or_block.stmts.len > 0 { - mut or_block_last_stmt := left_expr.or_block.stmts.last() - p.mark_last_call_return_as_used(mut or_block_last_stmt) - } - break - } else { - break - } - left_expr = next_left_expr - continue + } + found + } + ast.MapInit { + // e.g. `{'value': match value { .. }}` or + // `{ ...(match value { .. }), 'k': v }` as a call argument. + mut found := expr.has_update_expr && p.expr_contains_value_match_or_if(expr.update_expr) + if !found { + for element in expr.keys { + if p.expr_contains_value_match_or_if(element) { + found = true + break } } - ast.ComptimeCall, ast.ComptimeSelector, ast.PrefixExpr, ast.SelectorExpr { - if last_stmt.expr.or_block.stmts.len > 0 { - mut or_block_last_stmt := last_stmt.expr.or_block.stmts.last() - p.mark_last_call_return_as_used(mut or_block_last_stmt) + } + if !found { + for element in expr.vals { + if p.expr_contains_value_match_or_if(element) { + found = true + break } } - else {} } + found + } + ast.StringInterLiteral { + // e.g. `'x=${match value { .. }}'` as a call argument. + mut found := false + for e in expr.exprs { + if p.expr_contains_value_match_or_if(e) { + found = true + break + } + } + found + } + ast.DumpExpr { + // e.g. `dump(match value { .. })` as a call argument. + p.expr_contains_value_match_or_if(expr.expr) + } + ast.Likely { + // e.g. `_likely_(match value { .. })` as a call argument. + p.expr_contains_value_match_or_if(expr.expr) + } + else { + false + } + } +} + +fn (mut p Parser) mark_last_call_expr_return_as_used(mut expr ast.Expr) { + match mut expr { + ast.CallExpr { + // last stmt on block is CallExpr + expr.is_return_used = true + if expr.or_block.stmts.len > 0 { + mut or_block_last_stmt := expr.or_block.stmts.last() + p.mark_last_call_return_as_used(mut or_block_last_stmt) + } + // an argument may itself be (or nest) a block-value match/if, e.g. + // `wrap(match value { First { foo()! } })` or + // `wrap(wrap(match value { .. }))`; mark its arm calls too. + for mut arg in expr.args { + if p.expr_contains_value_match_or_if(arg.expr) { + p.mark_last_call_expr_return_as_used(mut arg.expr) + } + } + // a method-call receiver may itself be a block-value match/if, e.g. + // `(match value { First { foo()! } }).get()`; mark its arm calls too. + if expr.is_method && p.expr_contains_value_match_or_if(expr.left) { + p.mark_last_call_expr_return_as_used(mut expr.left) + } + } + ast.ConcatExpr { + // last stmt on block is a multi-return value list, e.g. + // `ret1(), ret2()` or `match value { .. }, 9`; recurse into every + // value so nested/wrapped match/if/call values are marked as + // return-used, not just direct calls. + for mut val in expr.vals { + p.mark_last_call_expr_return_as_used(mut val) + } + } + ast.ArrayInit { + // last stmt on block is an array literal, e.g. `[match value { .. }]` + // or a spread `[...(match value { .. })]`; mark any element or the + // spread operand that is (or nests) a block-value match/if. + for mut element in expr.exprs { + if p.expr_contains_value_match_or_if(element) { + p.mark_last_call_expr_return_as_used(mut element) + } + } + if expr.has_update_expr && p.expr_contains_value_match_or_if(expr.update_expr) { + p.mark_last_call_expr_return_as_used(mut expr.update_expr) + } + } + ast.StructInit { + // last stmt on block is a struct literal, e.g. + // `Holder{ value: match value { .. } }` or an update + // `Holder{ ...(match value { .. }), other: 1 }`; mark any field value + // or the update operand that is (or nests) a block-value match/if. + for mut field in expr.init_fields { + if p.expr_contains_value_match_or_if(field.expr) { + p.mark_last_call_expr_return_as_used(mut field.expr) + } + } + if expr.has_update_expr && p.expr_contains_value_match_or_if(expr.update_expr) { + p.mark_last_call_expr_return_as_used(mut expr.update_expr) + } + } + ast.MapInit { + // last stmt on block is a map literal, e.g. + // `{'value': match value { .. }}` or an update + // `{ ...(match value { .. }), 'k': v }`; mark any key/value or the + // update operand that is (or nests) a block-value match/if. + for mut element in expr.keys { + if p.expr_contains_value_match_or_if(element) { + p.mark_last_call_expr_return_as_used(mut element) + } + } + for mut element in expr.vals { + if p.expr_contains_value_match_or_if(element) { + p.mark_last_call_expr_return_as_used(mut element) + } + } + if expr.has_update_expr && p.expr_contains_value_match_or_if(expr.update_expr) { + p.mark_last_call_expr_return_as_used(mut expr.update_expr) + } + } + ast.StringInterLiteral { + // last stmt on block is a string interpolation, e.g. + // `'x=${match value { .. }}'`; mark any interpolated expression that is + // (or nests) a block-value match/if. + for mut e in expr.exprs { + if p.expr_contains_value_match_or_if(e) { + p.mark_last_call_expr_return_as_used(mut e) + } + } + } + ast.IndexExpr { + if expr.or_expr.stmts.len > 0 { + mut or_block_last_stmt := expr.or_expr.stmts.last() + p.mark_last_call_return_as_used(mut or_block_last_stmt) + } + // last stmt on block is an index expr, e.g. `values[match value { .. }]` + // or `(match value { .. })[0]`; mark the indexed expr and the index if + // either is (or nests) a block-value match/if. + if p.expr_contains_value_match_or_if(expr.left) { + p.mark_last_call_expr_return_as_used(mut expr.left) + } + if p.expr_contains_value_match_or_if(expr.index) { + p.mark_last_call_expr_return_as_used(mut expr.index) + } + } + ast.RangeExpr { + // last stmt on block is a range/slice bound, e.g. + // `values[(match value { .. })..]`; mark either bound that is (or + // nests) a block-value match/if. + if expr.has_low && p.expr_contains_value_match_or_if(expr.low) { + p.mark_last_call_expr_return_as_used(mut expr.low) + } + if expr.has_high && p.expr_contains_value_match_or_if(expr.high) { + p.mark_last_call_expr_return_as_used(mut expr.high) + } + } + ast.ParExpr { + // last stmt on block is parenthesized: ( match .. { a { foo() } } ) + p.mark_last_call_expr_return_as_used(mut expr.expr) + } + ast.UnsafeExpr { + // last stmt on block is unsafe-wrapped: unsafe { match .. { a { foo() } } } + p.mark_last_call_expr_return_as_used(mut expr.expr) + } + ast.CastExpr { + // last stmt on block is cast-wrapped: i64(match .. { a { foo() } }) + p.mark_last_call_expr_return_as_used(mut expr.expr) + } + ast.AsCast { + // last stmt on block is as-cast-wrapped: (match .. { a { foo() } }) as T + p.mark_last_call_expr_return_as_used(mut expr.expr) + } + ast.IfExpr { + // last stmt on block is: if .. { foo() } else { bar() } + for mut branch in expr.branches { + if branch.stmts.len > 0 { + mut last_if_stmt := branch.stmts.last() + p.mark_last_call_return_as_used(mut last_if_stmt) + } + } + } + ast.MatchExpr { + // last stmt on block is: match .. { a { foo() } b { bar() } } + for mut branch in expr.branches { + if branch.stmts.len > 0 { + mut last_match_stmt := branch.stmts.last() + p.mark_last_call_return_as_used(mut last_match_stmt) + } + } + } + ast.InfixExpr { + if expr.or_block.stmts.len > 0 { + mut or_block_last_stmt := expr.or_block.stmts.last() + p.mark_last_call_return_as_used(mut or_block_last_stmt) + } + // last stmt has infix expr with value operands, e.g. + // `foo()? + 'a'` or `1 + (match value { First { bar()! } })`. + // Recurse into both sides so nested/wrapped match/if/call values on + // either operand are marked as return-used. + p.mark_last_call_expr_return_as_used(mut expr.left) + p.mark_last_call_expr_return_as_used(mut expr.right) + } + ast.PrefixExpr { + if expr.or_block.stmts.len > 0 { + mut or_block_last_stmt := expr.or_block.stmts.last() + p.mark_last_call_return_as_used(mut or_block_last_stmt) + } + // last stmt is a prefix expr with a value operand, e.g. + // `-(match value { First { bar()! } })`; recurse into the operand. + p.mark_last_call_expr_return_as_used(mut expr.right) + } + ast.SelectorExpr { + if expr.or_block.stmts.len > 0 { + mut or_block_last_stmt := expr.or_block.stmts.last() + p.mark_last_call_return_as_used(mut or_block_last_stmt) + } + // last stmt on block is a selector, e.g. `(match value { .. }).field`; + // recurse into the receiver if it is (or nests) a block-value match/if. + if p.expr_contains_value_match_or_if(expr.expr) { + p.mark_last_call_expr_return_as_used(mut expr.expr) + } + } + ast.ComptimeCall, ast.ComptimeSelector { + if expr.or_block.stmts.len > 0 { + mut or_block_last_stmt := expr.or_block.stmts.last() + p.mark_last_call_return_as_used(mut or_block_last_stmt) + } + } + ast.DumpExpr { + // last stmt is `dump(match value { First { foo()! } })`; the dumped + // operand is a value, so recurse into it. + p.mark_last_call_expr_return_as_used(mut expr.expr) + } + ast.Likely { + // last stmt is `_likely_(match value { First { foo()! } })`; recurse + // into the wrapped operand. + p.mark_last_call_expr_return_as_used(mut expr.expr) } else {} } @@ -1157,7 +1426,31 @@ fn (mut p Parser) stmt(is_top_level bool) ast.Stmt { match p.tok.kind { .lcbr { mut pos := p.tok.pos() - if p.peek_token(2).kind == .colon { + // `{ ...m, k: v }` is a map update literal (a block cannot start with + // `...`), e.g. `{ ...(match x { .. }), 'k': v }`. + mut is_map_lit := p.peek_token(2).kind == .colon || p.peek_tok.kind == .ellipsis + if !is_map_lit && p.peek_tok.kind == .lpar { + // A parenthesized first key, e.g. `{ (match x { .. }) : v }`, puts the + // `:` past the closing `)`, so the single-token `peek_token(2)` check + // above misses it and the `{` would be misparsed as a block. Scan to + // the matching `)` and treat it as a map literal when a `:` follows. + mut depth := 1 + for n := 2; true; n++ { + kind := p.peek_token(n).kind + if kind == .eof { + break + } else if kind == .lpar { + depth++ + } else if kind == .rpar { + depth-- + if depth == 0 { + is_map_lit = p.peek_token(n + 1).kind == .colon + break + } + } + } + } + if is_map_lit { expr := p.expr(0) // `{ 'abc' : 22 }` return ast.ExprStmt{ diff --git a/vlib/v/tests/match_as_if_expr_value_with_propagation_test.v b/vlib/v/tests/match_as_if_expr_value_with_propagation_test.v new file mode 100644 index 00000000000000..aac61f08d2aedd --- /dev/null +++ b/vlib/v/tests/match_as_if_expr_value_with_propagation_test.v @@ -0,0 +1,808 @@ +// Regression test for https://github.com/vlang/v/issues/28000 +// A `match` whose arms use `!`/`?` propagation, used as the value of an +// `if`-expression (or directly), used to emit invalid C (`_t = ;`) because +// the calls in the match arms were not marked as having their return used. + +struct First {} + +struct Second {} + +type Node = First | Second + +fn lower_first(_ First) !int { + return 1 +} + +fn lower_second(_ Second) !int { + return 2 +} + +fn opt_first(_ First) ?int { + return 10 +} + +fn opt_second(_ Second) ?int { + return 20 +} + +// match inside an if-guard, assigned to a variable (the original repro) +fn select_value(node ?Node) !int { + result := if value := node { + match value { + First { lower_first(value)! } + Second { lower_second(value)! } + } + } else { + 0 + } + return result +} + +// parenthesized match value: `( match .. { .. } )` keeps an ast.ParExpr wrapper +fn select_value_paren(node ?Node) !int { + result := if value := node { + (match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) + } else { + 0 + } + return result +} + +// unsafe-wrapped match value: `( unsafe { match .. { .. } } )` keeps an +// ast.UnsafeExpr (inside an ast.ParExpr) around the match +fn select_value_unsafe(node ?Node) !int { + result := if value := node { + (unsafe { + match value { + First { lower_first(value)! } + Second { lower_second(value)! } + } + }) + } else { + 0 + } + return result +} + +// cast-wrapped match value: `i64(match .. { .. })` keeps an ast.CastExpr +// around the match (which is also nested in a void-context if-branch) +fn select_value_cast(node ?Node) !i64 { + result := if value := node { + i64(match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) + } else { + i64(0) + } + return result +} + +// composed wrappers: cast around unsafe around match, `i64(unsafe { match .. })` +fn select_value_cast_unsafe(node ?Node) !i64 { + result := if value := node { + i64(unsafe { + match value { + First { lower_first(value)! } + Second { lower_second(value)! } + } + }) + } else { + i64(0) + } + return result +} + +// match on the right of an infix expression: `1 + (match .. { .. })` +fn select_value_infix_right(node ?Node) !int { + result := if value := node { + 1 + (match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) + } else { + 0 + } + return result +} + +// match on the left of an infix expression: `(match .. { .. }) + 10` +fn select_value_infix_left(node ?Node) !int { + result := if value := node { + (match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) + 10 + } else { + 0 + } + return result +} + +fn wrap(x int) int { + return x * 10 +} + +// match as a call argument: `wrap(match .. { .. })` +fn select_value_callarg(node ?Node) !int { + result := if value := node { + wrap(match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) + } else { + 0 + } + return result +} + +// match nested inside a call argument that is itself a call: +// `wrap(wrap(match .. { .. }))` +fn select_value_nested_callarg(node ?Node) !int { + result := if value := node { + wrap(wrap(match value { + First { lower_first(value)! } + Second { lower_second(value)! } + })) + } else { + 0 + } + return result +} + +// match inside an infix expression inside a call argument: +// `wrap(1 + (match .. { .. }))` +fn select_value_callarg_infix(node ?Node) !int { + result := if value := node { + wrap(1 + (match value { + First { lower_first(value)! } + Second { lower_second(value)! } + })) + } else { + 0 + } + return result +} + +// match as an array-literal element: `[match .. { .. }]` +fn select_value_arraylit(node ?Node) ![]int { + result := if value := node { + [ + match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }, + ] + } else { + [0] + } + return result +} + +fn side_effect() int { + return 5 +} + +// an array-element match whose arm contains a nested *statement* match/if (with a +// void/empty branch) before the propagated value. The value-element flag must not +// leak into the nested statement, which is valid as a statement, not an expression. +fn select_value_arraylit_nested_stmt(node ?Node, cond bool) ![]int { + result := if value := node { + [ + match value { + First { + match cond { + true { side_effect() } + else {} + } + if cond { + side_effect() + } + lower_first(value)! + } + Second { + lower_second(value)! + } + }, + ] + } else { + [0] + } + return result +} + +struct Holder { + value int + other int +} + +// match as a struct-literal field value: `Holder{ value: match .. { .. } }` +fn select_value_structinit(node ?Node) !Holder { + result := if value := node { + Holder{ + value: match value { + First { lower_first(value)! } + Second { lower_second(value)! } + } + other: 100 + } + } else { + Holder{} + } + return result +} + +// match as a map-literal value: `{'value': match .. { .. }}` +fn select_value_mapinit(node ?Node) !map[string]int { + result := if value := node { + { + 'value': match value { + First { lower_first(value)! } + Second { lower_second(value)! } + } + } + } else { + { + 'value': 0 + } + } + return result +} + +// match as a prefix-expression operand: `-(match .. { .. })` +fn select_value_prefix(node ?Node) !int { + result := if value := node { + -(match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) + } else { + 0 + } + return result +} + +// match as an index-expression operand: `values[match .. { .. }]` +fn select_value_index(node ?Node) !string { + values := ['a', 'b', 'c'] + result := if value := node { + values[match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }] + } else { + 'x' + } + return result +} + +// match on the left of a membership operator: `(match .. { .. }) in [1, 2]` +fn select_value_membership(node ?Node) !bool { + result := if value := node { + (match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) in [1, 2] + } else { + false + } + return result +} + +struct Boxed { + value int +} + +fn boxed(v int) Boxed { + return Boxed{v} +} + +// match as a selector receiver: `(match .. { .. }).value` +fn select_value_selector(node ?Node) !int { + result := if value := node { + (match value { + First { boxed(lower_first(value)!) } + Second { boxed(lower_second(value)!) } + }).value + } else { + 0 + } + return result +} + +// match as the first (parenthesized) key of an inferred map: `{(match ..): v}` +fn select_value_mapkey(node ?Node) !map[int]string { + result := if value := node { + { + (match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }): 'x' + } + } else { + { + 0: 'x' + } + } + return result +} + +fn lower_first_array(_ First) ![]int { + return [1, 1] +} + +fn lower_second_array(_ Second) ![]int { + return [2, 2] +} + +// match as an array spread operand: `[...(match .. { .. })]` +fn select_value_spread(node ?Node) ![]int { + result := if value := node { + [...(match value { + First { lower_first_array(value)! } + Second { lower_second_array(value)! } + })] + } else { + [0] + } + return result +} + +fn make_holder_first(_ First) !Holder { + return Holder{ + value: 1 + } +} + +fn make_holder_second(_ Second) !Holder { + return Holder{ + value: 2 + } +} + +// match as a struct update operand: `Holder{ ...(match .. { .. }), other: 9 }` +fn select_value_struct_update(node ?Node) !Holder { + result := if value := node { + Holder{ + ...(match value { + First { make_holder_first(value)! } + Second { make_holder_second(value)! } + }) + other: 9 + } + } else { + Holder{} + } + return result +} + +fn map_first(_ First) !map[string]int { + return { + 'a': 1 + } +} + +fn map_second(_ Second) !map[string]int { + return { + 'a': 2 + } +} + +// match as a map update operand: `{ ...(match .. { .. }), 'b': 5 }` +fn select_value_map_update(node ?Node) !map[string]int { + result := if value := node { + { + ...(match value { + First { map_first(value)! } + Second { map_second(value)! } + }) + 'b': 5 + } + } else { + { + 'a': 0 + } + } + return result +} + +// match as a string interpolation operand: `'x=${match .. { .. }}'` +fn select_value_interp(node ?Node) !string { + result := if value := node { + 'x=${match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }}' + } else { + 'x=0' + } + return result +} + +// match as a `dump()` operand: `dump(match .. { .. })` +fn select_value_dump(node ?Node) !int { + result := if value := node { + dump(match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) + } else { + 0 + } + return result +} + +fn bool_first(_ First) !bool { + return true +} + +fn bool_second(_ Second) !bool { + return false +} + +// match as a `_likely_()` operand: `_likely_(match .. { .. })` +fn select_value_likely(node ?Node) !bool { + result := if value := node { + _likely_(match value { + First { bool_first(value)! } + Second { bool_second(value)! } + }) + } else { + false + } + return result +} + +// match as one value of a multi-return value list: `match .. { .. }, 9` +fn select_value_multiret(node ?Node) !(int, int) { + a, b := if value := node { + match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }, 9 + } else { + 0, 0 + } + return a, b +} + +struct Getter { + value int +} + +fn (g Getter) get() int { + return g.value +} + +fn make_getter_first(_ First) !Getter { + return Getter{1} +} + +fn make_getter_second(_ Second) !Getter { + return Getter{2} +} + +// match as a method-call receiver: `(match .. { .. }).get()` +fn select_value_method_recv(node ?Node) !int { + result := if value := node { + (match value { + First { make_getter_first(value)! } + Second { make_getter_second(value)! } + }).get() + } else { + 0 + } + return result +} + +// match as a slice lower bound: `values[(match .. { .. })..]` +fn select_value_slice_bound(node ?Node) ![]int { + values := [10, 20, 30, 40] + result := if value := node { + values[(match value { + First { lower_first(value)! } + Second { lower_second(value)! } + })..] + } else { + [0] + } + return result +} + +struct Circle { + r int +} + +struct Square { + s int +} + +type Shape = Circle | Square + +fn make_circle(r int) !Shape { + return Circle{r} +} + +// as-cast wrapped match value: `(match .. { .. }) as Circle` keeps an +// ast.AsCast (around an ast.ParExpr) with propagating arms +fn select_value_ascast(node ?int) !int { + shape := if v := node { + (match v { + 0 { make_circle(v)! } + else { make_circle(v + 1)! } + }) as Circle + } else { + Circle{99} + } + return shape.r +} + +// composed wrappers: as-cast around unsafe around match, +// `(unsafe { match .. }) as Circle` +fn select_value_ascast_unsafe(node ?int) !int { + shape := if v := node { + (unsafe { + match v { + 0 { make_circle(v)! } + else { make_circle(v + 1)! } + } + }) as Circle + } else { + Circle{99} + } + return shape.r +} + +// match with `?` option propagation +fn select_opt(node ?Node) ?int { + result := if value := node { + match value { + First { opt_first(value)? } + Second { opt_second(value)? } + } + } else { + 0 + } + return result +} + +// match used directly as the return value +fn direct_match(node Node) !int { + return match node { + First { lower_first(node)! } + Second { lower_second(node)! } + } +} + +// match assigned directly to a variable +fn assign_match(node Node) !int { + x := match node { + First { lower_first(node)! } + Second { lower_second(node)! } + } + return x +} + +fn test_match_as_if_expr_value_with_propagation() { + assert select_value(First{})! == 1 + assert select_value(Second{})! == 2 + assert select_value(none) or { -1 } == 0 +} + +fn test_parenthesized_match_as_if_expr_value_with_propagation() { + assert select_value_paren(First{})! == 1 + assert select_value_paren(Second{})! == 2 + assert select_value_paren(none) or { -1 } == 0 +} + +fn test_unsafe_wrapped_match_as_if_expr_value_with_propagation() { + assert select_value_unsafe(First{})! == 1 + assert select_value_unsafe(Second{})! == 2 + assert select_value_unsafe(none) or { -1 } == 0 +} + +fn test_cast_wrapped_match_as_if_expr_value_with_propagation() { + assert select_value_cast(First{})! == i64(1) + assert select_value_cast(Second{})! == i64(2) + assert select_value_cast(none) or { i64(-1) } == i64(0) +} + +fn test_cast_unsafe_wrapped_match_as_if_expr_value_with_propagation() { + assert select_value_cast_unsafe(First{})! == i64(1) + assert select_value_cast_unsafe(Second{})! == i64(2) + assert select_value_cast_unsafe(none) or { i64(-1) } == i64(0) +} + +fn test_as_cast_wrapped_match_as_if_expr_value_with_propagation() { + assert select_value_ascast(0)! == 0 + assert select_value_ascast(5)! == 6 + assert select_value_ascast(none) or { -1 } == 99 +} + +fn test_as_cast_unsafe_wrapped_match_as_if_expr_value_with_propagation() { + assert select_value_ascast_unsafe(0)! == 0 + assert select_value_ascast_unsafe(5)! == 6 + assert select_value_ascast_unsafe(none) or { -1 } == 99 +} + +fn test_infix_right_match_as_if_expr_value_with_propagation() { + assert select_value_infix_right(First{})! == 2 + assert select_value_infix_right(Second{})! == 3 + assert select_value_infix_right(none) or { -1 } == 0 +} + +fn test_infix_left_match_as_if_expr_value_with_propagation() { + assert select_value_infix_left(First{})! == 11 + assert select_value_infix_left(Second{})! == 12 + assert select_value_infix_left(none) or { -1 } == 0 +} + +fn test_call_argument_match_as_if_expr_value_with_propagation() { + assert select_value_callarg(First{})! == 10 + assert select_value_callarg(Second{})! == 20 + assert select_value_callarg(none) or { -1 } == 0 +} + +fn test_nested_call_argument_match_as_if_expr_value_with_propagation() { + assert select_value_nested_callarg(First{})! == 100 + assert select_value_nested_callarg(Second{})! == 200 + assert select_value_nested_callarg(none) or { -1 } == 0 +} + +fn test_call_argument_infix_match_as_if_expr_value_with_propagation() { + assert select_value_callarg_infix(First{})! == 20 + assert select_value_callarg_infix(Second{})! == 30 + assert select_value_callarg_infix(none) or { -1 } == 0 +} + +fn test_array_literal_match_as_if_expr_value_with_propagation() { + assert select_value_arraylit(First{})! == [1] + assert select_value_arraylit(Second{})! == [2] + assert select_value_arraylit(none) or { [-1] } == [0] +} + +fn test_array_literal_match_with_nested_statement_match() { + assert select_value_arraylit_nested_stmt(First{}, true)! == [1] + assert select_value_arraylit_nested_stmt(Second{}, false)! == [2] + assert select_value_arraylit_nested_stmt(none, true) or { [-1] } == [0] +} + +fn test_struct_init_field_match_as_if_expr_value_with_propagation() { + assert select_value_structinit(First{})!.value == 1 + assert select_value_structinit(Second{})!.value == 2 + assert select_value_structinit(none) or { + Holder{ + value: -1 + } + }.value == 0 +} + +fn test_map_init_value_match_as_if_expr_value_with_propagation() { + assert select_value_mapinit(First{})!['value'] == 1 + assert select_value_mapinit(Second{})!['value'] == 2 + assert (select_value_mapinit(none) or { + { + 'value': -1 + } + })['value'] == 0 +} + +fn test_prefix_operand_match_as_if_expr_value_with_propagation() { + assert select_value_prefix(First{})! == -1 + assert select_value_prefix(Second{})! == -2 + assert select_value_prefix(none) or { 42 } == 0 +} + +fn test_index_operand_match_as_if_expr_value_with_propagation() { + assert select_value_index(First{})! == 'b' + assert select_value_index(Second{})! == 'c' + assert select_value_index(none) or { 'z' } == 'x' +} + +fn test_membership_operand_match_as_if_expr_value_with_propagation() { + assert select_value_membership(First{})! == true + assert select_value_membership(Second{})! == true + assert select_value_membership(none) or { true } == false +} + +fn test_selector_receiver_match_as_if_expr_value_with_propagation() { + assert select_value_selector(First{})! == 1 + assert select_value_selector(Second{})! == 2 + assert select_value_selector(none) or { -1 } == 0 +} + +fn test_map_key_match_as_if_expr_value_with_propagation() { + assert select_value_mapkey(First{})![1] == 'x' + assert select_value_mapkey(Second{})![2] == 'x' + assert (select_value_mapkey(none) or { + { + 9: 'z' + } + })[0] == 'x' +} + +fn test_array_spread_match_as_if_expr_value_with_propagation() { + assert select_value_spread(First{})! == [1, 1] + assert select_value_spread(Second{})! == [2, 2] + assert select_value_spread(none) or { [-1] } == [0] +} + +fn test_struct_update_match_as_if_expr_value_with_propagation() { + assert select_value_struct_update(First{})!.value == 1 + assert select_value_struct_update(First{})!.other == 9 + assert select_value_struct_update(Second{})!.value == 2 + assert select_value_struct_update(none) or { + Holder{ + value: -1 + } + }.value == 0 +} + +fn test_map_update_match_as_if_expr_value_with_propagation() { + assert select_value_map_update(First{})!['a'] == 1 + assert select_value_map_update(First{})!['b'] == 5 + assert select_value_map_update(Second{})!['a'] == 2 + assert (select_value_map_update(none) or { + { + 'a': -1 + } + })['a'] == 0 +} + +fn test_string_interp_match_as_if_expr_value_with_propagation() { + assert select_value_interp(First{})! == 'x=1' + assert select_value_interp(Second{})! == 'x=2' + assert select_value_interp(none) or { 'x=z' } == 'x=0' +} + +fn test_dump_operand_match_as_if_expr_value_with_propagation() { + assert select_value_dump(First{})! == 1 + assert select_value_dump(Second{})! == 2 + assert select_value_dump(none) or { -1 } == 0 +} + +fn test_likely_operand_match_as_if_expr_value_with_propagation() { + assert select_value_likely(First{})! == true + assert select_value_likely(Second{})! == false + assert select_value_likely(none) or { true } == false +} + +fn test_multi_return_value_match_as_if_expr_value_with_propagation() { + a1, b1 := select_value_multiret(First{})! + assert a1 == 1 + assert b1 == 9 + a2, b2 := select_value_multiret(Second{})! + assert a2 == 2 + assert b2 == 9 + a3, b3 := select_value_multiret(none) or { -1, -1 } + assert a3 == 0 + assert b3 == 0 +} + +fn test_method_receiver_match_as_if_expr_value_with_propagation() { + assert select_value_method_recv(First{})! == 1 + assert select_value_method_recv(Second{})! == 2 + assert select_value_method_recv(none) or { -1 } == 0 +} + +fn test_slice_bound_match_as_if_expr_value_with_propagation() { + assert select_value_slice_bound(First{})! == [20, 30, 40] + assert select_value_slice_bound(Second{})! == [30, 40] + assert select_value_slice_bound(none) or { [-1] } == [0] +} + +fn test_match_as_if_expr_value_with_option_propagation() { + assert select_opt(First{})? == 10 + assert select_opt(Second{})? == 20 + assert select_opt(none) or { -1 } == 0 +} + +fn test_match_as_return_and_assign_value_with_propagation() { + assert direct_match(First{})! == 1 + assert direct_match(Second{})! == 2 + assert assign_match(First{})! == 1 + assert assign_match(Second{})! == 2 +} diff --git a/vlib/v3/tests/match_as_if_expr_value_propagation_codegen_test.v b/vlib/v3/tests/match_as_if_expr_value_propagation_codegen_test.v new file mode 100644 index 00000000000000..c29b88364adbf3 --- /dev/null +++ b/vlib/v3/tests/match_as_if_expr_value_propagation_codegen_test.v @@ -0,0 +1,411 @@ +// Regression test for https://github.com/vlang/v/issues/28000 +// A `match` whose arms use `!`/`?` propagation, used as the value of an +// `if`-expression, must assign the unwrapped result to the if-expression's +// result temp (it used to emit an empty expression `_t = ;`). +import os + +const vexe = @VEXE +const tests_dir = os.dir(@FILE) +const v3_dir = os.dir(tests_dir) +const vlib_dir = os.dir(v3_dir) +const v3_src = os.join_path(v3_dir, 'v3.v') + +fn test_match_as_if_expr_value_with_propagation() { + v3_bin := os.join_path(os.temp_dir(), 'v3_match_as_if_expr_value_propagation_test') + build := + os.execute('${vexe} -gc none -path "${vlib_dir}|@vlib|@vmodules" -o ${v3_bin} ${v3_src}') + assert build.exit_code == 0, build.output + + src := os.join_path(os.temp_dir(), 'v3_match_as_if_expr_value_propagation_input.v') + os.write_file(src, 'module main + +struct First {} +struct Second {} + +type Node = First | Second + +fn lower_first(_ First) !int { + return 1 +} + +fn lower_second(_ Second) !int { + return 2 +} + +fn select_value(node ?Node) !int { + result := if value := node { + match value { + First { lower_first(value)! } + Second { lower_second(value)! } + } + } else { + 0 + } + return result +} + +fn select_value_paren(node ?Node) !int { + result := if value := node { + (match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) + } else { + 0 + } + return result +} + +fn select_value_unsafe(node ?Node) !int { + result := if value := node { + (unsafe { + match value { + First { lower_first(value)! } + Second { lower_second(value)! } + } + }) + } else { + 0 + } + return result +} + +fn select_value_cast(node ?Node) !i64 { + result := if value := node { + i64(match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) + } else { + i64(0) + } + return result +} + +fn select_value_cast_unsafe(node ?Node) !i64 { + result := if value := node { + i64(unsafe { + match value { + First { lower_first(value)! } + Second { lower_second(value)! } + } + }) + } else { + i64(0) + } + return result +} + +fn select_value_infix_right(node ?Node) !int { + result := if value := node { + 1 + (match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) + } else { + 0 + } + return result +} + +fn select_value_infix_left(node ?Node) !int { + result := if value := node { + (match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) + 10 + } else { + 0 + } + return result +} + +fn wrap(x int) int { + return x * 10 +} + +fn select_value_callarg(node ?Node) !int { + result := if value := node { + wrap(match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) + } else { + 0 + } + return result +} + +fn select_value_nested_callarg(node ?Node) !int { + result := if value := node { + wrap(wrap(match value { + First { lower_first(value)! } + Second { lower_second(value)! } + })) + } else { + 0 + } + return result +} + +fn select_value_callarg_infix(node ?Node) !int { + result := if value := node { + wrap(1 + (match value { + First { lower_first(value)! } + Second { lower_second(value)! } + })) + } else { + 0 + } + return result +} + +fn select_value_arraylit(node ?Node) ![]int { + result := if value := node { + [match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }] + } else { + [0] + } + return result +} + +fn select_value_prefix(node ?Node) !int { + result := if value := node { + -(match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) + } else { + 0 + } + return result +} + +fn select_value_index(node ?Node) !int { + values := [10, 20, 30] + result := if value := node { + values[match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }] + } else { + 0 + } + return result +} + +fn select_value_slice_bound(node ?Node) ![]int { + values := [10, 20, 30, 40] + result := if value := node { + values[(match value { + First { lower_first(value)! } + Second { lower_second(value)! } + })..] + } else { + [0] + } + return result +} + +fn select_value_membership(node ?Node) !bool { + result := if value := node { + (match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }) in [1, 2] + } else { + false + } + return result +} + +fn select_value_mapkey(node ?Node) !map[int]int { + result := if value := node { + {(match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }): 100} + } else { + {0: 100} + } + return result +} + +fn select_value_interp(node ?Node) !string { + result := if value := node { + "x=\${match value { + First { lower_first(value)! } + Second { lower_second(value)! } + }}" + } else { + "x=0" + } + return result +} + +fn bool_first(_ First) !bool { + return true +} + +fn bool_second(_ Second) !bool { + return false +} + +fn select_value_likely(node ?Node) !bool { + result := if value := node { + _likely_(match value { + First { bool_first(value)! } + Second { bool_second(value)! } + }) + } else { + false + } + return result +} + +struct Boxed { + value int +} + +fn boxed(v int) Boxed { + return Boxed{v} +} + +fn select_value_selector(node ?Node) !int { + result := if value := node { + (match value { + First { boxed(lower_first(value)!) } + Second { boxed(lower_second(value)!) } + }).value + } else { + 0 + } + return result +} + +struct Holder { + value int + other int +} + +fn select_value_mapinit(node ?Node) !map[int]int { + result := if value := node { + { + 7: match value { + First { lower_first(value)! } + Second { lower_second(value)! } + } + } + } else { + { + 7: 0 + } + } + return result +} + +fn select_value_structinit(node ?Node) !Holder { + result := if value := node { + Holder{ + value: match value { + First { lower_first(value)! } + Second { lower_second(value)! } + } + other: 100 + } + } else { + Holder{} + } + return result +} + +struct Circle { + r int +} + +struct Square { + s int +} + +type Shape = Circle | Square + +fn make_circle(r int) !Shape { + return Circle{r} +} + +fn select_value_ascast(node ?int) !int { + shape := if v := node { + (match v { + 0 { make_circle(v)! } + else { make_circle(v + 1)! } + }) as Circle + } else { + Circle{99} + } + return shape.r +} + +fn select_value_ascast_unsafe(node ?int) !int { + shape := if v := node { + (unsafe { + match v { + 0 { make_circle(v)! } + else { make_circle(v + 1)! } + } + }) as Circle + } else { + Circle{99} + } + return shape.r +} + +fn direct_match(node Node) !int { + return match node { + First { lower_first(node)! } + Second { lower_second(node)! } + } +} + +fn main() { + println(select_value(First{})!) + println(select_value(Second{})!) + println(select_value_paren(First{})!) + println(select_value_unsafe(Second{})!) + println(select_value_cast(First{})!) + println(select_value_cast_unsafe(Second{})!) + println(select_value_infix_right(First{})!) + println(select_value_infix_left(Second{})!) + println(select_value_callarg(Second{})!) + println(select_value_nested_callarg(First{})!) + println(select_value_callarg_infix(First{})!) + println(select_value_arraylit(First{})!) + println(select_value_prefix(First{})!) + println(select_value_index(First{})!) + println(select_value_slice_bound(First{})!) + println(select_value_membership(First{})!) + println(select_value_selector(First{})!) + println(select_value_mapkey(First{})![1]) + println(select_value_interp(First{})!) + println(select_value_likely(First{})!) + println(select_value_structinit(First{})!.value) + println(select_value_mapinit(Second{})![7]) + println(select_value_ascast(5)!) + println(select_value_ascast_unsafe(5)!) + println(direct_match(Second{})!) +} +') or { + panic(err) + } + + bin := os.join_path(os.temp_dir(), 'v3_match_as_if_expr_value_propagation_out') + compile := os.execute('${v3_bin} ${src} -b c -o ${bin}') + assert compile.exit_code == 0, compile.output + assert !compile.output.contains('C compilation failed'), compile.output + + run := os.execute(bin) + assert run.exit_code == 0, run.output + assert run.output.trim_space() == '1\n2\n1\n2\n1\n2\n2\n12\n20\n100\n20\n[1]\n-1\n20\n[20, 30, 40]\ntrue\n1\n100\nx=1\ntrue\n1\n2\n6\n6\n2' +} diff --git a/vlib/v3/transform/sum.v b/vlib/v3/transform/sum.v index 78471b009f6615..b68a893fe7b436 100644 --- a/vlib/v3/transform/sum.v +++ b/vlib/v3/transform/sum.v @@ -1288,6 +1288,31 @@ fn (mut t Transformer) transform_as_expr(id flat.NodeId, node flat.Node) flat.No if node.children_count == 0 { return id } + first_child := t.a.child(&node, 0) + if t.is_value_match_or_if_operand(first_child) { + // A `match`/`if` operand of an `as` cast, e.g. `(match x { ... }) as Variant`, + // is a value expression whose (possibly propagating) branch tails must be + // lowered as values. Materialize it into a value temp first, then re-run the + // `as` conversion over that temp (mirrors the option-source path below). + mut operand_type := t.raw_expr_type_without_smartcast(first_child) + if operand_type.len == 0 { + operand_type = t.node_type(first_child) + } + if operand_type.len == 0 { + operand_type = t.resolve_expr_type(first_child) + } + value := t.transform_expr_for_type(first_child, operand_type) + start := t.a.children.len + t.a.children << value + return t.transform_as_expr(id, flat.Node{ + kind: .as_expr + value: node.value + typ: node.typ + children_start: start + children_count: 1 + pos: node.pos + }) + } expr_id := t.a.child(&node, 0) // `as` converts from the expression's storage type. Inside an `is` branch, // `node_type` reports the smartcast target instead; using that here makes an diff --git a/vlib/v3/transform/transform.v b/vlib/v3/transform/transform.v index 0f4315c75aa35b..f28f029e5eee8b 100644 --- a/vlib/v3/transform/transform.v +++ b/vlib/v3/transform/transform.v @@ -4636,7 +4636,9 @@ fn (mut t Transformer) transform_string_interp_part(child_id flat.NodeId) flat.N t.mark_string_interp_call_part_used(expr_id) saved_in_string_interp_part := t.in_string_interp_part t.in_string_interp_part = true - mut transformed := t.transform_expr(expr_id) + // route a value `match`/`if` interpolation operand (e.g. `'${match x { ... }}'`) + // through its target type so its propagating arms are lowered as values. + mut transformed := t.transform_value_operand(expr_id) t.in_string_interp_part = saved_in_string_interp_part mut typ := t.raw_alias_type_for_expr(expr_id) if typ.len == 0 { @@ -8016,7 +8018,9 @@ fn (mut t Transformer) transform_dump_expr(node flat.Node) flat.NodeId { if typ.len == 0 || typ == 'unknown' { typ = t.resolve_expr_type(child_id) } - child := t.transform_expr(child_id) + // route a value `match`/`if` dumped operand (e.g. `dump(match x { ... })`) + // through its target type so its propagating arms are lowered as values. + child := t.transform_value_operand(child_id) temp_name := t.new_temp('dump') t.pending_stmts << t.make_decl_assign_typed(temp_name, child, typ) if isnil(t.tc) || !t.tc.suppress_dump_output { @@ -11052,7 +11056,11 @@ fn (mut t Transformer) transform_block_expr_for_type(_id flat.NodeId, node flat. last := t.a.nodes[int(last_id)] tail_expr_id := if last.kind == .expr_stmt && last.children_count > 0 { t.a.child(&last, 0) - } else if last.kind == .block && t.stmt_value_type(last_id).len > 0 { + } else if last.kind in [.block, .match_stmt, .if_expr] && t.stmt_value_type(last_id).len > 0 { + // A block whose value tail is a bare `match`/`if` expression, e.g. + // `unsafe { match x { ... } }`. Treat the statement-shaped tail as the + // value expression so the target type reaches its (possibly propagating) + // branch tails instead of lowering them in a value-less statement context. last_id } else if !t.is_stmt_kind(last.kind) { last_id @@ -14511,6 +14519,24 @@ fn (mut t Transformer) transform_children_expr(id flat.NodeId, node flat.Node) f }) } +// transform_value_operand transforms an operand of an infix/prefix expression, +// routing a value `match`/`if` operand (e.g. `1 + (match x { ... })` or +// `-(match x { ... })`) through `transform_expr_for_type` so its (possibly +// propagating) branch tails are lowered as values instead of in a value-less +// statement context. +fn (mut t Transformer) transform_value_operand(id flat.NodeId) flat.NodeId { + if t.is_value_match_or_if_operand(id) { + mut typ := t.node_type(id) + if typ.len == 0 { + typ = t.resolve_expr_type(id) + } + if typ.len > 0 && typ != 'void' { + return t.transform_expr_for_type(id, typ) + } + } + return t.transform_expr(id) +} + // transform_infix_expr transforms transform infix expr data for transform. fn (mut t Transformer) transform_infix_expr(id flat.NodeId, node flat.Node) flat.NodeId { if node.children_count < 2 { @@ -14617,13 +14643,13 @@ fn (mut t Transformer) transform_infix_expr(id flat.NodeId, node flat.Node) flat lhs_id := t.a.children[node.children_start] rhs_id := t.a.children[node.children_start + 1] pending_start := t.pending_stmts.len - new_lhs := t.transform_expr(lhs_id) + new_lhs := t.transform_value_operand(lhs_id) mut lhs_pending := []flat.NodeId{} if t.pending_stmts.len > pending_start { lhs_pending = t.pending_stmts[pending_start..].clone() t.pending_stmts = t.pending_stmts[..pending_start].clone() } - new_rhs := t.transform_expr(rhs_id) + new_rhs := t.transform_value_operand(rhs_id) if lhs_pending.len > 0 { rhs_pending := t.pending_stmts[pending_start..].clone() t.pending_stmts = t.pending_stmts[..pending_start].clone() @@ -15082,7 +15108,9 @@ fn (mut t Transformer) transform_index_expr(id flat.NodeId, node flat.Node) flat mut changed := false for i in 0 .. node.children_count { child_id := t.a.child(&node, i) - mut new_child := t.transform_expr(child_id) + // route a value `match`/`if` operand (e.g. `values[match x { ... }]`) + // through its target type so its propagating arms are lowered as values. + mut new_child := t.transform_value_operand(child_id) if i == 0 { base := t.a.nodes[int(new_child)] if base.kind == .cast_expr { @@ -15585,7 +15613,9 @@ fn (mut t Transformer) transform_selector_base_expr(id flat.NodeId) flat.NodeId // transparent parentheses (`(x).field`, `((x)).field`), where `x` is still the // direct receiver. if !t.selector_base_is_ident_receiver(id) { - return t.transform_expr(id) + // route a value `match`/`if` receiver (e.g. `(match x { ... }).field`) + // through its target type so its propagating arms are lowered as values. + return t.transform_value_operand(id) } old_in_selector_base := t.in_selector_base t.in_selector_base = true @@ -16630,7 +16660,9 @@ fn (mut t Transformer) transform_prefix_expr(id flat.NodeId, node flat.Node) fla mut new_child := if node.op == .not { t.transform_expr_for_type(child_id, 'bool') } else { - t.transform_expr(child_id) + // route a value `match`/`if` operand (e.g. `-(match x { ... })`) + // through its target type so its propagating arms are lowered as values. + t.transform_value_operand(child_id) } if node.op == .not { child := t.a.nodes[int(new_child)] @@ -17093,6 +17125,28 @@ fn (mut t Transformer) transform_postfix_expr(id flat.NodeId, node flat.Node) fl }) } +// is_value_match_or_if_operand reports whether the node is a `match`/`if` +// expression used as a value, e.g. a cast operand like `i64(match x { ... })`. +// It looks through transparent wrappers: `(...)` parens, `unsafe { }` (a `.block` +// whose value tail is the expression), and a trailing `expr_stmt` — including +// compositions like `i64(unsafe { match ... })`. Such an operand must be +// transformed with its target type so its (possibly propagating) branch tails +// are lowered as values. +@[direct_array_access] +fn (t &Transformer) is_value_match_or_if_operand(id flat.NodeId) bool { + if int(id) < 0 { + return false + } + node := t.a.nodes[int(id)] + if node.kind in [.paren, .expr_stmt] && node.children_count > 0 { + return t.is_value_match_or_if_operand(t.a.child(&node, 0)) + } + if node.kind == .block && node.children_count > 0 { + return t.is_value_match_or_if_operand(t.a.child(&node, node.children_count - 1)) + } + return node.kind in [.match_stmt, .if_expr] +} + // transform_cast_expr transforms transform cast expr data for transform. @[direct_array_access] fn (mut t Transformer) transform_cast_expr(id flat.NodeId, node flat.Node) flat.NodeId { @@ -17274,6 +17328,12 @@ fn (mut t Transformer) transform_cast_expr(id flat.NodeId, node flat.Node) flat. new_children << t.transform_expr_for_type(child_id, target_type) } else if target_type in ['voidptr', 'byteptr', 'charptr'] { new_children << t.transform_expr_preserving_pointer_value(child_id) + } else if t.is_value_match_or_if_operand(child_id) { + // A `match`/`if` cast operand is a value expression whose (possibly + // propagating) branch tails must be lowered as values, e.g. + // `i64(match x { ... foo()! ... })`. Plain `transform_expr` would + // lower them in statement context and emit an empty ternary. + new_children << t.transform_expr_for_type(child_id, target_type) } else { new_children << t.transform_expr(child_id) }