Skip to content

Commit bd26669

Browse files
retronymretronym
committed
Nuke trait implementation classes
 - Leave the members in the trait instead (these will be emitted as default methods in the interface) - Add the trait mixin constructor to the interface, rather than the impl class - Change the invocation of the mixin constructor to use an invokespecial. This is encoded with the AST: `Apply(Select(Super(_, _), mixinConstructor)))` I've tried to remove all traces of the interface / implclass distinction. To allow us to call arbitrary super methods with invokespecial, the backend will add a transitively inherited interface as a direct when needed to circumvent the JVM restriction that invokespecial can only use a direct parent as the receiver.
1 parent ce6ce56 commit bd26669

35 files changed

+250
-816
lines changed

‎src/compiler/scala/tools/nsc/ast/TreeGen.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ abstract class TreeGen extends scala.reflect.internal.TreeGen with TreeDSL {
145145
override def mkCast(tree: Tree, pt: Type): Tree = {
146146
debuglog("casting " + tree + ":" + tree.tpe + " to " + pt + " at phase: " + phase)
147147
assert(!tree.tpe.isInstanceOf[MethodType], tree)
148+
assert(!pt.isInstanceOf[MethodType], tree)
148149
assert(pt eq pt.normalize, tree +" : "+ debugString(pt) +" ~>"+ debugString(pt.normalize))
149150
atPos(tree.pos) {
150151
mkAsInstanceOf(tree, pt, any = !phase.next.erasedTypes, wrapInApply = isAtPhaseAfter(currentRun.uncurryPhase))

‎src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -588,12 +588,13 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
588588
// to call super constructors explicitly and/or use their 'returned' value.
589589
// therefore, we can ignore this fact, and generate code that leaves nothing
590590
// on the stack (contrary to what the type in the AST says).
591-
case Apply(fun @ Select(Super(_, _), _), args) =>
591+
case Apply(fun @ Select(Super(qual, mix), _), args) =>
592592
val invokeStyle = InvokeStyle.Super
593593
// if (fun.symbol.isConstructor) Static(true) else SuperCall(mix);
594594
mnode.visitVarInsn(asm.Opcodes.ALOAD, 0)
595+
val hostClass = qual.symbol.parentSymbols.find(_.name == mix).orNull
595596
genLoadArguments(args, paramTKs(app))
596-
genCallMethod(fun.symbol, invokeStyle, app.pos)
597+
genCallMethod(fun.symbol, invokeStyle, app.pos, hostClass)
597598
generatedType = methodBTypeFromSymbol(fun.symbol).returnType
598599

599600
// 'new' constructor call: Note: since constructors are
@@ -1050,19 +1051,26 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
10501051
hostSymbol.info ; methodOwner.info
10511052

10521053
def needsInterfaceCall(sym: Symbol) = (
1053-
sym.isInterface
1054+
sym.isTraitOrInterface
10541055
|| sym.isJavaDefined && sym.isNonBottomSubClass(definitions.ClassfileAnnotationClass)
10551056
)
10561057

1058+
val isTraitCallToObjectMethod =
1059+
hostSymbol != methodOwner && methodOwner.isTraitOrInterface && ObjectTpe.decl(method.name) != NoSymbol && method.overrideChain.last.owner == ObjectClass
1060+
10571061
// whether to reference the type of the receiver or
10581062
// the type of the method owner
1059-
val useMethodOwner = (
1063+
val useMethodOwner = ((
10601064
!style.isVirtual
10611065
|| hostSymbol.isBottomClass
10621066
|| methodOwner == definitions.ObjectClass
1063-
)
1067+
) && !(style.isSuper && hostSymbol != null)) || isTraitCallToObjectMethod
10641068
val receiver = if (useMethodOwner) methodOwner else hostSymbol
10651069
val jowner = internalName(receiver)
1070+
1071+
if (style.isSuper && (isTraitCallToObjectMethod || receiver.isTraitOrInterface) && !cnode.interfaces.contains(jowner))
1072+
cnode.interfaces.add(jowner)
1073+
10661074
val jname = method.javaSimpleName.toString
10671075
val bmType = methodBTypeFromSymbol(method)
10681076
val mdescr = bmType.descriptor
@@ -1342,7 +1350,7 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
13421350
def asmType(sym: Symbol) = classBTypeFromSymbol(sym).toASMType
13431351

13441352
val implMethodHandle =
1345-
new asm.Handle(if (lambdaTarget.hasFlag(Flags.STATIC)) asm.Opcodes.H_INVOKESTATIC else asm.Opcodes.H_INVOKEVIRTUAL,
1353+
new asm.Handle(if (lambdaTarget.hasFlag(Flags.STATIC)) asm.Opcodes.H_INVOKESTATIC else if (lambdaTarget.owner.isTrait) asm.Opcodes.H_INVOKEINTERFACE else asm.Opcodes.H_INVOKEVIRTUAL,
13461354
classBTypeFromSymbol(lambdaTarget.owner).internalName,
13471355
lambdaTarget.name.toString,
13481356
methodBTypeFromSymbol(lambdaTarget).descriptor)

‎src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala

Lines changed: 6 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
3232
* the InnerClass / EnclosingMethod classfile attributes. See comment in BTypes.
3333
*/
3434
def considerAsTopLevelImplementationArtifact(classSym: Symbol) =
35-
classSym.isImplClass || classSym.isSpecialized
35+
classSym.isSpecialized
3636

3737
/**
3838
* Cache the value of delambdafy == "inline" for each run. We need to query this value many
@@ -248,7 +248,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
248248
* Build the [[InlineInfo]] for a class symbol.
249249
*/
250250
def buildInlineInfoFromClassSymbol(classSym: Symbol, classSymToInternalName: Symbol => InternalName, methodSymToDescriptor: Symbol => String): InlineInfo = {
251-
val traitSelfType = if (classSym.isTrait && !classSym.isImplClass) {
251+
val traitSelfType = if (classSym.isTrait) {
252252
// The mixin phase uses typeOfThis for the self parameter in implementation class methods.
253253
val selfSym = classSym.typeOfThis.typeSymbol
254254
if (selfSym != classSym) Some(classSymToInternalName(selfSym)) else None
@@ -259,7 +259,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
259259
val isEffectivelyFinal = classSym.isEffectivelyFinal
260260

261261
val sam = {
262-
if (classSym.isImplClass || classSym.isEffectivelyFinal) None
262+
if (classSym.isEffectivelyFinal) None
263263
else {
264264
// Phase travel necessary. For example, nullary methods (getter of an abstract val) get an
265265
// empty parameter list in later phases and would therefore be picked as SAM.
@@ -284,41 +284,15 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
284284
val name = methodSym.javaSimpleName.toString // same as in genDefDef
285285
val signature = name + methodSymToDescriptor(methodSym)
286286

287-
// Some detours are required here because of changing flags (lateDEFERRED, lateMODULE):
287+
// Some detours are required here because of changing flags (lateDEFERRED):
288288
// 1. Why the phase travel? Concrete trait methods obtain the lateDEFERRED flag in Mixin.
289289
// This makes isEffectivelyFinalOrNotOverridden false, which would prevent non-final
290290
// but non-overridden methods of sealed traits from being inlined.
291-
// 2. Why the special case for `classSym.isImplClass`? Impl class symbols obtain the
292-
// lateMODULE flag during Mixin. During the phase travel to exitingPickler, the late
293-
// flag is ignored. The members are therefore not isEffectivelyFinal (their owner
294-
// is not a module). Since we know that all impl class members are static, we can
295-
// just take the shortcut.
296-
val effectivelyFinal = classSym.isImplClass || exitingPickler(methodSym.isEffectivelyFinalOrNotOverridden)
297-
298-
// Identify trait interface methods that have a static implementation in the implementation
299-
// class. Invocations of these methods can be re-wrired directly to the static implementation
300-
// if they are final or the receiver is known.
301-
//
302-
// Using `erasure.needsImplMethod` is not enough: it keeps field accessors, module getters
303-
// and super accessors. When AddInterfaces creates the impl class, these methods are
304-
// initially added to it.
305-
//
306-
// The mixin phase later on filters out most of these members from the impl class (see
307-
// Mixin.isImplementedStatically). However, accessors for concrete lazy vals remain in the
308-
// impl class after mixin. So the filter in mixin is not exactly what we need here (we
309-
// want to identify concrete trait methods, not any accessors). So we check some symbol
310-
// properties manually.
311-
val traitMethodWithStaticImplementation = {
312-
import symtab.Flags._
313-
classSym.isTrait && !classSym.isImplClass &&
314-
erasure.needsImplMethod(methodSym) &&
315-
!methodSym.isModule &&
316-
!(methodSym hasFlag (ACCESSOR | SUPERACCESSOR))
317-
}
291+
val effectivelyFinal = exitingPickler(methodSym.isEffectivelyFinalOrNotOverridden) && !(methodSym.owner.isTrait && methodSym.isModule)
318292

319293
val info = MethodInlineInfo(
320294
effectivelyFinal = effectivelyFinal,
321-
traitMethodWithStaticImplementation = traitMethodWithStaticImplementation,
295+
traitMethodWithStaticImplementation = false,
322296
annotatedInline = methodSym.hasAnnotation(ScalaInlineClass),
323297
annotatedNoInline = methodSym.hasAnnotation(ScalaNoInlineClass)
324298
)
@@ -866,7 +840,6 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
866840
|| sym.isArtifact
867841
|| sym.isLiftedMethod
868842
|| sym.isBridge
869-
|| (sym.ownerChain exists (_.isImplClass))
870843
)
871844

872845
/* @return

‎src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
174174
if (lmoc != NoSymbol) {
175175
// it must be a top level class (name contains no $s)
176176
val isCandidateForForwarders = {
177-
exitingPickler { !(lmoc.name.toString contains '$') && lmoc.hasModuleFlag && !lmoc.isImplClass && !lmoc.isNestedClass }
177+
exitingPickler { !(lmoc.name.toString contains '$') && lmoc.hasModuleFlag && !lmoc.isNestedClass }
178178
}
179179
if (isCandidateForForwarders) {
180180
log(s"Adding static forwarders from '$claszSymbol' to implementations in '$lmoc'")
@@ -563,7 +563,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
563563
}
564564

565565
val isNative = methSymbol.hasAnnotation(definitions.NativeAttr)
566-
val isAbstractMethod = (methSymbol.isDeferred || methSymbol.owner.isInterface) && !methSymbol.hasFlag(Flags.JAVA_DEFAULTMETHOD)
566+
val isAbstractMethod = rhs == EmptyTree
567567
val flags = GenBCode.mkFlags(
568568
javaFlags(methSymbol),
569569
if (isAbstractMethod) asm.Opcodes.ACC_ABSTRACT else 0,

‎src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,6 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
172172
*/
173173
def primitiveOrClassToBType(sym: Symbol): BType = {
174174
assertClassNotArray(sym)
175-
assert(!sym.isImplClass, sym)
176175
primitiveTypeToBType.getOrElse(sym, classBTypeFromSymbol(sym))
177176
}
178177

@@ -337,7 +336,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
337336
// Check for hasAnnotationFlag for SI-9393: the classfile / java source parsers add
338337
// scala.annotation.Annotation as superclass to java annotations. In reality, java
339338
// annotation classfiles have superclass Object (like any interface classfile).
340-
val superClassSym = if (classSym.isImplClass || classSym.hasJavaAnnotationFlag) ObjectClass else {
339+
val superClassSym = if (classSym.hasJavaAnnotationFlag) ObjectClass else {
341340
val sc = classSym.superClass
342341
// SI-9393: Java annotation classes don't have the ABSTRACT/INTERFACE flag, so they appear
343342
// (wrongly) as superclasses. Fix this for BTypes: the java annotation will appear as interface
@@ -603,11 +602,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
603602
*/
604603
final def isTopLevelModuleClass(sym: Symbol): Boolean = exitingPickler {
605604
// phase travel to pickler required for isNestedClass (looks at owner)
606-
val r = sym.isModuleClass && !sym.isNestedClass
607-
// The mixin phase adds the `lateMODULE` flag to trait implementation classes. Since the flag
608-
// is late, it should not be visible here inside the time travel. We check this.
609-
if (r) assert(!sym.isImplClass, s"isModuleClass should be false for impl class $sym")
610-
r
605+
sym.isModuleClass && !sym.isNestedClass
611606
}
612607

613608
/**
@@ -684,7 +679,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
684679

685680
val finalFlag = (
686681
(((sym.rawflags & symtab.Flags.FINAL) != 0) || isTopLevelModuleClass(sym))
687-
&& !sym.enclClass.isInterface
682+
&& !sym.enclClass.isTrait
688683
&& !sym.isClassConstructor
689684
&& !sym.isMutable // lazy vals and vars both
690685
)
@@ -697,12 +692,12 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
697692
GenBCode.mkFlags(
698693
if (privateFlag) ACC_PRIVATE else ACC_PUBLIC,
699694
if ((sym.isDeferred && !sym.hasFlag(symtab.Flags.JAVA_DEFAULTMETHOD))|| sym.hasAbstractFlag) ACC_ABSTRACT else 0,
700-
if (sym.isInterface) ACC_INTERFACE else 0,
695+
if (sym.isTraitOrInterface) ACC_INTERFACE else 0,
701696
if (finalFlag && !sym.hasAbstractFlag) ACC_FINAL else 0,
702697
if (sym.isStaticMember) ACC_STATIC else 0,
703698
if (sym.isBridge) ACC_BRIDGE | ACC_SYNTHETIC else 0,
704699
if (sym.isArtifact) ACC_SYNTHETIC else 0,
705-
if (sym.isClass && !sym.isInterface) ACC_SUPER else 0,
700+
if (sym.isClass && !sym.isTraitOrInterface) ACC_SUPER else 0,
706701
if (sym.hasJavaEnumFlag) ACC_ENUM else 0,
707702
if (sym.isVarargsMethod) ACC_VARARGS else 0,
708703
if (sym.hasFlag(symtab.Flags.SYNCHRONIZED)) ACC_SYNCHRONIZED else 0,

‎src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
137137
callee = method,
138138
calleeDeclarationClass = declarationClassBType,
139139
safeToInline = safeToInline,
140-
safeToRewrite = safeToRewrite,
140+
safeToRewrite = false,
141141
canInlineFromSource = canInlineFromSource,
142142
annotatedInline = annotatedInline,
143143
annotatedNoInline = annotatedNoInline,
@@ -299,7 +299,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
299299
receiverType.info.orThrow.inlineInfo.isEffectivelyFinal // (1)
300300
}
301301

302-
val isRewritableTraitCall = isStaticallyResolved && methodInlineInfo.traitMethodWithStaticImplementation
302+
val isRewritableTraitCall = false
303303

304304
val warning = calleeDeclarationClassBType.info.orThrow.inlineInfo.warning.map(
305305
MethodInlineInfoIncomplete(calleeDeclarationClassBType.internalName, calleeMethodNode.name, calleeMethodNode.desc, _))

‎src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class Inliner[BT <: BTypes](val btypes: BT) {
2727
import backendUtils._
2828

2929
def runInliner(): Unit = {
30-
rewriteFinalTraitMethodInvocations()
30+
// rewriteFinalTraitMethodInvocations()
3131

3232
for (request <- collectAndOrderInlineRequests) {
3333
val Right(callee) = request.callsite.callee // collectAndOrderInlineRequests returns callsites with a known callee

0 commit comments

Comments
 (0)