Skip to content

Commit

Permalink
feat(checker): check for exhaustive match and unreachable else branch (
Browse files Browse the repository at this point in the history
  • Loading branch information
serkonda7 authored Jan 8, 2024
1 parent 3978c66 commit c402b02
Show file tree
Hide file tree
Showing 15 changed files with 148 additions and 20 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ _unreleased_
- `strings.Builder`: Replace `write_chars(data []u8)` with `write_u8(c u8)`

### Error Checks
- match: Prevent duplicate branch conditions
- match
- Require match to be exhaustive
- Prevent duplicate branch conditions
- Warn if else branch is unreachable

### Type Checks
- Ensure function parameter type exists
Expand Down
2 changes: 1 addition & 1 deletion lib/bait/ast/repr.bt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub fun ( e Expr) repr() string {
CallExpr{ e.name + '()'} // TODO left.repr, args
CharLiteral { e.val }
ComptimeVar { e.name }
EnumVal{ e.name + '.' + e.val }
EnumVal{ e.val }
FloatLiteral { e.val }
HashExpr { '#XX' } // TODO lang
Ident { e.name }
Expand Down
49 changes: 46 additions & 3 deletions lib/bait/checker/if_match.bt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ fun (mut c Checker) if_match(mut node ast.IfMatch) ast.Type {
node.typ = c.expected_type
}

mut branch_exprs := []string
mut cond_sym := ast.TypeSymbol{}
if node.is_match {
// Check at least one branch except `else` exists
min_branches := if node.has_else { 2 } else { 1 }
Expand All @@ -28,10 +28,11 @@ fun (mut c Checker) if_match(mut node ast.IfMatch) ast.Type {

// Are we in a sumtype match?
cond_type := c.expr((node.branches[0].cond as ast.InfixExpr).left)
sym := c.table.get_sym(cond_type)
c.is_sumtype_match = sym.kind == .sum_type
cond_sym = c.table.get_sym(cond_type)
c.is_sumtype_match = cond_sym.kind == .sum_type
}

mut branch_exprs := []string
mut nr_branches_return := 0
for i, branch in node.branches {
c.open_scope()
Expand Down Expand Up @@ -85,9 +86,51 @@ fun (mut c Checker) if_match(mut node ast.IfMatch) ast.Type {
}
}

if node.is_match {
c.check_exhaustive_match(cond_sym.info, branch_exprs, node)
}

c.returns = nr_branches_return == node.branches.length
c.is_if_match_expr = save_is_if_match_expr
c.is_sumtype_match = false

return node.typ
}

fun (mut c Checker) check_exhaustive_match(info ast.TypeInfo, branch_exprs []string, node ast.IfMatch) {
mut is_exhaustive := true
mut unhandled := []string

if info is ast.EnumInfo {
for val in info.vals {
if not branch_exprs.contains(val) {
is_exhaustive = false
unhandled.push(val)
}
}
} else if info is ast.SumTypeInfo {
for typ in info.variants {
variant := c.table.type_name(typ)
if not branch_exprs.contains(variant) {
is_exhaustive = false
unhandled.push(variant)
}
}
} else {
is_exhaustive = false
}

if is_exhaustive {
if node.has_else {
c.warn('match is exhaustive, else is unreachable', node.pos)
}
} else if not node.has_else {
mut msg := 'match must be exhaustive'
if unhandled.length > 0 {
msg += ' (add `else {}` or branches for ${unhandled.join(", ")})'
} else {
msg += ' (add `else {}` branch)'
}
c.error(msg, node.pos)
}
}
8 changes: 2 additions & 6 deletions lib/bait/gen/c/type.bt
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,8 @@ fun (mut g Gen) write_types() {
g.type_impls_out += '};\n'
} else if sym.info is ast.ArrayInfo {
g.type_defs_out += 'typedef Array ${cname};\n'
} else {
match sym.kind {
.alias_type {
g.type_defs_out += 'typedef ${g.typ(sym.parent)} ${cname};\n'
}
}
} else if sym.kind == .alias_type {
g.type_defs_out += 'typedef ${g.typ(sym.parent)} ${cname};\n'
}
}
}
1 change: 1 addition & 0 deletions lib/bait/gen/js/comptime.bt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ fun (mut g Gen) get_comptime_val(name string, pos token.Pos) string {
'BAITEXE' { g.comptime_baitexe() }
'BAITDIR' { g.comptime_baitdir() }
'BAITHASH' { g.comptime_baithash() }
else { '' }
}
}

