Skip to content

Commit c8e6050

Browse files
retronymretronym
committed
New trait encoding: use default methods, jettison impl classes
Until now, concrete methods in traits were encoded with "trait implementation classes". - Such a trait would compile to two class files - the trait interface, a Java interface, and - the implementation class, containing "trait implementation methods" - trait implementation methods are static methods has an explicit self parameter. - some methods don't require addition of an interface method, such as private methods. Calls to these directly call the implementation method - classes that mixin a trait install "trait forwarders", which implement the abstract method in the interface by forwarding to the trait implementation method. The new encoding: - no longer emits trait implementation classes or trait implementation methods. - instead, concrete methods are simply retained in the interface, as JVM 8 default interface methods (the JVM spec changes in [JSR-335](http://download.oracle.com/otndocs/jcp/lambda-0_9_3-fr-eval-spec/index.html) pave the way) - use `invokespecial` to call private or particular super implementations of a method (rather `invokestatic`) - in cases when we `invokespecial` to a method in an indirect ancestor, we add that ancestor redundantly as a direct parent. We are investigating alternatives approaches here. - we still emit trait fowrarders, although we are [investigating](scala/scala-dev#98) ways to only do this when the JVM would be unable to resolve the correct method using its rules for default method resolution. Here's an example: ``` trait T { println("T") def m1 = m2 private def m2 = "m2" } trait U extends T { println("T") override def m1 = super[T].m1 } class C extends U { println("C") def test = m1 } ``` The old and new encodings are displayed and diffed here: https://gist.github.com/retronym/f174d23f859f0e053580 Some notes in the implementation: - No need to filter members from class decls at all in AddInterfaces (although we do have to trigger side effecting info transformers) - We can now emit an EnclosingMethod attribute for classes nested in private trait methods - Created a factory method for an AST shape that is used in a number of places to symbolically bind to a particular super method without needed to specify the qualifier of the `Super` tree (which is too limiting, as it only allows you to refer to direct parents.) - I also found a similar tree shape created in Delambdafy, that is better expressed with an existing tree creation factory method, mkSuperInit.
1 parent 699a5d9 commit c8e6050

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+294
-893
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: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,22 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
535535
private def genApply(app: Apply, expectedType: BType): BType = {
536536
var generatedType = expectedType
537537
lineNumber(app)
538+
539+
def genSuperApply(hostClass: Symbol, fun: Symbol, args: List[Tree]) = {
540+
// 'super' call: Note: since constructors are supposed to
541+
// return an instance of what they construct, we have to take
542+
// special care. On JVM they are 'void', and Scala forbids (syntactically)
543+
// to call super constructors explicitly and/or use their 'returned' value.
544+
// therefore, we can ignore this fact, and generate code that leaves nothing
545+
// on the stack (contrary to what the type in the AST says).
546+
547+
val invokeStyle = InvokeStyle.Super
548+
mnode.visitVarInsn(asm.Opcodes.ALOAD, 0)
549+
genLoadArguments(args, paramTKs(app))
550+
genCallMethod(fun, invokeStyle, app.pos, hostClass)
551+
generatedType = methodBTypeFromSymbol(fun).returnType
552+
}
553+
538554
app match {
539555

540556
case Apply(TypeApply(fun, targs), _) =>
@@ -582,19 +598,19 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
582598

583599
generatedType = genTypeApply()
584600

585-
// 'super' call: Note: since constructors are supposed to
586-
// return an instance of what they construct, we have to take
587-
// special care. On JVM they are 'void', and Scala forbids (syntactically)
588-
// to call super constructors explicitly and/or use their 'returned' value.
589-
// therefore, we can ignore this fact, and generate code that leaves nothing
590-
// on the stack (contrary to what the type in the AST says).
591-
case Apply(fun @ Select(Super(_, _), _), args) =>
592-
val invokeStyle = InvokeStyle.Super
593-
// if (fun.symbol.isConstructor) Static(true) else SuperCall(mix);
594-
mnode.visitVarInsn(asm.Opcodes.ALOAD, 0)
595-
genLoadArguments(args, paramTKs(app))
596-
genCallMethod(fun.symbol, invokeStyle, app.pos)
597-
generatedType = methodBTypeFromSymbol(fun.symbol).returnType
601+
case Apply(fun @ Select(Super(qual, mix), _), args) =>
602+
val hostClass = qual.symbol.parentSymbols.filter(_.name == mix) match {
603+
case Nil =>
604+
// We get here for trees created by SuperSelect which use tpnme.EMPTY as the super qualifier
605+
// Subsequent code uses the owner of fun.symbol to target the call.
606+
null
607+
case parent :: Nil=>
608+
parent
609+
case parents =>
610+
devWarning("ambiguous parent class qualifier: " + qual.symbol.parentSymbols)
611+
null
612+
}
613+
genSuperApply(hostClass, fun.symbol, args)
598614

599615
// 'new' constructor call: Note: since constructors are
600616
// thought to return an instance of what they construct,
@@ -1050,19 +1066,26 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
10501066
hostSymbol.info ; methodOwner.info
10511067

10521068
def needsInterfaceCall(sym: Symbol) = (
1053-
sym.isInterface
1069+
sym.isTraitOrInterface
10541070
|| sym.isJavaDefined && sym.isNonBottomSubClass(definitions.ClassfileAnnotationClass)
10551071
)
10561072

1073+
val isTraitCallToObjectMethod =
1074+
hostSymbol != methodOwner && methodOwner.isTraitOrInterface && ObjectTpe.decl(method.name) != NoSymbol && method.overrideChain.last.owner == ObjectClass
1075+
10571076
// whether to reference the type of the receiver or
10581077
// the type of the method owner
1059-
val useMethodOwner = (
1078+
val useMethodOwner = ((
10601079
!style.isVirtual
10611080
|| hostSymbol.isBottomClass
10621081
|| methodOwner == definitions.ObjectClass
1063-
)
1082+
) && !(style.isSuper && hostSymbol != null)) || isTraitCallToObjectMethod
10641083
val receiver = if (useMethodOwner) methodOwner else hostSymbol
10651084
val jowner = internalName(receiver)
1085+
1086+
if (style.isSuper && (isTraitCallToObjectMethod || receiver.isTraitOrInterface) && !cnode.interfaces.contains(jowner))
1087+
cnode.interfaces.add(jowner)
1088+
10661089
val jname = method.javaSimpleName.toString
10671090
val bmType = methodBTypeFromSymbol(method)
10681091
val mdescr = bmType.descriptor
@@ -1342,7 +1365,7 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
13421365
def asmType(sym: Symbol) = classBTypeFromSymbol(sym).toASMType
13431366

13441367
val implMethodHandle =
1345-
new asm.Handle(if (lambdaTarget.hasFlag(Flags.STATIC)) asm.Opcodes.H_INVOKESTATIC else asm.Opcodes.H_INVOKEVIRTUAL,
1368+
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,
13461369
classBTypeFromSymbol(lambdaTarget.owner).internalName,
13471370
lambdaTarget.name.toString,
13481371
methodBTypeFromSymbol(lambdaTarget).descriptor)

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

Lines changed: 9 additions & 39 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
@@ -145,15 +145,12 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
145145
assert(classSym.isClass, classSym)
146146

147147
def doesNotExist(method: Symbol) = {
148-
// (1) SI-9124, some trait methods don't exist in the generated interface. see comment in BTypes.
149-
// (2) Value classes. Member methods of value classes exist in the generated box class. However,
148+
// Value classes. Member methods of value classes exist in the generated box class. However,
150149
// nested methods lifted into a value class are moved to the companion object and don't exist
151150
// in the value class itself. We can identify such nested methods: the initial enclosing class
152151
// is a value class, but the current owner is some other class (the module class).
153-
method.owner.isTrait && method.isImplOnly || { // (1)
154-
val enclCls = nextEnclosingClass(method)
155-
exitingPickler(enclCls.isDerivedValueClass) && method.owner != enclCls // (2)
156-
}
152+
val enclCls = nextEnclosingClass(method)
153+
exitingPickler(enclCls.isDerivedValueClass) && method.owner != enclCls
157154
}
158155

159156
def enclosingMethod(sym: Symbol): Option[Symbol] = {
@@ -248,7 +245,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
248245
* Build the [[InlineInfo]] for a class symbol.
249246
*/
250247
def buildInlineInfoFromClassSymbol(classSym: Symbol, classSymToInternalName: Symbol => InternalName, methodSymToDescriptor: Symbol => String): InlineInfo = {
251-
val traitSelfType = if (classSym.isTrait && !classSym.isImplClass) {
248+
val traitSelfType = if (classSym.isTrait) {
252249
// The mixin phase uses typeOfThis for the self parameter in implementation class methods.
253250
val selfSym = classSym.typeOfThis.typeSymbol
254251
if (selfSym != classSym) Some(classSymToInternalName(selfSym)) else None
@@ -259,7 +256,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
259256
val isEffectivelyFinal = classSym.isEffectivelyFinal
260257

261258
val sam = {
262-
if (classSym.isImplClass || classSym.isEffectivelyFinal) None
259+
if (classSym.isEffectivelyFinal) None
263260
else {
264261
// Phase travel necessary. For example, nullary methods (getter of an abstract val) get an
265262
// empty parameter list in later phases and would therefore be picked as SAM.
@@ -284,41 +281,15 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
284281
val name = methodSym.javaSimpleName.toString // same as in genDefDef
285282
val signature = name + methodSymToDescriptor(methodSym)
286283

287-
// Some detours are required here because of changing flags (lateDEFERRED, lateMODULE):
284+
// Some detours are required here because of changing flags (lateDEFERRED):
288285
// 1. Why the phase travel? Concrete trait methods obtain the lateDEFERRED flag in Mixin.
289286
// This makes isEffectivelyFinalOrNotOverridden false, which would prevent non-final
290287
// 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-
}
288+
val effectivelyFinal = exitingPickler(methodSym.isEffectivelyFinalOrNotOverridden) && !(methodSym.owner.isTrait && methodSym.isModule)
318289

319290
val info = MethodInlineInfo(
320291
effectivelyFinal = effectivelyFinal,
321-
traitMethodWithStaticImplementation = traitMethodWithStaticImplementation,
292+
traitMethodWithStaticImplementation = false,
322293
annotatedInline = methodSym.hasAnnotation(ScalaInlineClass),
323294
annotatedNoInline = methodSym.hasAnnotation(ScalaNoInlineClass)
324295
)
@@ -866,7 +837,6 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
866837
|| sym.isArtifact
867838
|| sym.isLiftedMethod
868839
|| sym.isBridge
869-
|| (sym.ownerChain exists (_.isImplClass))
870840
)
871841

872842
/* @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)