Fix caching bug: don't assume that tvars instantiation cannot be retracted #1020
+4
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/rebuild |
smarter
force-pushed
the
fix/tvar-ephemeral
branch
from
24dbdce to
ad1090a
Compare
|
@DarkDimius : note that I'm getting test failures (https://scala-ci.typesafe.com/job/dotty-master-validate-partest/900/) with the non-bootstrapped partest |
|
/rebuild |
|
Good to know that failures are independent from bootstrap. |
|
/rebuild |
1 similar comment
|
/rebuild |
smarter
force-pushed
the
fix/tvar-ephemeral
branch
4 times, most recently
from
c688907 to
438702c
Compare
|
/rebuild |
2 similar comments
|
/rebuild |
|
/rebuild |
|
needs to be rebased to master |
smarter
force-pushed
the
fix/tvar-ephemeral
branch
from
438702c to
b4f335e
Compare
|
Rebased. |
…acted When TypeVar#inst is empty but an instantiation exists in the typer state, we should set ephemeral to true, because this instantiation will be retracted if we throw away the current typer state. This makes hkrange.scala pass, it compiled before but the type parameter of `f` was inferred to be `Nothing` because of this bug, and this failed Ycheck. For anyone who wonders how caching bugs manifest themselves, here's what happened in details in hkrange.scala: 1. In an ExploreTyperState we set `CC` to be `IndexedSeq` in the constraint set 2. In that same typer state the TypeRef `CC[Int]` (it's a TypeRef because `CC` is a type lambda) gets the denotation `IndexedSeq[Int]`, which is correct, but the denotation is cached since `ephemeral` is false, which is wrong. 3. Later, we retract the ExplorerTyperState, so `CC` is uninstantiated again and unconstrained. 4. Then we do the subtyping check `CC[Int] <:< IndexedSeq[Int]`, because the denotation of `CC[Int]` was cached, this returns true, but `CC` stays unconstrained. 5. This means that when we instantiate `CC`, we get `Nothing` After this fix, the TypeRef denotation is no longer cached, so when we do `CC[Int] <:< IndexedSeq[Int]`, `CC` gets constrained as expected.
smarter
force-pushed
the
fix/tvar-ephemeral
branch
from
b4f335e to
0a2b676
Compare
|
/synch |
|
LGTM |
odersky
added a commit
that referenced
this pull request
Jan 18, 2016
Fix caching bug: don't assume that tvars instantiation cannot be retracted
OlivierBlanvillain
pushed a commit
to OlivierBlanvillain/dotty
that referenced
this pull request
Dec 8, 2016
OlivierBlanvillain
pushed a commit
to OlivierBlanvillain/dotty
that referenced
this pull request
Dec 12, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When TypeVar#inst is empty but an instantiation exists in the typer
state, we should set ephemeral to true, because this instantiation will
be retracted if we throw away the current typer state.
This makes hkrange.scala pass, it compiled before but the type parameter
of
fwas inferred to beNothingbecause of this bug, and this failedYcheck.
For anyone who wonders how caching bugs manifest themselves, here's what
happened in details in hkrange.scala:
CCto beIndexedSeqin theconstraint set
CC[Int](it's a TypeRefbecause
CCis a type lambda) gets the denotationIndexedSeq[Int],which is correct, but the denotation is cached since
ephemeralisfalse, which is wrong.
CCis uninstantiatedagain and unconstrained.
CC[Int] <:< IndexedSeq[Int], becausethe denotation of
CC[Int]was cached, this returns true, butCCstaysunconstrained.
CC, we getNothingAfter this fix, the TypeRef denotation is no longer cached, so when we
do
CC[Int] <:< IndexedSeq[Int],CCgets constrained as expected.Review by @odersky