Fix bug in python if/else chains #1183
Conversation
| expr match { | ||
| case If(c1, t1, e1) => | ||
| val (ifs, e2) = combine(e1) | ||
| (((c1, t1)) :: ifs, e2) |
There was a problem hiding this comment.
previously this code was in PythonGen, but it really only depends on Matchless.Expr. The order was being reversed by appending to the tail: ifs :+ ((c1, t1)) which for some branches changes semantics (where Matchless has leveraged prior information to do less work).
| expr match { | ||
| case If(c1, t1, e1) => | ||
| val (ifs, e2) = combine(e1) | ||
| (ifs :+ ((c1, t1)), e2) |
There was a problem hiding this comment.
note the order is wrong here: we are pushing c1, t1 on to the end of ifs but that came from expanding e1 so it should come before e1 since it was before originally.
| @@ -151,4 +156,21 @@ class MatchlessTest extends AnyFunSuite { | |||
| assert(matchlessRes == matchRes) | |||
| } | |||
| } | |||
|
|
|||
| test("If.flatten can be unflattened") { | |||
There was a problem hiding this comment.
this test didn't always find the bug (since randomly finding a case is somewhat rare, but after 3 runs, one found it, so I think it would be flakey enough that we would discover the issue).
| @@ -71,6 +67,10 @@ def cmp_BinNat(a: BinNat, b: BinNat) -> Comparison: | |||
| case GT | EQ: GT | |||
| case LT: LT | |||
| case Zero: GT | |||
| case Zero: | |||
There was a problem hiding this comment.
this is more efficient since Zero is rare, but triggered the bug before this fix.
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1183 +/- ##
==========================================
+ Coverage 91.32% 91.68% +0.36%
==========================================
Files 96 96
Lines 11846 12011 +165
Branches 2675 2819 +144
==========================================
+ Hits 10818 11012 +194
+ Misses 1028 999 -29 ☔ View full report in Codecov by Sentry. |
close #1181
This bug has been lurking in python code generation for some time, but only with generative testing did we push hard enough to find it.