Skip to content

Commit 47121ec

Browse files
akonwiakonwi
authored
fix(checker): preserve else-if chains (#112)
1 parent 0580fca commit 47121ec

6 files changed

Lines changed: 127 additions & 121 deletions

File tree

‎TODO.md‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
11
# TODO
2-
- [ ] Fix Go target `else if` lowering bug
3-
* Repro seen in `~/Developer/agent/vaxis-ard/counter`: an `if/else if` chain for key handling lowered so later branches collapsed into a broad fallback; `r` reset was treated as decrement.
4-
* Add a compiler regression with an `else if` chain where an early branch, middle branch, and final fallback produce distinct values/effects.
52
- [ ] try to improve FFI perf by autocasting args to externs
63
- [ ] allow FFI in user land
74
- [ ] make duplicate FFI binding registration return an error

‎compiler/air/lower.go‎

Lines changed: 21 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2741,53 +2741,39 @@ func (fl *functionLowerer) lowerConditionalCases(typeID TypeID, cases []checker.
27412741
}
27422742

27432743
func (fl *functionLowerer) lowerIf(typeID TypeID, expr *checker.If) (*Expr, error) {
2744-
condition, err := fl.lowerExpr(expr.Condition)
2745-
if err != nil {
2746-
return nil, err
2744+
if len(expr.Branches) == 0 {
2745+
return nil, fmt.Errorf("if expression missing branches")
2746+
}
2747+
return fl.lowerIfBranches(typeID, expr.Branches, expr.Else)
2748+
}
2749+
2750+
func (fl *functionLowerer) lowerIfBranches(typeID TypeID, branches []checker.IfBranch, elseBlock *checker.Block) (*Expr, error) {
2751+
if len(branches) == 0 {
2752+
return nil, fmt.Errorf("if expression missing branches")
27472753
}
2748-
thenBlock, err := fl.lowerBlockWithDefault(expr.Body.Stmts, typeID)
2754+
branch := branches[0]
2755+
condition, err := fl.lowerExpr(branch.Condition)
27492756
if err != nil {
27502757
return nil, err
27512758
}
2752-
elseBlock, err := fl.lowerElse(expr, typeID)
2759+
thenBlock, err := fl.lowerBlockWithDefault(branch.Body.Stmts, typeID)
27532760
if err != nil {
27542761
return nil, err
27552762
}
2756-
return &Expr{Kind: ExprIf, Type: typeID, Condition: condition, Then: thenBlock, Else: elseBlock}, nil
2757-
}
2758-
2759-
func (fl *functionLowerer) lowerElse(expr *checker.If, defaultType TypeID) (Block, error) {
2760-
if expr.ElseIf != nil {
2761-
condition, err := fl.lowerExpr(expr.ElseIf.Condition)
2763+
var airElse Block
2764+
if len(branches) > 1 {
2765+
nested, err := fl.lowerIfBranches(typeID, branches[1:], elseBlock)
27622766
if err != nil {
2763-
return Block{}, err
2764-
}
2765-
thenBlock, err := fl.lowerBlockWithDefault(expr.ElseIf.Body.Stmts, defaultType)
2766-
if err != nil {
2767-
return Block{}, err
2767+
return nil, err
27682768
}
2769-
elseBlock, err := fl.lowerElse(expr.ElseIf, defaultType)
2769+
airElse = Block{Result: nested}
2770+
} else if elseBlock != nil {
2771+
airElse, err = fl.lowerBlockWithDefault(elseBlock.Stmts, typeID)
27702772
if err != nil {
2771-
return Block{}, err
2772-
}
2773-
if expr.Else != nil {
2774-
elseBlock, err = fl.lowerBlockWithDefault(expr.Else.Stmts, defaultType)
2775-
if err != nil {
2776-
return Block{}, err
2777-
}
2773+
return nil, err
27782774
}
2779-
return Block{Result: &Expr{
2780-
Kind: ExprIf,
2781-
Type: defaultType,
2782-
Condition: condition,
2783-
Then: thenBlock,
2784-
Else: elseBlock,
2785-
}}, nil
2786-
}
2787-
if expr.Else != nil {
2788-
return fl.lowerBlockWithDefault(expr.Else.Stmts, defaultType)
27892775
}
2790-
return Block{}, nil
2776+
return &Expr{Kind: ExprIf, Type: typeID, Condition: condition, Then: thenBlock, Else: airElse}, nil
27912777
}
27922778

27932779
func (fl *functionLowerer) lowerResultConstructor(kind ExprKind, typeID TypeID, call *checker.ModuleFunctionCall) (*Expr, error) {

‎compiler/checker/checker.go‎

Lines changed: 44 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -2142,6 +2142,49 @@ func (c *Checker) extractGenericBindingsFromSpecializedStruct(originalDef, speci
21422142
return bindings
21432143
}
21442144

2145+
func (c *Checker) checkIfChain(s *parse.IfStatement) Expression {
2146+
if s == nil || s.Condition == nil {
2147+
return nil
2148+
}
2149+
branches := []IfBranch{}
2150+
var elseBlock *Block
2151+
var referenceType Type
2152+
current := s
2153+
for current != nil {
2154+
if current.Condition == nil {
2155+
block := c.checkBlock(current.Body, nil)
2156+
if referenceType != nil && !block.Type().equal(referenceType) {
2157+
c.addError("All branches must have the same result type", current.GetLocation())
2158+
return nil
2159+
}
2160+
elseBlock = block
2161+
break
2162+
}
2163+
condition := c.checkExpr(current.Condition)
2164+
if condition == nil {
2165+
return nil
2166+
}
2167+
if condition.Type() != Bool {
2168+
c.addError("If conditions must be boolean expressions", current.GetLocation())
2169+
return nil
2170+
}
2171+
body := c.checkBlock(current.Body, nil)
2172+
if referenceType == nil {
2173+
referenceType = body.Type()
2174+
} else if !body.Type().equal(referenceType) {
2175+
c.addError("All branches must have the same result type", current.GetLocation())
2176+
return nil
2177+
}
2178+
branches = append(branches, IfBranch{Condition: condition, Body: body})
2179+
next, ok := current.Else.(*parse.IfStatement)
2180+
if !ok {
2181+
break
2182+
}
2183+
current = next
2184+
}
2185+
return &If{Branches: branches, Else: elseBlock}
2186+
}
2187+
21452188
func (c *Checker) checkExpr(expr parse.Expression) Expression {
21462189
if c.halted {
21472190
return nil
@@ -3022,65 +3065,7 @@ func (c *Checker) checkExpr(expr parse.Expression) Expression {
30223065
}
30233066
}
30243067
case *parse.IfStatement:
3025-
{
3026-
cond := c.checkExpr(s.Condition)
3027-
if cond == nil {
3028-
return nil
3029-
}
3030-
if cond.Type() != Bool {
3031-
c.addError("If conditions must be boolean expressions", s.GetLocation())
3032-
return nil
3033-
}
3034-
3035-
body := c.checkBlock(s.Body, nil)
3036-
3037-
var elseIf *If
3038-
var elseBody *Block
3039-
3040-
// does not recurse. reach into AST for each level since it's fixed
3041-
if s.Else != nil {
3042-
next := s.Else.(*parse.IfStatement)
3043-
if next.Condition != nil {
3044-
cond := c.checkExpr(next.Condition)
3045-
if cond == nil {
3046-
return nil
3047-
}
3048-
if cond.Type() != Bool {
3049-
c.addError("If conditions must be boolean expressions", next.GetLocation())
3050-
return nil
3051-
}
3052-
3053-
elseIfBody := c.checkBlock(next.Body, nil)
3054-
if elseIfBody.Type() != body.Type() {
3055-
c.addError("All branches must have the same result type", next.GetLocation())
3056-
return nil
3057-
}
3058-
3059-
elseIf = &If{
3060-
Condition: cond,
3061-
Body: elseIfBody,
3062-
}
3063-
3064-
if next, ok := next.Else.(*parse.IfStatement); ok {
3065-
elseBody = c.checkBlock(next.Body, nil)
3066-
}
3067-
} else {
3068-
b := c.checkBlock(next.Body, nil)
3069-
if b.Type() != body.Type() {
3070-
c.addError("All branches must have the same result type", next.GetLocation())
3071-
return nil
3072-
}
3073-
elseBody = b
3074-
}
3075-
}
3076-
3077-
return &If{
3078-
Condition: cond,
3079-
Body: body,
3080-
ElseIf: elseIf,
3081-
Else: elseBody,
3082-
}
3083-
}
3068+
return c.checkIfChain(s)
30843069
case *parse.FunctionDeclaration:
30853070
return c.checkFunction(s, nil)
30863071
case *parse.ExternalFunction:

‎compiler/checker/checker_test.go‎

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,12 +1038,14 @@ func TestIfStatements(t *testing.T) {
10381038
},
10391039
{
10401040
Expr: &checker.If{
1041-
Condition: &checker.Variable{},
1042-
Body: &checker.Block{
1043-
Stmts: []checker.Statement{
1044-
{Expr: &checker.StrLiteral{"on"}},
1041+
Branches: []checker.IfBranch{{
1042+
Condition: &checker.Variable{},
1043+
Body: &checker.Block{
1044+
Stmts: []checker.Statement{
1045+
{Expr: &checker.StrLiteral{"on"}},
1046+
},
10451047
},
1046-
},
1048+
}},
10471049
},
10481050
},
10491051
},
@@ -1073,12 +1075,14 @@ func TestIfStatements(t *testing.T) {
10731075
Statements: []checker.Statement{
10741076
{
10751077
Expr: &checker.If{
1076-
Condition: &checker.BoolLiteral{true},
1077-
Body: &checker.Block{
1078-
Stmts: []checker.Statement{
1079-
{Expr: &checker.StrLiteral{"bar"}},
1078+
Branches: []checker.IfBranch{{
1079+
Condition: &checker.BoolLiteral{true},
1080+
Body: &checker.Block{
1081+
Stmts: []checker.Statement{
1082+
{Expr: &checker.StrLiteral{"bar"}},
1083+
},
10801084
},
1081-
},
1085+
}},
10821086
Else: &checker.Block{
10831087
Stmts: []checker.Statement{
10841088
{Expr: &checker.StrLiteral{"baz"}},
@@ -1104,17 +1108,21 @@ func TestIfStatements(t *testing.T) {
11041108
Statements: []checker.Statement{
11051109
{
11061110
Expr: &checker.If{
1107-
Condition: &checker.BoolLiteral{true},
1108-
Body: &checker.Block{
1109-
Stmts: []checker.Statement{
1110-
{Expr: &checker.StrLiteral{"bar"}},
1111+
Branches: []checker.IfBranch{
1112+
{
1113+
Condition: &checker.BoolLiteral{true},
1114+
Body: &checker.Block{
1115+
Stmts: []checker.Statement{
1116+
{Expr: &checker.StrLiteral{"bar"}},
1117+
},
1118+
},
11111119
},
1112-
},
1113-
ElseIf: &checker.If{
1114-
Condition: &checker.BoolLiteral{false},
1115-
Body: &checker.Block{
1116-
Stmts: []checker.Statement{
1117-
{Expr: &checker.StrLiteral{"baz"}},
1120+
{
1121+
Condition: &checker.BoolLiteral{false},
1122+
Body: &checker.Block{
1123+
Stmts: []checker.Statement{
1124+
{Expr: &checker.StrLiteral{"baz"}},
1125+
},
11181126
},
11191127
},
11201128
},

‎compiler/checker/nodes.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -844,15 +844,21 @@ func (b *Block) Type() Type {
844844
return Void
845845
}
846846

847-
type If struct {
847+
type IfBranch struct {
848848
Condition Expression
849849
Body *Block
850-
ElseIf *If
851-
Else *Block
850+
}
851+
852+
type If struct {
853+
Branches []IfBranch
854+
Else *Block
852855
}
853856

854857
func (i *If) Type() Type {
855-
return i.Body.Type()
858+
if len(i.Branches) == 0 || i.Branches[0].Body == nil {
859+
return Void
860+
}
861+
return i.Branches[0].Body.Type()
856862
}
857863

858864
type ForIntRange struct {

‎compiler/go/parity_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,30 @@ func TestGoTargetParityCoreCorpus(t *testing.T) {
6060
}
6161
`,
6262
},
63+
{
64+
name: "if else if chain preserves all conditions",
65+
input: `
66+
fn classify(key: Str) Int {
67+
mut result = 0
68+
if key == "q" {
69+
result = 1
70+
} else if key == "up" {
71+
result = 2
72+
} else if key == "down" {
73+
result = 3
74+
} else if key == "r" {
75+
result = 4
76+
} else {
77+
result = 5
78+
}
79+
result
80+
}
81+
82+
fn main() Bool {
83+
classify("q") == 1 and classify("up") == 2 and classify("down") == 3 and classify("r") == 4 and classify("?") == 5
84+
}
85+
`,
86+
},
6387
{
6488
name: "inline block expression",
6589
input: `

0 commit comments

Comments
 (0)