From 3e68e429b6a4fa0b67932cb2795961d6d4882214 Mon Sep 17 00:00:00 2001 From: Enzo Baldisserri Date: Sat, 25 Apr 2020 21:51:44 +0200 Subject: [PATCH] checker: avert if else is unnecessary --- vlib/log/log.v | 1 - vlib/time/format.v | 3 +- vlib/v/builder/compile.v | 4 - vlib/v/checker/checker.v | 95 +++++++++++-------- .../v/checker/tests/inout/match_expr_else.out | 14 +++ vlib/v/checker/tests/inout/match_expr_else.vv | 28 ++++++ vlib/v/pref/os.v | 5 - .../v/tests/match_expression_for_types_test.v | 4 - 8 files changed, 96 insertions(+), 58 deletions(-) diff --git a/vlib/log/log.v b/vlib/log/log.v index 07377881de..97eaac0281 100644 --- a/vlib/log/log.v +++ b/vlib/log/log.v @@ -21,7 +21,6 @@ fn tag(l Level) string { .warn { term.yellow('WARN ') } .info { term.white('INFO ') } .debug { term.blue('DEBUG') } - else { ' ' } } } diff --git a/vlib/time/format.v b/vlib/time/format.v index b60bebeae3..ac38fbf9a5 100644 --- a/vlib/time/format.v +++ b/vlib/time/format.v @@ -150,8 +150,7 @@ mut res := match fmt_date { .space{ ' ' } - else { - 'unknown enumeration $fmt_dlmtr'}}) + }) return res } diff --git a/vlib/v/builder/compile.v b/vlib/v/builder/compile.v index e905009c16..2c64c5228b 100644 --- a/vlib/v/builder/compile.v +++ b/vlib/v/builder/compile.v @@ -42,10 +42,6 @@ pub fn compile(command string, pref &pref.Preferences) { .x64 { b.compile_x64() } - else { - eprintln('backend not implemented `$pref.backend`') - exit(1) - } } if pref.is_stats { tmark.stop() diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 8fba3763b9..4b7bb9086a 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -292,22 +292,22 @@ pub fn (mut c Checker) infix_expr(infix_expr mut ast.InfixExpr) table.Type { .array { right_sym := c.table.get_type_symbol(right.array_info().elem_type) if left.kind != right_sym.kind { - c.error('the data type on the left of `in` does not match the array item type', + c.error('the data type on the left of `in` does not match the array item type', infix_expr.pos) } } .map { key_sym := c.table.get_type_symbol(right.map_info().key_type) if left.kind != key_sym.kind { - c.error('the data type on the left of `in` does not match the map key type', + c.error('the data type on the left of `in` does not match the map key type', infix_expr.pos) - } - } + } + } .string { if left.kind != .string { c.error('the data type on the left of `in` must be a string', infix_expr.pos) } - } + } else { c.error('`in` can only be used with an array/map/string', infix_expr.pos) } @@ -365,10 +365,10 @@ pub fn (mut c Checker) infix_expr(infix_expr mut ast.InfixExpr) table.Type { c.error('mismatched types `$left.name` and `$right.name`', infix_expr.right.position()) } else if !left.is_int() && right.is_int() { c.error('mismatched types `$left.name` and `$right.name`', infix_expr.left.position()) - } else if left.kind in [.f32, .f64, .string, .array, .array_fixed, .map, .struct_] && + } else if left.kind in [.f32, .f64, .string, .array, .array_fixed, .map, .struct_] && !left.has_method(infix_expr.op.str()) { c.error('mismatched types `$left.name` and `$right.name`', infix_expr.left.position()) - } else if right.kind in [.f32, .f64, .string, .array, .array_fixed, .map, .struct_] && + } else if right.kind in [.f32, .f64, .string, .array, .array_fixed, .map, .struct_] && !right.has_method(infix_expr.op.str()) { c.error('mismatched types `$left.name` and `$right.name`', infix_expr.right.position()) } @@ -377,7 +377,7 @@ pub fn (mut c Checker) infix_expr(infix_expr mut ast.InfixExpr) table.Type { } // TODO: Absorb this block into the above single side check block to accelerate. if left_type == table.bool_type && !(infix_expr.op in [.eq, .ne, .logical_or, .and]) { - c.error('bool types only have the following operators defined: `==`, `!=`, `||`, and `&&`', + c.error('bool types only have the following operators defined: `==`, `!=`, `||`, and `&&`', infix_expr.pos) } else if left_type == table.string_type && !(infix_expr.op in [.plus, .eq, .ne, .lt, .gt, .le, .ge]) { // TODO broken !in @@ -1601,46 +1601,57 @@ fn (mut c Checker) match_exprs(node mut ast.MatchExpr, type_sym table.TypeSymbol // 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 { + mut is_exhaustive := true + mut 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 { + is_exhaustive = false + unhandled << '`$v_str`' + } + } + } + table.Enum { + for v in it.vals { + if v !in branch_exprs { + is_exhaustive = false + unhandled << '`.$v`' + } + } + } + else { + is_exhaustive = false + } + } + mut else_branch := node.branches[node.branches.len - 1] + mut has_else := else_branch.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) - return + else_branch = branch + has_else = true } } - 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) - } } + if is_exhaustive { + if has_else { + c.error('match expression is exhaustive, `else` is unnecessary', else_branch.pos) + } + return + } + if has_else { + return + } + mut err_details := 'match must be exhaustive' + if unhandled.len > 0 { + err_details += ' (add match branches for: ' + unhandled.join(', ') + ' or `else {}` at the end)' + } else { + err_details += ' (add `else {}` at the end)' + } + c.error(err_details, node.pos) } pub fn (mut c Checker) if_expr(node mut ast.IfExpr) table.Type { diff --git a/vlib/v/checker/tests/inout/match_expr_else.out b/vlib/v/checker/tests/inout/match_expr_else.out index 1801afa151..d269437812 100644 --- a/vlib/v/checker/tests/inout/match_expr_else.out +++ b/vlib/v/checker/tests/inout/match_expr_else.out @@ -5,3 +5,17 @@ vlib/v/checker/tests/inout/match_expr_else.v:5:6: error: match must be exhaustiv ~~~~~~~~~ 6| int { 7| 'int' +vlib/v/checker/tests/inout/match_expr_else.v:23:3: error: match expression is exhaustive, `else` is unnecessary + 21| 'f64' + 22| } + 23| else { + ~~~~~~ + 24| 'else' + 25| } +vlib/v/checker/tests/inout/match_expr_else.v:34:3: error: `else` must be the last branch of `match` + 32| 'string' + 33| } + 34| else { + ~~~~~~ + 35| 'else' + 36| } diff --git a/vlib/v/checker/tests/inout/match_expr_else.vv b/vlib/v/checker/tests/inout/match_expr_else.vv index d120acbd19..fff7be0253 100644 --- a/vlib/v/checker/tests/inout/match_expr_else.vv +++ b/vlib/v/checker/tests/inout/match_expr_else.vv @@ -10,4 +10,32 @@ fn main() { 'string' } } + _ := match x { + int { + 'int' + } + string { + 'string' + } + f64 { + 'f64' + } + else { + 'else' + } + } + _ := match x { + int { + 'int' + } + string { + 'string' + } + else { + 'else' + } + f64 { + 'f64' + } + } } diff --git a/vlib/v/pref/os.v b/vlib/v/pref/os.v index dccc775c30..e1a9296dff 100644 --- a/vlib/v/pref/os.v +++ b/vlib/v/pref/os.v @@ -107,11 +107,6 @@ pub fn (o OS) str() string { .haiku { return 'Haiku' } - else { - //TODO Remove when V is smart enough to know that there's no other possibilities - //should never be reached as all enum types have been enumerated - panic('unknown OS enum type: $o') - } } } diff --git a/vlib/v/tests/match_expression_for_types_test.v b/vlib/v/tests/match_expression_for_types_test.v index 9cf783409c..e3aca2ec34 100644 --- a/vlib/v/tests/match_expression_for_types_test.v +++ b/vlib/v/tests/match_expression_for_types_test.v @@ -55,10 +55,6 @@ fn test_match_expression_on_sumtype_full(){ c = 2 eprintln('hi') 'a string' - }else{ - c = 3 - eprintln('hi') - 'unknown' } } assert res == 'an integer'