From 326e43385b09774deab66faee043dcd041b4a091 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Tue, 21 Mar 2023 06:38:30 -0300 Subject: [PATCH] cgen: fix match with option type (#17713) --- vlib/v/checker/match.v | 5 ++ .../tests/match_cond_with_parenthesis_err.out | 16 ++++- .../v/checker/tests/option_with_match_err.out | 7 ++ vlib/v/checker/tests/option_with_match_err.vv | 7 ++ vlib/v/gen/c/match.v | 18 ++++- vlib/v/tests/option_match_test.v | 66 +++++++++++++++++++ 6 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 vlib/v/checker/tests/option_with_match_err.out create mode 100644 vlib/v/checker/tests/option_with_match_err.vv create mode 100644 vlib/v/tests/option_match_test.v diff --git a/vlib/v/checker/match.v b/vlib/v/checker/match.v index b37677a831..4e00b17424 100644 --- a/vlib/v/checker/match.v +++ b/vlib/v/checker/match.v @@ -29,6 +29,7 @@ fn (mut c Checker) match_expr(mut node ast.MatchExpr) ast.Type { c.ensure_type_exists(node.cond_type, node.pos) or { return ast.void_type } c.check_expr_opt_call(node.cond, cond_type) cond_type_sym := c.table.sym(cond_type) + cond_is_option := cond_type.has_flag(.option) node.is_sum_type = cond_type_sym.kind in [.interface_, .sum_type] c.match_exprs(mut node, cond_type_sym) c.expected_type = cond_type @@ -58,6 +59,10 @@ fn (mut c Checker) match_expr(mut node ast.MatchExpr) ast.Type { c.expected_type = node.expected_type } expr_type := c.expr(stmt.expr) + if !branch.is_else && cond_is_option && branch.exprs[0] !is ast.None { + c.error('`match` expression with Option type only checks against `none`, to match its value you must unwrap it first `var?`', + branch.pos) + } stmt.typ = expr_type if first_iteration { if node.is_expr && (node.expected_type.has_flag(.option) diff --git a/vlib/v/checker/tests/match_cond_with_parenthesis_err.out b/vlib/v/checker/tests/match_cond_with_parenthesis_err.out index f11b9bd9d2..5a8f82c940 100644 --- a/vlib/v/checker/tests/match_cond_with_parenthesis_err.out +++ b/vlib/v/checker/tests/match_cond_with_parenthesis_err.out @@ -1,7 +1,21 @@ vlib/v/checker/tests/match_cond_with_parenthesis_err.vv:14:15: error: unnecessary `()` in `match` condition, use `match expr {` instead of `match (expr) {`. - 12 | + 12 | 13 | fn bar() bool { 14 | return match (foo()) { | ~~~~~~~ 15 | .a { true } 16 | .b, .c { false } +vlib/v/checker/tests/match_cond_with_parenthesis_err.vv:15:3: error: `match` expression with Option type only checks against `none`, to match its value you must unwrap it first `var?` + 13 | fn bar() bool { + 14 | return match (foo()) { + 15 | .a { true } + | ~~ + 16 | .b, .c { false } + 17 | } +vlib/v/checker/tests/match_cond_with_parenthesis_err.vv:16:3: error: `match` expression with Option type only checks against `none`, to match its value you must unwrap it first `var?` + 14 | return match (foo()) { + 15 | .a { true } + 16 | .b, .c { false } + | ~~~~~~ + 17 | } + 18 | } diff --git a/vlib/v/checker/tests/option_with_match_err.out b/vlib/v/checker/tests/option_with_match_err.out new file mode 100644 index 0000000000..0881877ca3 --- /dev/null +++ b/vlib/v/checker/tests/option_with_match_err.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/option_with_match_err.vv:4:3: error: `match` expression with Option type only checks against `none`, to match its value you must unwrap it first `var?` + 2 | mut a := ?int(12) + 3 | match a { + 4 | 12 { println(a) } + | ~~ + 5 | else { println('else') } + 6 | } diff --git a/vlib/v/checker/tests/option_with_match_err.vv b/vlib/v/checker/tests/option_with_match_err.vv new file mode 100644 index 0000000000..00ff541c5f --- /dev/null +++ b/vlib/v/checker/tests/option_with_match_err.vv @@ -0,0 +1,7 @@ +fn main() { + mut a := ?int(12) + match a { + 12 { println(a) } + else { println('else') } + } +} \ No newline at end of file diff --git a/vlib/v/gen/c/match.v b/vlib/v/gen/c/match.v index dd8131e272..9c445c074e 100644 --- a/vlib/v/gen/c/match.v +++ b/vlib/v/gen/c/match.v @@ -65,7 +65,9 @@ fn (mut g Gen) match_expr(node ast.MatchExpr) { g.inside_match_result = true } } - if node.cond in [ast.Ident, ast.IntegerLiteral, ast.StringLiteral, ast.FloatLiteral] + if (node.cond in [ast.Ident, ast.IntegerLiteral, ast.StringLiteral, ast.FloatLiteral] + && (node.cond !is ast.Ident || (node.cond is ast.Ident + && (node.cond as ast.Ident).or_expr.kind == .absent))) || (node.cond is ast.SelectorExpr && (node.cond as ast.SelectorExpr).or_block.kind == .absent && ((node.cond as ast.SelectorExpr).expr !is ast.CallExpr @@ -437,6 +439,14 @@ fn (mut g Gen) match_expr_classic(node ast.MatchExpr, is_expr bool, cond_var str if i > 0 { g.write(' || ') } + if expr is ast.None { + old_left_is_opt := g.left_is_opt + g.left_is_opt = true + g.expr(node.cond) + g.left_is_opt = old_left_is_opt + g.write('.state == 2') + continue + } match type_sym.kind { .array { ptr_typ := g.equality_fn(node.cond_type) @@ -478,6 +488,12 @@ fn (mut g Gen) match_expr_classic(node ast.MatchExpr, is_expr bool, cond_var str g.write('${cond_var} <= ') g.expr(expr.high) g.write(')') + } else if expr is ast.None { + old_left_is_opt := g.left_is_opt + g.left_is_opt = true + g.expr(node.cond) + g.left_is_opt = old_left_is_opt + g.write('.state == 2') } else { g.write('${cond_var} == (') g.expr(expr) diff --git a/vlib/v/tests/option_match_test.v b/vlib/v/tests/option_match_test.v new file mode 100644 index 0000000000..ef9e082210 --- /dev/null +++ b/vlib/v/tests/option_match_test.v @@ -0,0 +1,66 @@ +fn test_simple_match_expr() { + mut a := ?int(12) + match a? { + 12 { + println(a) + } + else { + println('else') + assert false + } + } + + match a { + none { + println('none') + assert false + } + else { + println('else') + } + } + + a = none + + match a { + none { + println('none') + } + else { + println('else') + assert false + } + } + + mut b := ?string('aaa') + match b? { + 'aaa' { + println(b) + } + else { + println('else') + assert false + } + } + + match b { + none { + println('none') + assert false + } + else { + println('else') + } + } + + b = none + match b { + none { + println('none') + } + else { + println('else') + assert false + } + } +}