Expand Down
16 changes: 8 additions & 8 deletions lib/bait/parser/type.bt
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,13 @@ fun (mut p Parser) generic_type_names() []string {
}

fun (p Parser) infer_expr_type(expr ast.Expr) ast.Type {
match expr {
ast.BoolLiteral{ return ast.BOOL_TYPE }
ast.CharLiteral{ return ast.U8_TYPE }
ast.IntegerLiteral{ return ast.I32_TYPE }
ast.StringLiteral{ return ast.STRING_TYPE }
ast.StructInit{ return expr.typ }
ast.MapInit{ return expr.typ }
return match expr {
ast.BoolLiteral{ ast.BOOL_TYPE }
ast.CharLiteral{ ast.U8_TYPE }
ast.IntegerLiteral{ ast.I32_TYPE }
ast.StringLiteral{ ast.STRING_TYPE }
ast.StructInit{ expr.typ }
ast.MapInit{ expr.typ }
else { ast.VOID_TYPE }
}
return ast.VOID_TYPE
}
1 change: 1 addition & 0 deletions tests/match/expr_test.bt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ fun get_num() i32 {
return match x {
'a' { 1 }
'b' { 2 }
else { 0 }
}
}

Expand Down
50 changes: 50 additions & 0 deletions tests/match/match_test.bt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,57 @@ fun test_type_alias(){
match i {
INT_1 { assert false }
INT_5 { a += 1 }
else { assert false }
}

assert a == 1
}

enum Color {
red
green
blue
}

fun test_exhaustive_enum() {
mut ok := 0
c := Color.green

// All variants covered
match c {
.red { assert false }
.green { ok += 1 }
.blue { assert false }
}

// Has else
match c {
.blue { assert false }
else { ok += 1 }
}

assert ok == 2
}

struct Mars{}
struct Venus{}
type Planet := Mars | Venus

fun test_exhaustive_sumtype() {
mut ok := 0
p := Mars{} as Planet

// All variants covered
match p {
Mars { ok += 1}
Venus { assert false }
}

// Has else
match p {
Venus { assert false }
else { ok += 1 }
}

assert ok == 2
}
1 change: 0 additions & 1 deletion tests/match/sumtype_test.bt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ fun test_sumtype_match() {
assert true
}
Bar { assert false }
else { assert false }
}
assert n == 1
}
1 change: 1 addition & 0 deletions tests/out/error/match/case_handled.in.bt
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ match x {
2 {}
1 {}
2 {}
else {}
}
11 changes: 11 additions & 0 deletions tests/out/error/match/else_unreachable.in.bt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
enum JobStatus {
unemployed
employed
}

s := JobStatus.employed
match s {
.unemployed {}
.employed {}
else {}
}
1 change: 1 addition & 0 deletions tests/out/error/match/else_unreachable.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
tests/out/error/match/else_unreachable.in.bt:7:0 warning: match is exhaustive, else is unreachable
19 changes: 19 additions & 0 deletions tests/out/error/match/not_exhaustive.in.bt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
enum Color {
red
green
blue
}

struct Mars{}
struct Venus{}
type Planet := Mars | Venus

c := Color.green
match c {
.blue {}
}

p := Mars{} as Planet
match p {
Mars {}
}
2 changes: 2 additions & 0 deletions tests/out/error/match/not_exhaustive.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
tests/out/error/match/not_exhaustive.in.bt:12:0 error: match must be exhaustive (add `else {}` or branches for red, green)
tests/out/error/match/not_exhaustive.in.bt:17:0 error: match must be exhaustive (add `else {}` or branches for Venus)
1 change: 1 addition & 0 deletions tests/out/error/other/match.in.bt
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ match n {
'asd' {}
456 {}
true {}
else {}
}

0 comments on commit c402b02

Please sign in to comment.