Skip to content

Commit b53ffde

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 f11c5fb commit b53ffde

27 files changed

+128
-811
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: 10 additions & 5 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
@@ -1028,7 +1029,7 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
10281029
hostSymbol.info ; methodOwner.info
10291030

10301031
def needsInterfaceCall(sym: Symbol) = (
1031-
sym.isInterface
1032+
sym.isTraitOrInterface
10321033
|| sym.isJavaDefined && sym.isNonBottomSubClass(definitions.ClassfileAnnotationClass)
10331034
)
10341035

@@ -1038,9 +1039,13 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
10381039
!style.isVirtual
10391040
|| hostSymbol.isBottomClass
10401041
|| methodOwner == definitions.ObjectClass
1041-
)
1042+
) && !(style.isSuper && hostSymbol != null)
10421043
val receiver = if (useMethodOwner) methodOwner else hostSymbol
10431044
val jowner = internalName(receiver)
1045+
1046+
if (style.isSuper && receiver.isTraitOrInterface && !cnode.interfaces.contains(jowner))
1047+
cnode.interfaces.add(jowner)
1048+
10441049
val jname = method.javaSimpleName.toString
10451050
val bmType = methodBTypeFromSymbol(method)
10461051
val mdescr = bmType.descriptor
@@ -1322,7 +1327,7 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
13221327
def asmType(sym: Symbol) = classBTypeFromSymbol(sym).toASMType
13231328

13241329
val implMethodHandle =
1325-
new asm.Handle(if (lambdaTarget.hasFlag(Flags.STATIC)) asm.Opcodes.H_INVOKESTATIC else asm.Opcodes.H_INVOKEVIRTUAL,
1330+
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,
13261331
classBTypeFromSymbol(lambdaTarget.owner).internalName,
13271332
lambdaTarget.name.toString,
13281333
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
@@ -251,7 +251,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
251251
* Build the [[InlineInfo]] for a class symbol.
252252
*/
253253
def buildInlineInfoFromClassSymbol(classSym: Symbol, classSymToInternalName: Symbol => InternalName, methodSymToDescriptor: Symbol => String): InlineInfo = {
254-
val traitSelfType = if (classSym.isTrait && !classSym.isImplClass) {
254+
val traitSelfType = if (classSym.isTrait) {
255255
// The mixin phase uses typeOfThis for the self parameter in implementation class methods.
256256
val selfSym = classSym.typeOfThis.typeSymbol
257257
if (selfSym != classSym) Some(classSymToInternalName(selfSym)) else None
@@ -262,7 +262,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
262262
val isEffectivelyFinal = classSym.isEffectivelyFinal
263263

264264
val sam = {
265-
if (classSym.isImplClass || classSym.isEffectivelyFinal) None
265+
if (classSym.isEffectivelyFinal) None
266266
else {
267267
// Phase travel necessary. For example, nullary methods (getter of an abstract val) get an
268268
// empty parameter list in later phases and would therefore be picked as SAM.
@@ -287,41 +287,15 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
287287
val name = methodSym.javaSimpleName.toString // same as in genDefDef
288288
val signature = name + methodSymToDescriptor(methodSym)
289289

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

322296
val info = MethodInlineInfo(
323297
effectivelyFinal = effectivelyFinal,
324-
traitMethodWithStaticImplementation = traitMethodWithStaticImplementation,
298+
traitMethodWithStaticImplementation = false,
325299
annotatedInline = methodSym.hasAnnotation(ScalaInlineClass),
326300
annotatedNoInline = methodSym.hasAnnotation(ScalaNoInlineClass)
327301
)
@@ -858,7 +832,6 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
858832
|| sym.isArtifact
859833
|| sym.isLiftedMethod
860834
|| sym.isBridge
861-
|| (sym.ownerChain exists (_.isImplClass))
862835
)
863836

864837
/* @return

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

Lines changed: 1 addition & 1 deletion
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'")

‎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
primitiveTypeMap.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 || nme.isTraitSetterName(sym.name)) // lazy vals and vars and their setters cannot be final, but trait setters are
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
annotatedInline = annotatedInline,
142142
annotatedNoInline = annotatedNoInline,
143143
samParamTypes = samParamTypes,
@@ -298,7 +298,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
298298
receiverType.info.orThrow.inlineInfo.isEffectivelyFinal // (1)
299299
}
300300

301-
val isRewritableTraitCall = isStaticallyResolved && methodInlineInfo.traitMethodWithStaticImplementation
301+
val isRewritableTraitCall = false
302302

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

0 commit comments

Comments
 (0)