From 86402204a72fea3b5873f4a37dc3fbf0131bb5bf Mon Sep 17 00:00:00 2001 From: Enzo Baldisserri Date: Tue, 14 Apr 2020 20:31:51 +0200 Subject: [PATCH] checker: fail if else isn't the last branch of match --- vlib/v/checker/checker.v | 117 ++++++++++-------- .../tests/inout/match_else_last_expr.out | 7 ++ .../tests/inout/match_else_last_expr.vv | 7 ++ vlib/v/parser/parser.v | 4 +- 4 files changed, 80 insertions(+), 55 deletions(-) create mode 100644 vlib/v/checker/tests/inout/match_else_last_expr.out create mode 100644 vlib/v/checker/tests/inout/match_else_last_expr.vv diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index b830bca714..6a60ec8acd 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -1355,69 +1355,80 @@ pub fn (c mut Checker) match_expr(node mut ast.MatchExpr) table.Type { } else {} } - if !node.branches[node.branches.len - 1].is_else { - mut used_values_count := 0 - for bi, 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 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 } } - 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) + + if !has_else { + mut used_values_count := 0 + for bi, 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 {} } } } - 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) + 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 + } } - 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) } } - 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.expected_type = cond_type mut ret_type := table.void_type diff --git a/vlib/v/checker/tests/inout/match_else_last_expr.out b/vlib/v/checker/tests/inout/match_else_last_expr.out new file mode 100644 index 0000000000..03f72e7893 --- /dev/null +++ b/vlib/v/checker/tests/inout/match_else_last_expr.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/inout/match_else_last_expr.v:4:3: error: `else` must be the last branch of `match` + 2| match 1 { + 3| 1 { println('1') } + 4| else { println('else') } + ~~~~~~ + 5| 4 { println('4') } + 6| } diff --git a/vlib/v/checker/tests/inout/match_else_last_expr.vv b/vlib/v/checker/tests/inout/match_else_last_expr.vv new file mode 100644 index 0000000000..3aa5cc5944 --- /dev/null +++ b/vlib/v/checker/tests/inout/match_else_last_expr.vv @@ -0,0 +1,7 @@ +fn main() { + match 1 { + 1 { println('1') } + else { println('else') } + 4 { println('4') } + } +} diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v index d7368313db..f74a4819dd 100644 --- a/vlib/v/parser/parser.v +++ b/vlib/v/parser/parser.v @@ -1919,8 +1919,8 @@ fn (p mut Parser) match_expr() ast.MatchExpr { // p.warn('match block') stmts := p.parse_block() pos := token.Position{ - line_nr: match_first_pos.line_nr - pos: match_first_pos.pos + line_nr: branch_first_pos.line_nr + pos: branch_first_pos.pos len: branch_last_pos.pos - branch_first_pos.pos + branch_last_pos.len } branches << ast.MatchBranch{