checker: check for empty branches in match expressions (#10593)

pull/10606/head
shadowninja55 2021-06-28 12:32:28 -04:00 committed by GitHub
parent 63638fd271
commit 06a6a8e199
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 60 additions and 16 deletions

View File

@ -774,6 +774,7 @@ pub:
pos token.Position pos token.Position
is_else bool is_else bool
post_comments []Comment // comments below ´... }´ post_comments []Comment // comments below ´... }´
branch_pos token.Position // for checker errors about invalid branches
pub mut: pub mut:
exprs []Expr // left side exprs []Expr // left side
scope &Scope scope &Scope

View File

@ -5497,19 +5497,27 @@ pub fn (mut c Checker) match_expr(mut node ast.MatchExpr) ast.Type {
node.is_sum_type = cond_type_sym.kind in [.interface_, .sum_type] node.is_sum_type = cond_type_sym.kind in [.interface_, .sum_type]
c.match_exprs(mut node, cond_type_sym) c.match_exprs(mut node, cond_type_sym)
c.expected_type = cond_type c.expected_type = cond_type
mut first_iteration := true
mut ret_type := ast.void_type mut ret_type := ast.void_type
mut nbranches_with_return := 0 mut nbranches_with_return := 0
mut nbranches_without_return := 0 mut nbranches_without_return := 0
for branch in node.branches { for branch in node.branches {
c.stmts(branch.stmts) c.stmts(branch.stmts)
if node.is_expr && branch.stmts.len > 0 { if node.is_expr {
if branch.stmts.len > 0 {
// ignore last statement - workaround // ignore last statement - workaround
// currently the last statement in a match branch does not have an // currently the last statement in a match branch does not have an
// expected value set, so e.g. IfExpr.is_expr is not set. // expected value set, so e.g. IfExpr.is_expr is not set.
// probably any mismatch will be caught by not producing a value instead // probably any mismatch will be caught by not producing a value instead
for st in branch.stmts[0..branch.stmts.len - 1] { for st in branch.stmts[0..branch.stmts.len - 1] {
// must not contain C statements // must not contain C statements
st.check_c_expr() or { c.error('`match` expression branch has $err.msg', st.pos) } st.check_c_expr() or {
c.error('`match` expression branch has $err.msg', st.pos)
}
}
} else if ret_type != ast.void_type {
c.error('`match` expression requires an expression as the last statement of every branch',
branch.branch_pos)
} }
} }
// If the last statement is an expression, return its type // If the last statement is an expression, return its type
@ -5521,7 +5529,7 @@ pub fn (mut c Checker) match_expr(mut node ast.MatchExpr) ast.Type {
c.expected_type = node.expected_type c.expected_type = node.expected_type
} }
expr_type := c.expr(stmt.expr) expr_type := c.expr(stmt.expr)
if ret_type == ast.void_type { if first_iteration {
if node.is_expr && !node.expected_type.has_flag(.optional) if node.is_expr && !node.expected_type.has_flag(.optional)
&& c.table.get_type_symbol(node.expected_type).kind == .sum_type { && c.table.get_type_symbol(node.expected_type).kind == .sum_type {
ret_type = node.expected_type ret_type = node.expected_type
@ -5540,15 +5548,14 @@ pub fn (mut c Checker) match_expr(mut node ast.MatchExpr) ast.Type {
} }
} }
else { else {
// TODO: ask alex about this if node.is_expr && ret_type != ast.void_type {
// typ := c.expr(stmt.expr) c.error('`match` expression requires an expression as the last statement of every branch',
// type_sym := c.table.get_type_symbol(typ) stmt.pos)
// p.warn('match expr ret $type_sym.name')
// node.typ = typ
// return typ
} }
} }
} }
}
first_iteration = false
if has_return := c.has_return(branch.stmts) { if has_return := c.has_return(branch.stmts) {
if has_return { if has_return {
nbranches_with_return++ nbranches_with_return++

View File

@ -0,0 +1,6 @@
vlib/v/checker/tests/match_expr_empty_branch.vv:3:2: error: `match` expression requires an expression as the last statement of every branch
1 | _ := match true {
2 | true { 0 }
3 | false {}
| ~~~~~~~~
4 | }

View File

@ -0,0 +1,4 @@
_ := match true {
true { 0 }
false {}
}

View File

@ -0,0 +1,7 @@
vlib/v/checker/tests/match_expr_non_void_stmt_last.vv:9:4: error: `match` expression requires an expression as the last statement of every branch
7 | }
8 | []int {
9 | return arr.str()
| ~~~~~~~~~~~~~~~~
10 | }
11 | }

View File

@ -0,0 +1,17 @@
type Arr = []int | []string
fn (arr Arr) str() string {
return match arr {
[]string {
arr.join(' ')
}
[]int {
return arr.str()
}
}
}
fn main() {
println(Arr([0, 0]))
println(Arr(['0', '0']))
}

View File

@ -237,12 +237,14 @@ fn (mut p Parser) match_expr() ast.MatchExpr {
p.close_scope() p.close_scope()
p.inside_match_body = false p.inside_match_body = false
pos := branch_first_pos.extend_with_last_line(branch_last_pos, p.prev_tok.line_nr) pos := branch_first_pos.extend_with_last_line(branch_last_pos, p.prev_tok.line_nr)
branch_pos := branch_first_pos.extend_with_last_line(p.tok.position(), p.tok.line_nr)
post_comments := p.eat_comments({}) post_comments := p.eat_comments({})
branches << ast.MatchBranch{ branches << ast.MatchBranch{
exprs: exprs exprs: exprs
ecmnts: ecmnts ecmnts: ecmnts
stmts: stmts stmts: stmts
pos: pos pos: pos
branch_pos: branch_pos
is_else: is_else is_else: is_else
post_comments: post_comments post_comments: post_comments
scope: branch_scope scope: branch_scope