Fix #9107: Restore constraints after MT reduction #9108
Conversation
OlivierBlanvillain
force-pushed
the
fix-9107
branch
from
785d314 to
071a8a9
Compare
| recur(cases) | ||
| // inFrozenConstraint is not sufficient here as it doesn't prevent | ||
| // parameters of type lambdas from being added to `constraint`, see | ||
| // ConstraintHandling.canConstrain and tests/neg/9107.scala. |
There was a problem hiding this comment.
But why does canConstrain behave that way? To me it seems pretty surprising that isFrozenConstraint does not in fact fully freeze the constraints
There was a problem hiding this comment.
There's an exception made for parameters of cases that are tested in a match type. These need to be instantiatable even though the constraint as a whole is frozen.
Maybe resetting the constraint should be done in matchCase? That's where we set caseLambda and where we instantiate it at the end if it's a match. So it seems to be cleaner to reset the constraint there.
| // `6570-1.scala` for examples that exploit emptiness to break match | ||
| // type soundness. | ||
|
|
||
| // If we revered the uncertainty case of this empty check, that is, |
| recur(cases) | ||
| // inFrozenConstraint is not sufficient here as it doesn't prevent | ||
| // parameters of type lambdas from being added to `constraint`, see | ||
| // ConstraintHandling.canConstrain and tests/neg/9107.scala. |
There was a problem hiding this comment.
There's an exception made for parameters of cases that are tested in a match type. These need to be instantiatable even though the constraint as a whole is frozen.
Maybe resetting the constraint should be done in matchCase? That's where we set caseLambda and where we instantiate it at the end if it's a match. So it seems to be cleaner to reset the constraint there.
That doesn't seem to work, I get an assertion error during bootstrap: [error] ## Exception when compiling 204 sources to /home/olivier/workspace/dotty/library/../out/bootstrap/dotty-library-bootstra
pped/scala-0.26/classes
[error] java.lang.AssertionError: assertion failed: param = x
[error] constraint = Constraint(
[error] uninstVars = ;
[error] constrained types =
[error] bounds =
[error] ordering =
[error] )
[error] dotty.DottyPredef$.assertFail(DottyPredef.scala:17)
[error] dotty.tools.dotc.core.ConstraintHandling.approximation(ConstraintHandling.scala:292) |
OlivierBlanvillain
force-pushed
the
fix-9107
branch
from
071a8a9 to
9dc889d
Compare
OlivierBlanvillain
force-pushed
the
fix-9107
branch
2 times, most recently
from
51fc0bf to
1167ac2
Compare
Because of `caseLambda = constrained(cas)` in `def matchCase`, subtyping wrongly things that it's OK to further constrain types introduced in match type patterns. This commit fixes the issue with a stronger version of `inFrozenConstraint` around match type reduction.
This subtyping check use to work because of the bug in question. Note that 6697 tries to use GADT knowledge that is currently unavailable on expression patterns (as demonstrated in 6697-2.scala), so I don't think we are in a urge to support this with match types.
OlivierBlanvillain
force-pushed
the
fix-9107
branch
from
1167ac2 to
c20a8f9
Compare
Because of
caseLambda = constrained(cas)indef matchCase, subtyping wrongly things that it's OK to further constrain types introduced in match type patterns. This commit fixes the issue with a stronger version ofinFrozenConstraintaround match type reduction.