Remove tvars introduced while testing normalizedCompatible #21466
Conversation
|
@odersky I assume you've seen this as well. I'm not sure of the best way to fix this, but this way works and seems clean enough. But I'm happy to experiment with other ways, if you have ideas. |
|
I am out of bandwidth to page in the context for this one. Maybe @smarter can take a look? |
dwijnand
force-pushed
the
dnw-drop-lambda
branch
from
820c0ca
to
ca72e16
Compare
dwijnand
force-pushed
the
dnw-drop-lambda
branch
2 times, most recently
from
57522df
to
b6f36d0
Compare
| // This is done to compensate for the fact that normally every | ||
| // reference to a polytype would have to be a fresh copy of that type, | ||
| // but we want to avoid that because it would increase compilation cost. | ||
| // See pos/i6682a.scala for a test case where the defensive copying matters. | ||
| val ensureFresh = new TypeMap with CaptureSet.IdempotentCaptRefMap: |
There was a problem hiding this comment.
How are the changes in this file and Typer related to the change in ProtoTypes?
There was a problem hiding this comment.
So the ones in Typer, i.e. when adapting by applying fresh type vars, is necessary for the recursive programs like i6682a (and in the comment). I then randomly grepped the reference i6682a and found the code in TypeAssigner, so I started expanding my Typer comment. But I noticed that the way its done in TypeAssigner is in cloning the lambda in the args. So I thought I could hit two birds with one stone by (1) do it consistently in both (making it easier to document) and (2) do it more efficiently in TypeAssigner by doing it once rather than for each argument.
| // variable. NOTE: this does not suffice to discard type variables | ||
| // in ancestors of `protoTyperState`, if this situation ever | ||
| // comes up, an assertion in TyperState will trigger and this code | ||
| // will need to be generalized. |
There was a problem hiding this comment.
I believe this code is flawed since the situation in the NOTE can happen which I think is what causes #21555 and similar issues, but it's not clear to me that just removing this is fine, and it seems unrelated to the rest of this PR?
There was a problem hiding this comment.
I went to reverify things, and I can't find a test case that fails without your code here (and without anything in this PR). So I can't say that my code replaces it, because it doesn't seem to do anything for any of our tests. So I put it back (by dropping my commit).
dwijnand
force-pushed
the
dnw-drop-lambda
branch
from
1d009a4
to
6d9c46b
Compare
|
@smarter good now, right? |
dwijnand
force-pushed
the
dnw-drop-lambda
branch
from
6d9c46b
to
be08cec
Compare
Having looked at the type var constraints for so long, I wanted to finally get rid of these temporary typevars (and a type lambda) added for the purpose of typing a qualifier against a selection prototype, and when the selection is actually typed, brand new typevars (and a type lambda) are added. Given it's from the same type lambda, they're necessarily all called the same, so they're indistinguishable in the constraint debugging.