From 323ca2b3bb2956a53c969098ae68f5b41da63335 Mon Sep 17 00:00:00 2001 From: Enzo Baldisserri Date: Fri, 24 Apr 2020 16:04:39 +0200 Subject: [PATCH] checker: check duplicates on match with no else Refactor match duplication test to work even if there is not else, and to include every expression. Add tests for duplicate expressions in match. --- vlib/os/os.v | 18 +- vlib/v/ast/str.v | 68 +++---- vlib/v/checker/checker.v | 166 ++++++++---------- .../tests/inout/match_duplicate_branch.out | 35 ++++ .../tests/inout/match_duplicate_branch.vv | 52 ++++++ .../v/checker/tests/inout/match_expr_else.out | 2 +- ...match_err.out => match_undefined_cond.out} | 2 +- .../{match_err.vv => match_undefined_cond.vv} | 0 vlib/v/gen/cgen.v | 14 +- 9 files changed, 210 insertions(+), 147 deletions(-) create mode 100644 vlib/v/checker/tests/inout/match_duplicate_branch.out create mode 100644 vlib/v/checker/tests/inout/match_duplicate_branch.vv rename vlib/v/checker/tests/inout/{match_err.out => match_undefined_cond.out} (63%) rename vlib/v/checker/tests/inout/{match_err.vv => match_undefined_cond.vv} (100%) diff --git a/vlib/os/os.v b/vlib/os/os.v index 00eeb9226d..a96d79908a 100644 --- a/vlib/os/os.v +++ b/vlib/os/os.v @@ -467,28 +467,30 @@ pub fn sigint_to_signal_name(si int) string { $if linux { // From `man 7 signal` on linux: match si { - 30, 10, 16 { + // TODO dependent on platform + // works only on x86/ARM/most others + 10 /*, 30, 16 */ { return 'SIGUSR1' } - 31, 12, 17 { + 12 /*, 31, 17 */ { return 'SIGUSR2' } - 20, 17, 18 { + 17 /*, 20, 18 */ { return 'SIGCHLD' } - 19, 18, 25 { + 18 /*, 19, 25 */ { return 'SIGCONT' } - 17, 19, 23 { + 19 /*, 17, 23 */ { return 'SIGSTOP' } - 18, 20, 24 { + 20 /*, 18, 24 */ { return 'SIGTSTP' } - 21, 21, 26 { + 21 /*, 26 */ { return 'SIGTTIN' } - 22, 22, 27 { + 22 /*, 27 */ { return 'SIGTTOU' } // ///////////////////////////// diff --git a/vlib/v/ast/str.v b/vlib/v/ast/str.v index f0aaef3c1e..0ca45e3332 100644 --- a/vlib/v/ast/str.v +++ b/vlib/v/ast/str.v @@ -77,26 +77,48 @@ pub fn (node &FnDecl) str(t &table.Table) string { // string representaiton of expr pub fn (x Expr) str() string { match x { - Ident { - return it.name + BoolLiteral { + return it.val.str() } - InfixExpr { - return '${it.left.str()} $it.op.str() ${it.right.str()}' + CastExpr { + return '${it.typname}(${it.expr.str()})' } - PrefixExpr { - return it.op.str() + it.right.str() + CallExpr { + sargs := args2str(it.args) + if it.is_method { + return '${it.left.str()}.${it.name}($sargs)' + } + return '${it.mod}.${it.name}($sargs)' } CharLiteral { return '`$it.val`' } - IntegerLiteral { - return it.val + EnumVal { + return '.${it.val}' } FloatLiteral { return it.val } - StringLiteral { - return '"$it.val"' + Ident { + return it.name + } + IndexExpr { + return '${it.left.str()}[${it.index.str()}]' + } + IntegerLiteral { + return it.val + } + InfixExpr { + return '${it.left.str()} $it.op.str() ${it.right.str()}' + } + ParExpr { + return it.expr.str() + } + PrefixExpr { + return it.op.str() + it.right.str() + } + SelectorExpr { + return '${it.expr.str()}.${it.field}' } StringInterLiteral { res := []string @@ -119,34 +141,12 @@ pub fn (x Expr) str() string { res << "'" return res.join('') } - BoolLiteral { - return it.val.str() - } - ParExpr { - return it.expr.str() - } - IndexExpr { - return '${it.left.str()}[${it.index.str()}]' - } - CastExpr { - return '${it.typname}(${it.expr.str()})' - } - SelectorExpr { - return '${it.expr.str()}.${it.field}' + StringLiteral { + return '"$it.val"' } TypeOf { return 'typeof(${it.expr.str()})' } - EnumVal { - return '.${it.val}' - } - CallExpr { - sargs := args2str(it.args) - if it.is_method { - return '${it.left.str()}.${it.name}($sargs)' - } - return '${it.mod}.${it.name}($sargs)' - } else { return '[unhandled expr type ${typeof(x)}]' } diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 2437d72347..bf21ebdae2 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -1510,100 +1510,7 @@ pub fn (c mut Checker) match_expr(node mut ast.MatchExpr) table.Type { if type_sym.kind != .sum_type { node.is_sum_type = false } - // all_possible_left_subtypes is a histogram of - // type => how many times it was used in the match - mut all_possible_left_subtypes := map[string]int - // all_possible_left_enum_vals is a histogram of - // enum value name => how many times it was used in the match - mut all_possible_left_enum_vals := map[string]int - match type_sym.info { - table.SumType { - for v in it.variants { - all_possible_left_subtypes[int(v).str()] = 0 - } - } - table.Enum { - for v in it.vals { - all_possible_left_enum_vals[v] = 0 - } - } - else {} - } - mut has_else := node.branches[node.branches.len - 1].is_else - if !has_else { - for i, branch in node.branches { - if branch.is_else && i != node.branches.len - 1 { - c.error('`else` must be the last branch of `match`', branch.pos) - has_else = true - break - } - } - - if !has_else { - mut used_values_count := 0 - for _, branch in node.branches { - used_values_count += branch.exprs.len - for bi_ei, bexpr in branch.exprs { - match bexpr { - ast.Type { - tidx := table.type_idx(it.typ) - stidx := tidx.str() - all_possible_left_subtypes[stidx] = all_possible_left_subtypes[stidx] + - 1 - } - ast.EnumVal { - all_possible_left_enum_vals[it.val] = all_possible_left_enum_vals[it.val] + - 1 - } - else {} - } - } - } - mut err := false - mut err_details := 'match must be exhaustive' - unhandled := []string - match type_sym.info { - table.SumType { - for k, v in all_possible_left_subtypes { - if v == 0 { - err = true - unhandled << '`' + c.table.type_to_str(table.new_type(k.int())) + '`' - } - if v > 1 { - err = true - multiple_type_name := '`' + c.table.type_to_str(table.new_type(k.int())) + - '`' - c.error('a match case for $multiple_type_name is handled more than once', - node.pos) - } - } - } - table.Enum { - for k, v in all_possible_left_enum_vals { - if v == 0 { - err = true - unhandled << '`.$k`' - } - if v > 1 { - err = true - multiple_enum_val := '`.$k`' - c.error('a match case for $multiple_enum_val is handled more than once', - node.pos) - } - } - } - else { - err = true - } - } - if err { - if unhandled.len > 0 { - err_details += ' (add match branches for: ' + unhandled.join(', ') + ' or an else{} branch)' - } - c.error(err_details, node.pos) - } - } - } + c.match_exprs(mut node, type_sym) c.expected_type = cond_type mut ret_type := table.void_type for branch in node.branches { @@ -1643,6 +1550,77 @@ pub fn (c mut Checker) match_expr(node mut ast.MatchExpr) table.Type { return ret_type } +fn (mut c Checker) match_exprs(node mut ast.MatchExpr, type_sym table.TypeSymbol) { + // branch_exprs is a histogram of how many times + // an expr was used in the match + mut branch_exprs := map[string]int + for branch in node.branches { + for expr in branch.exprs { + mut key := '' + match expr { + ast.Type { + key = c.table.type_to_str(it.typ) + } + ast.EnumVal { + key = it.val + } + else { + key = expr.str() + } + } + val := if key in branch_exprs { branch_exprs[key] } else { 0 } + if val == 1 { + c.error('match case `$key` is handled more than once', branch.pos) + } + branch_exprs[key] = val + 1 + } + } + // check that expressions are exhaustive + // this is achieved either by putting an else + // or, when the match is on a sum type or an enum + // by listing all variants or values + if !node.branches[node.branches.len - 1].is_else { + for i, branch in node.branches { + if branch.is_else && i != node.branches.len - 1 { + c.error('`else` must be the last branch of `match`', branch.pos) + return + } + } + mut err := false + mut err_details := 'match must be exhaustive' + unhandled := []string + match type_sym.info { + table.SumType { + for v in it.variants { + v_str := c.table.type_to_str(v) + if v_str !in branch_exprs { + err = true + unhandled << '`$v_str`' + } + } + } + table.Enum { + for v in it.vals { + if v !in branch_exprs { + err = true + unhandled << '`.$v`' + } + } + } + else { + println('else') + err = true + } + } + if err { + if unhandled.len > 0 { + err_details += ' (add match branches for: ' + unhandled.join(', ') + ' or `else {}` at the end)' + } + c.error(err_details, node.pos) + } + } +} + pub fn (c mut Checker) if_expr(node mut ast.IfExpr) table.Type { if c.expected_type != table.void_type { // | c.assigned_var_name != '' { diff --git a/vlib/v/checker/tests/inout/match_duplicate_branch.out b/vlib/v/checker/tests/inout/match_duplicate_branch.out new file mode 100644 index 0000000000..c5aeac6289 --- /dev/null +++ b/vlib/v/checker/tests/inout/match_duplicate_branch.out @@ -0,0 +1,35 @@ +vlib/v/checker/tests/inout/match_duplicate_branch.v:15:3: error: match case `St1` is handled more than once + 13| match i { + 14| St1 { println('St1') } + 15| St1 { println('St1') } + ~~~~~ + 16| St2 { println('St2') } + 17| } +vlib/v/checker/tests/inout/match_duplicate_branch.v:20:3: error: match case `St1` is handled more than once + 18| match i { + 19| St1 { println('St1') } + 20| St1 { println('St1') } + ~~~~~ + 21| else { println('else') } + 22| } +vlib/v/checker/tests/inout/match_duplicate_branch.v:29:3: error: match case `green` is handled more than once + 27| .red { println('red') } + 28| .green { println('green') } + 29| .green { println('green') } + ~~~~~~~~ + 30| .blue { println('blue') } + 31| } +vlib/v/checker/tests/inout/match_duplicate_branch.v:34:3: error: match case `green` is handled more than once + 32| match c { + 33| .red, .green { println('red green') } + 34| .green { println('green') } + ~~~~~~~~ + 35| else { println('else') } + 36| } +vlib/v/checker/tests/inout/match_duplicate_branch.v:43:3: error: match case `2` is handled more than once + 41| 1 { println('1') } + 42| 2 { println('2') } + 43| 2 { println('3') } + ~~~ + 44| else { println('else') } + 45| } diff --git a/vlib/v/checker/tests/inout/match_duplicate_branch.vv b/vlib/v/checker/tests/inout/match_duplicate_branch.vv new file mode 100644 index 0000000000..7ea3265bad --- /dev/null +++ b/vlib/v/checker/tests/inout/match_duplicate_branch.vv @@ -0,0 +1,52 @@ +enum Color { + red + green + blue +} + +struct St1 {} +struct St2 {} + +type St = St1 | St2 + +fn test_sum_type(i St) { + match i { + St1 { println('St1') } + St1 { println('St1') } + St2 { println('St2') } + } + match i { + St1 { println('St1') } + St1 { println('St1') } + else { println('else') } + } +} + +fn test_enum(c Color) { + match c { + .red { println('red') } + .green { println('green') } + .green { println('green') } + .blue { println('blue') } + } + match c { + .red, .green { println('red green') } + .green { println('green') } + else { println('else') } + } +} + +fn test_int(i int) { + match i { + 1 { println('1') } + 2 { println('2') } + 2 { println('3') } + else { println('else') } + } +} + +fn main() { + test_sum_type(St1{}) + test_enum(.red) + test_int(2) +} diff --git a/vlib/v/checker/tests/inout/match_expr_else.out b/vlib/v/checker/tests/inout/match_expr_else.out index 2544b8f7fd..1801afa151 100644 --- a/vlib/v/checker/tests/inout/match_expr_else.out +++ b/vlib/v/checker/tests/inout/match_expr_else.out @@ -1,4 +1,4 @@ -vlib/v/checker/tests/inout/match_expr_else.v:5:6: error: match must be exhaustive (add match branches for: `f64` or an else{} branch) +vlib/v/checker/tests/inout/match_expr_else.v:5:6: error: match must be exhaustive (add match branches for: `f64` or `else {}` at the end) 3| fn main() { 4| x := A('test') 5| _ = match x { diff --git a/vlib/v/checker/tests/inout/match_err.out b/vlib/v/checker/tests/inout/match_undefined_cond.out similarity index 63% rename from vlib/v/checker/tests/inout/match_err.out rename to vlib/v/checker/tests/inout/match_undefined_cond.out index 65ebe93a8c..7dff92aaab 100644 --- a/vlib/v/checker/tests/inout/match_err.out +++ b/vlib/v/checker/tests/inout/match_undefined_cond.out @@ -1,4 +1,4 @@ -vlib/v/checker/tests/inout/match_err.v:4:15: error: undefined: `Asd` +vlib/v/checker/tests/inout/match_undefined_cond.v:4:15: error: undefined: `Asd` 2| 3| fn main() { 4| res := match Asd { diff --git a/vlib/v/checker/tests/inout/match_err.vv b/vlib/v/checker/tests/inout/match_undefined_cond.vv similarity index 100% rename from vlib/v/checker/tests/inout/match_err.vv rename to vlib/v/checker/tests/inout/match_undefined_cond.vv diff --git a/vlib/v/gen/cgen.v b/vlib/v/gen/cgen.v index 537c7169dd..a323f8bf1c 100644 --- a/vlib/v/gen/cgen.v +++ b/vlib/v/gen/cgen.v @@ -407,13 +407,6 @@ fn (mut g Gen) stmt(node ast.Stmt) { // println('cgen.stmt()') // g.writeln('//// stmt start') match node { - ast.InterfaceDecl { - g.writeln('//interface') - g.writeln('typedef struct {') - g.writeln('\tvoid* _object;') - g.writeln('\tint _interface_idx;') - g.writeln('} $it.name;') - } ast.AssertStmt { g.gen_assert_stmt(it) } @@ -549,7 +542,11 @@ fn (mut g Gen) stmt(node ast.Stmt) { } ast.Import {} ast.InterfaceDecl { - g.writeln('// interface') + g.writeln('//interface') + g.writeln('typedef struct {') + g.writeln('\tvoid* _object;') + g.writeln('\tint _interface_idx;') + g.writeln('} $it.name;') } ast.Module {} ast.Return { @@ -2562,7 +2559,6 @@ fn (mut g Gen) comp_if_to_ifdef(name string, is_comptime_optional bool) string { 'openbsd' { return '__OpenBSD__' } 'netbsd' { return '__NetBSD__' } 'dragonfly' { return '__DragonFly__' } - 'msvc' { return '_MSC_VER' } 'android' { return '__ANDROID__' } 'solaris' { return '__sun' } 'haiku' { return '__haiku__' }