-
Notifications
You must be signed in to change notification settings - Fork 3.1k
SI-9089 Another REPL/FSC + specialization bug fix #4249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Review by @VladUreche @lrytz |
|
Before I left the office, I started looking at the previous PR to get oriented, but I guess that in my time zone, I'll have to get up pretty early in the morning to make a contribution. |
60851c7 to
87742b2
Compare
|
LGTM! Seems adding the member to the scope is already done by adding it to |
The enclosed test case stopped working in 2.11.5 on the back of scala#4040. The key change was that we ran all post-typer info transformers on each run of the compiler, rather than trying to reuse the results of the previous run. In that patch, I noticed one place [1] in specialization that aggressively entered specialized members into the owning scope, rather than relying on `transformInfo` to place the new members in the scope of the newly created element of the info history. I made that change after noticing that this code could actually mutated scopes of specializaed types at the parser phase, which led to fairly obscure failures. This bug is another one of these obscure failures, and has the same root cause. We effectively "double specialiaze" Function0, which trips an assertion when `method apply$mcI$sp` is found twice in a scope. I have found another spot that was directly manipulating the scope, and removed the offending code. [1] scala#4040 (comment)
|
LGTM, thanks for the links Vlad. |
SI-9089 Another REPL/FSC + specialization bug fix
|
Ditto the thanks. We don't take you for granted. |
|
Is this pull request also fixing the following compiler error (Java 1.8.0_25 and Scala 2.11.5)? crashes the compiler: |
|
Hi @hjacobs. Yes it does fix this problem. |
The enclosed test case stopped working in 2.11.5 on the back of
#4040.
The key change was that we ran all post-typer info transformers
on each run of the compiler, rather than trying to reuse the results
of the previous run.
In that patch, I noticed another place in specialization that
aggressively entered specialized members into the owning scope,
rather than relying on
transformInfoto place the new membersin the scope of the newly created element of the info history.
I made that change after noticing that this code could actually
mutated scopes of specializaed types at the parser phase, which
led to fairly obscure failures.
This bug is another one of these obscure failures, and has the
same root cause. We effectively "double specialiaze" Function0,
which trips an assertion when
method apply$mcI$spis foundtwice in a scope.
I have found another spot that was directly manipulating the scope,
and removed the offending code.