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.
pull/4581/head
Enzo Baldisserri 2020-04-24 16:04:39 +02:00 committed by GitHub
parent aa15dec660
commit 323ca2b3bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 210 additions and 147 deletions

View File

@ -467,28 +467,30 @@ pub fn sigint_to_signal_name(si int) string {
$if linux { $if linux {
// From `man 7 signal` on linux: // From `man 7 signal` on linux:
match si { match si {
30, 10, 16 { // TODO dependent on platform
// works only on x86/ARM/most others
10 /*, 30, 16 */ {
return 'SIGUSR1' return 'SIGUSR1'
} }
31, 12, 17 { 12 /*, 31, 17 */ {
return 'SIGUSR2' return 'SIGUSR2'
} }
20, 17, 18 { 17 /*, 20, 18 */ {
return 'SIGCHLD' return 'SIGCHLD'
} }
19, 18, 25 { 18 /*, 19, 25 */ {
return 'SIGCONT' return 'SIGCONT'
} }
17, 19, 23 { 19 /*, 17, 23 */ {
return 'SIGSTOP' return 'SIGSTOP'
} }
18, 20, 24 { 20 /*, 18, 24 */ {
return 'SIGTSTP' return 'SIGTSTP'
} }
21, 21, 26 { 21 /*, 26 */ {
return 'SIGTTIN' return 'SIGTTIN'
} }
22, 22, 27 { 22 /*, 27 */ {
return 'SIGTTOU' return 'SIGTTOU'
} }
// ///////////////////////////// // /////////////////////////////

View File

@ -77,26 +77,48 @@ pub fn (node &FnDecl) str(t &table.Table) string {
// string representaiton of expr // string representaiton of expr
pub fn (x Expr) str() string { pub fn (x Expr) str() string {
match x { match x {
Ident { BoolLiteral {
return it.name return it.val.str()
} }
InfixExpr { CastExpr {
return '${it.left.str()} $it.op.str() ${it.right.str()}' return '${it.typname}(${it.expr.str()})'
} }
PrefixExpr { CallExpr {
return it.op.str() + it.right.str() sargs := args2str(it.args)
if it.is_method {
return '${it.left.str()}.${it.name}($sargs)'
}
return '${it.mod}.${it.name}($sargs)'
} }
CharLiteral { CharLiteral {
return '`$it.val`' return '`$it.val`'
} }
IntegerLiteral { EnumVal {
return it.val return '.${it.val}'
} }
FloatLiteral { FloatLiteral {
return it.val return it.val
} }
StringLiteral { Ident {
return '"$it.val"' 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 { StringInterLiteral {
res := []string res := []string
@ -119,34 +141,12 @@ pub fn (x Expr) str() string {
res << "'" res << "'"
return res.join('') return res.join('')
} }
BoolLiteral { StringLiteral {
return it.val.str() return '"$it.val"'
}
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}'
} }
TypeOf { TypeOf {
return 'typeof(${it.expr.str()})' 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 { else {
return '[unhandled expr type ${typeof(x)}]' return '[unhandled expr type ${typeof(x)}]'
} }

View File

@ -1510,100 +1510,7 @@ pub fn (c mut Checker) match_expr(node mut ast.MatchExpr) table.Type {
if type_sym.kind != .sum_type { if type_sym.kind != .sum_type {
node.is_sum_type = false node.is_sum_type = false
} }
// all_possible_left_subtypes is a histogram of c.match_exprs(mut node, type_sym)
// 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.expected_type = cond_type c.expected_type = cond_type
mut ret_type := table.void_type mut ret_type := table.void_type
for branch in node.branches { 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 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 { pub fn (c mut Checker) if_expr(node mut ast.IfExpr) table.Type {
if c.expected_type != table.void_type { if c.expected_type != table.void_type {
// | c.assigned_var_name != '' { // | c.assigned_var_name != '' {

View File

@ -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| }

View File

@ -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)
}

View File

@ -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() { 3| fn main() {
4| x := A('test') 4| x := A('test')
5| _ = match x { 5| _ = match x {

View File

@ -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| 2|
3| fn main() { 3| fn main() {
4| res := match Asd { 4| res := match Asd {

View File

@ -407,13 +407,6 @@ fn (mut g Gen) stmt(node ast.Stmt) {
// println('cgen.stmt()') // println('cgen.stmt()')
// g.writeln('//// stmt start') // g.writeln('//// stmt start')
match node { 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 { ast.AssertStmt {
g.gen_assert_stmt(it) g.gen_assert_stmt(it)
} }
@ -549,7 +542,11 @@ fn (mut g Gen) stmt(node ast.Stmt) {
} }
ast.Import {} ast.Import {}
ast.InterfaceDecl { 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.Module {}
ast.Return { ast.Return {
@ -2562,7 +2559,6 @@ fn (mut g Gen) comp_if_to_ifdef(name string, is_comptime_optional bool) string {
'openbsd' { return '__OpenBSD__' } 'openbsd' { return '__OpenBSD__' }
'netbsd' { return '__NetBSD__' } 'netbsd' { return '__NetBSD__' }
'dragonfly' { return '__DragonFly__' } 'dragonfly' { return '__DragonFly__' }
'msvc' { return '_MSC_VER' }
'android' { return '__ANDROID__' } 'android' { return '__ANDROID__' }
'solaris' { return '__sun' } 'solaris' { return '__sun' }
'haiku' { return '__haiku__' } 'haiku' { return '__haiku__' }