-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Revert to the Scala 2 encoding for trait initialization and super calls #8652
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
Changes from all commits
90070e2
d495ee0
11de1d3
de5df36
ce4f7ba
3bd5da4
e12f1f9
d682455
d7bea54
84b230a
288fa1d
2eb743c
3f99915
d990e64
29a561b
cb84d04
80db021
a771574
1128a98
44de114
e88c5b7
64d3b9a
a754f24
77e2925
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,9 +14,9 @@ import dotty.tools.dotc.CompilationUnit | |
import dotty.tools.dotc.core.Annotations.Annotation | ||
import dotty.tools.dotc.core.Decorators._ | ||
import dotty.tools.dotc.core.Flags._ | ||
import dotty.tools.dotc.core.StdNames.str | ||
import dotty.tools.dotc.core.StdNames.{nme, str} | ||
import dotty.tools.dotc.core.Symbols._ | ||
import dotty.tools.dotc.core.Types.Type | ||
import dotty.tools.dotc.core.Types.{MethodType, Type} | ||
import dotty.tools.dotc.util.Spans._ | ||
import dotty.tools.dotc.report | ||
|
||
|
@@ -496,7 +496,23 @@ trait BCodeSkelBuilder extends BCodeHelpers { | |
|
||
case ValDef(name, tpt, rhs) => () // fields are added in `genPlainClass()`, via `addClassFields()` | ||
|
||
case dd: DefDef => genDefDef(dd) | ||
case dd: DefDef => | ||
/* First generate a static forwarder if this is a non-private trait | ||
* trait method. This is required for super calls to this method, which | ||
* go through the static forwarder in order to work around limitations | ||
* of the JVM. | ||
* In theory, this would go in a separate MiniPhase, but it would have to | ||
* sit in a MegaPhase of its own between GenSJSIR and GenBCode, so the cost | ||
* is not worth it. We directly do it in this back-end instead, which also | ||
* kind of makes sense because it is JVM-specific. | ||
*/ | ||
val sym = dd.symbol | ||
val needsStaticImplMethod = | ||
claszSymbol.isInterface && !dd.rhs.isEmpty && !sym.isPrivate && !sym.isStaticMember | ||
if needsStaticImplMethod then | ||
genStaticForwarderForDefDef(dd) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this be done in a mini-phase before the backend? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, because it must not be done for JS (nor Native). So it would have to be alone in its own MegaPhase between |
||
|
||
genDefDef(dd) | ||
|
||
case tree: Template => | ||
val body = | ||
|
@@ -537,6 +553,37 @@ trait BCodeSkelBuilder extends BCodeHelpers { | |
|
||
} // end of method initJMethod | ||
|
||
private def genStaticForwarderForDefDef(dd: DefDef): Unit = | ||
val forwarderDef = makeStaticForwarder(dd) | ||
genDefDef(forwarderDef) | ||
|
||
/* Generates a synthetic static forwarder for a trait method. | ||
* For a method such as | ||
* def foo(...args: Ts): R | ||
* in trait X, we generate the following method: | ||
* static def foo$($this: X, ...args: Ts): R = | ||
* invokespecial $this.X::foo(...args) | ||
* We force an invokespecial with the attachment UseInvokeSpecial. It is | ||
* necessary to make sure that the call will not follow overrides of foo() | ||
* in subtraits and subclasses, since the whole point of this forward is to | ||
* encode super calls. | ||
*/ | ||
private def makeStaticForwarder(dd: DefDef): DefDef = | ||
val origSym = dd.symbol.asTerm | ||
val name = traitSuperAccessorName(origSym) | ||
val info = origSym.info match | ||
case mt: MethodType => | ||
MethodType(nme.SELF :: mt.paramNames, origSym.owner.typeRef :: mt.paramInfos, mt.resType) | ||
val sym = origSym.copy( | ||
name = name.toTermName, | ||
flags = Method | JavaStatic, | ||
info = info | ||
) | ||
tpd.DefDef(sym.asTerm, { paramss => | ||
val params = paramss.head | ||
tpd.Apply(params.head.select(origSym), params.tail) | ||
.withAttachment(BCodeHelpers.UseInvokeSpecial, ()) | ||
}) | ||
|
||
def genDefDef(dd: DefDef): Unit = { | ||
val rhs = dd.rhs | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see there was some discussion on this in the PR, but it'd be great if this comment was expanded to explain exactly why this is needed.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added a comment on
BCodeSkilBuilder.makeStaticForwarder
with a summary of the discussion from this PR.