Phi missing predecessor in CFG
| Affects | Status | Importance | Assigned to | Milestone | |
|---|---|---|---|---|---|
| Mars |
Fix Released
|
Critical
|
Matt Giuca | ||
Bug Description
The following code results in an error "Phi missing predecessor in CFG" if you run twoswitch(Nothing, Nothing):
type Maybe(a):
Nothing
Just(a)
def twoswitch(x :: Maybe(a), y :: Maybe(a)) :: Int:
switch x:
case Nothing:
switch y:
case Just(m):
return 2
There is flaw in the generation of the phi for the exit block in ast_cfg. reconcile_def_map is supposed to avoid generating a Phi if the variable is not bound in all predecessors. However, the PredMap for the variable "m" is:
{bbref(4): bound(m:2), bbref(7): bound(m:0)}
Note that unbound variables are supposed to have a mapping entry for each predecessor. 3 is a predecessor. "bbref(3): unbound" should be in the PredMap, not be omitted. (This would cause reconcile_def_map to avoid generating the phi at all, which is what we want, since the variable m can't be accessed from the exit block.)
This is caused by the fact that the variable m is not actually in-scope in block 3, and therefore it isn't considered "unbound" -- it is not considered to exist.
Related branches
- Matt Giuca: Approve
-
Diff: 2876 lines (+1121/-410)53 files modifieddoc/intro.rst (+3/-3)
doc/ref/procedures.rst (+23/-6)
doc/ref/types.rst (+152/-5)
lib/prelude.mar (+2/-2)
src/ast_cfg.m (+269/-157)
src/builtins.m (+24/-24)
src/interactive.m (+3/-3)
src/ir.m (+6/-2)
src/mars.m (+2/-2)
src/marsc.m (+2/-2)
src/parsem.m (+7/-2)
src/pretty.m (+2/-1)
src/tables.m (+13/-1)
src/typecheck.m (+243/-177)
src/types.m (+7/-1)
src/util.m (+16/-0)
test/cases/compiler/implicitvar.mar (+47/-0)
test/cases/compiler/patterntypeerror.mar (+27/-0)
test/cases/compiler/patterntypeerror.mtc (+8/-0)
test/cases/compiler/phipred.mtc (+0/-1)
test/cases/compiler/switch.mar (+33/-0)
test/cases/compiler/switch.mtc (+0/-5)
test/cases/semantic/assignglobal.mar (+3/-1)
test/cases/semantic/assignglobal.mtc (+1/-3)
test/cases/semantic/casetypecheck.mtc (+0/-1)
test/cases/semantic/dupfield.mar (+3/-2)
test/cases/semantic/dupfield.mtc (+4/-2)
test/cases/semantic/dupfield2.mar (+5/-0)
test/cases/semantic/dupfield2.mtc (+5/-0)
test/cases/semantic/dupfield3.mtc (+1/-1)
test/cases/semantic/dupvarcase.mar (+16/-0)
test/cases/semantic/dupvarcase.mtc (+8/-0)
test/cases/semantic/dupvarcase2.mar (+17/-0)
test/cases/semantic/dupvarcase2.mtc (+8/-0)
test/cases/semantic/localreadwrite.mtc (+0/-1)
test/cases/semantic/localreadwrite2.mar (+12/-0)
test/cases/semantic/localreadwrite2.mtc (+5/-0)
test/cases/semantic/localtypevar.mar (+14/-0)
test/cases/semantic/localtypevar.mtc (+7/-0)
test/cases/semantic/localtypevar2.mar (+10/-0)
test/cases/semantic/localtypevar2.mtc (+6/-0)
test/cases/semantic/monomorphism.mar (+16/-0)
test/cases/semantic/monomorphism.mtc (+9/-0)
test/cases/semantic/monomorphism2.mar (+23/-0)
test/cases/semantic/monomorphism2.mtc (+9/-0)
test/cases/semantic/typeerror2.mar (+6/-0)
test/cases/semantic/typeerror2.mtc (+9/-0)
test/cases/semantic/typeerror3.mar (+6/-0)
test/cases/semantic/typeerror3.mtc (+9/-0)
test/cases/semantic/typeerror4.mar (+6/-0)
test/cases/semantic/typeerror4.mtc (+9/-0)
test/cases/semantic/undefvar2.mar (+4/-2)
test/cases/semantic/undefvar2.mtc (+1/-3)
| description: | updated |
| description: | updated |
| Changed in mars: | |
| importance: | High → Critical |
| status: | Incomplete → Triaged |
| Matt Giuca (mgiuca) wrote | #1 |
| Changed in mars: | |
| status: | Triaged → Fix Committed |
| Matt Giuca (mgiuca) wrote | #2 |
| Changed in mars: | |
| status: | Fix Committed → Fix Released |
A (possibly dodgy) solution to this is to avoid any block-level scoped variables. If all variables are scoped to the function, then they will definitely appear in the PredMap.
This was not always the case, but as of newtypes branch, r1019, it is (bug #513638). The above example no longer breaks from that revision onwards.
Comment in ast_cfg explaining this, in newtypes r1038 (considered fixed).