SI-1909 SI-3832 SI-7007 SI-7223 Improved handling of larval objects#2884
SI-1909 SI-3832 SI-7007 SI-7223 Improved handling of larval objects#2884retronym merged 5 commits intoscala:masterfrom
Conversation
|
I'll close this temporarily until I address the failures. |
|
Reopened for business. |
|
I'm done with this one. I can't actually find anything residual from SI-6666, so we can close that one too. |
|
@JamesIry: ping |
|
What's the status of this PR? In particular, will it make to M5? |
|
@magarciaEPFL: I hope so. I'll try to steal @JamesIry's attention tomorrow. |
|
@gkossakowski Did you get @JamesIry 's attention yesterday? |
|
I apologize on behalf of the reviewer for this not making the deadline. |
|
This one isn't super urgent. |
|
@retronym: please fix a19e05371fb0a57263b68c1ced0ba88e830e4a61 (both commit message and test file names). Other than that it can go in. Related to this PR: wouldn't make sense to perform all those checks at some late phase (genicode) when we see the final shape of trees and we perform checks that are very backend specific? |
When we're in the neighbourhood of VerifyErrors, it's better to run the code. This change is leading up to a fix for SI-3832, which regressed with fix for SI-1909.
In order to reduce the noise in OuterPathTransformer.
SI-1909 modified LambdaLift to lift in auxiliary constructors methods as STATIC
so they could be called before the self-constructor was called.
That allowed for:
class Foo (x: Int) {
def this() = this( {
def bar() = 5
bar
})
}
However, if the method is in a statement that trails the self constructor call,
this is unnecessary and in fact incorrect as it robs the lifted method of `this`.
This commit uses the machinery established in SI-6666 to limit the STATIC-ness
of lifted methods to those used in arguments for self-constructor calls.
This is used exclusively; the `isAuxillaryConstructor` check wasn't the right
way to solve this, as was seen by the regression it caused in SI-3832.
A new test case shows that we can statically lift methods in super-constructor
calls, rather than just self-constructor calls.
We also have to avoid statically lifting objects in these positions. For now,
I just emit a dev warning that a VerifyError is in your future. With some more
thought we could escalate that to a implementation restriction and emit an error.
Rather than the old behaviour, which compiled successfully but led us into the jaws of a LinkageError. Related to SI-6666.
|
I've just rebased and fixed the last commit message and test file name. |
It's a clunky flag used to determine very early on whether we're in the self-call, super-call or early-init section. In SI-6666 / fd61254, the check was improved to consider nesting. But, that caused this regression, as Function's haven't been translated to classes yet, so our check for enclosing non-term owners failed wrongly flagged definitins body of a anonymous function as INCONSTRUCTOR. With this patch, we correctly flag: class C extends D { // INCONSTRUCTOR () => { !INCONSTRUCTOR } // INCONSTRUCTOR }
SI-1909 SI-3832 SI-7007 SI-7223 Improved handling of larval objects
Reclaims some ground lost in the first swing at SI-6666, and solves a few more issues to boot.
I'm hopeful that I'll be able to knock off the residuals of SI-6666 too, so I might push one or two more commits.
Review by @JamesIry