From b36658d257115329cfe25b794685bc85ea1cfc22 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 4 Apr 2016 11:51:38 +0200 Subject: Remove dead code in the optimizer related to trait impl classes --- .../nsc/backend/jvm/opt/ScalaInlineInfoTest.scala | 41 +++++++++++----------- 1 file changed, 20 insertions(+), 21 deletions(-) (limited to 'test') diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala index 10ab006017..f449b8bb45 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala @@ -66,32 +66,31 @@ class ScalaInlineInfoTest extends ClearAfterClass { val cs @ List(t, tl, to) = compileClasses(compiler)(code) val info = inlineInfo(t) val expect = InlineInfo ( - None, // self type false, // final class None, // not a sam Map( // TODO SD-86: the module accessor used to be `effectivelyFinal` before nuke-impl-classes - ("O()LT$O$;", MethodInlineInfo(false,false,false,false)), - ("T$$super$toString()Ljava/lang/String;", MethodInlineInfo(false,false,false,false)), - ("T$_setter_$x1_$eq(I)V", MethodInlineInfo(false,false,false,false)), - ("f1()I", MethodInlineInfo(false,false,false,false)), - ("f3()I", MethodInlineInfo(false,false,false,false)), - ("f4()Ljava/lang/String;", MethodInlineInfo(false,false,true, false)), - ("f5()I", MethodInlineInfo(false,false,false,false)), - ("f6()I", MethodInlineInfo(false,false,false,true )), - ("x1()I", MethodInlineInfo(false,false,false,false)), - ("x3()I", MethodInlineInfo(false,false,false,false)), - ("x3_$eq(I)V", MethodInlineInfo(false,false,false,false)), - ("x4()I", MethodInlineInfo(false,false,false,false)), - ("x5()I", MethodInlineInfo(true, false,false,false)), - ("y2()I", MethodInlineInfo(false,false,false,false)), - ("y2_$eq(I)V", MethodInlineInfo(false,false,false,false)), - ("f2()I", MethodInlineInfo(true, false,false,false)), - ("L$lzycompute$1(Lscala/runtime/VolatileObjectRef;)LT$L$2$;",MethodInlineInfo(true, false,false,false)), + ("O()LT$O$;", MethodInlineInfo(false,false,false)), + ("T$$super$toString()Ljava/lang/String;", MethodInlineInfo(false,false,false)), + ("T$_setter_$x1_$eq(I)V", MethodInlineInfo(false,false,false)), + ("f1()I", MethodInlineInfo(false,false,false)), + ("f3()I", MethodInlineInfo(false,false,false)), + ("f4()Ljava/lang/String;", MethodInlineInfo(false,true, false)), + ("f5()I", MethodInlineInfo(false,false,false)), + ("f6()I", MethodInlineInfo(false,false,true )), + ("x1()I", MethodInlineInfo(false,false,false)), + ("x3()I", MethodInlineInfo(false,false,false)), + ("x3_$eq(I)V", MethodInlineInfo(false,false,false)), + ("x4()I", MethodInlineInfo(false,false,false)), + ("x5()I", MethodInlineInfo(true, false,false)), + ("y2()I", MethodInlineInfo(false,false,false)), + ("y2_$eq(I)V", MethodInlineInfo(false,false,false)), + ("f2()I", MethodInlineInfo(true, false,false)), + ("L$lzycompute$1(Lscala/runtime/VolatileObjectRef;)LT$L$2$;",MethodInlineInfo(true, false,false)), // TODO SD-86: should probably be effectivelyFinal - ("L$1(Lscala/runtime/VolatileObjectRef;)LT$L$2$;", MethodInlineInfo(false,false,false,false)), - ("nest$1()I", MethodInlineInfo(true, false,false,false)), - ("$init$()V", MethodInlineInfo(false,false,false,false))), + ("L$1(Lscala/runtime/VolatileObjectRef;)LT$L$2$;", MethodInlineInfo(false,false,false)), + ("nest$1()I", MethodInlineInfo(true, false,false)), + ("$init$()V", MethodInlineInfo(false,false,false))), None // warning ) assert(info == expect, info) -- cgit v1.2.3 From 2d62eea9f99411adda6725364690d126dcb12d98 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 4 Apr 2016 13:52:35 +0200 Subject: Remove unused optimizer warnings related to trait impl classes --- .../tools/nsc/backend/jvm/BackendReporting.scala | 6 ------ .../nsc/backend/jvm/opt/InlineWarningTest.scala | 24 ---------------------- 2 files changed, 30 deletions(-) (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala b/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala index 13f4738d3d..01206aa6eb 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala @@ -170,9 +170,6 @@ object BackendReporting { case MethodInlineInfoError(_, _, _, cause) => s"Error while computing the inline information for method $warningMessageSignature:\n" + cause - - case RewriteTraitCallToStaticImplMethodFailed(_, _, _, cause) => - cause.toString } def emitWarning(settings: ScalaSettings): Boolean = this match { @@ -182,15 +179,12 @@ object BackendReporting { case MethodInlineInfoMissing(_, _, _, None) => settings.YoptWarningNoInlineMissingBytecode case MethodInlineInfoError(_, _, _, cause) => cause.emitWarning(settings) - - case RewriteTraitCallToStaticImplMethodFailed(_, _, _, cause) => cause.emitWarning(settings) } } case class MethodInlineInfoIncomplete(declarationClass: InternalName, name: String, descriptor: String, cause: ClassInlineInfoWarning) extends CalleeInfoWarning case class MethodInlineInfoMissing(declarationClass: InternalName, name: String, descriptor: String, cause: Option[ClassInlineInfoWarning]) extends CalleeInfoWarning case class MethodInlineInfoError(declarationClass: InternalName, name: String, descriptor: String, cause: NoClassBTypeInfo) extends CalleeInfoWarning - case class RewriteTraitCallToStaticImplMethodFailed(declarationClass: InternalName, name: String, descriptor: String, cause: OptimizerWarning) extends CalleeInfoWarning sealed trait CannotInlineWarning extends OptimizerWarning { def calleeDeclarationClass: InternalName diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala index 01d97855c8..90236265e6 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala @@ -70,30 +70,6 @@ class InlineWarningTest extends ClearAfterClass { assert(count == 4, count) } - // TODO SD-86: no more impl classes. this test (and the warning it tests!) can be removed - @org.junit.Ignore @Test - def traitMissingImplClass(): Unit = { - val codeA = "trait T { @inline final def f = 1 }" - val codeB = "class C { def t1(t: T) = t.f }" - - val removeImpl = (outDir: AbstractFile) => { - val f = outDir.lookupName("T$class.class", directory = false) - if (f != null) f.delete() - } - - val warn = - """T::f()I is annotated @inline but cannot be inlined: the trait method call could not be rewritten to the static implementation method. Possible reason: - |The method f(LT;)I could not be found in the class T$class or any of its parents. - |Note that the following parent classes could not be found on the classpath: T$class""".stripMargin - - var c = 0 - compileSeparately(List(codeA, codeB), extraArgs = InlineWarningTest.args, afterEach = removeImpl, allowMessage = i => {c += 1; i.msg contains warn}) - assert(c == 1, c) - - // only summary here - compileSeparately(List(codeA, codeB), extraArgs = InlineWarningTest.argsNoWarn, afterEach = removeImpl, allowMessage = _.msg contains "there was one inliner warning") - } - @Test def handlerNonEmptyStack(): Unit = { val code = -- cgit v1.2.3 From 2ed0d2ad63afc4ed9b1a95d85c0b15898ce66e2f Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 4 Apr 2016 13:43:24 +0200 Subject: Remove references to trait impl classes, mostly in doc comments --- .../scala/tools/nsc/backend/jvm/BCodeHelpers.scala | 28 +++++------------ .../tools/nsc/backend/jvm/BCodeSkelBuilder.scala | 7 ----- .../scala/tools/nsc/backend/jvm/BTypes.scala | 21 ++++--------- .../tools/nsc/backend/jvm/BTypesFromSymbols.scala | 36 +++++----------------- src/reflect/scala/reflect/api/Symbols.scala | 6 +--- src/reflect/scala/reflect/internal/Flags.scala | 2 +- src/reflect/scala/reflect/internal/Symbols.scala | 6 +--- src/reflect/scala/reflect/internal/Types.scala | 1 - src/repl/scala/tools/nsc/interpreter/Power.scala | 2 -- test/files/jvm/innerClassAttribute/Classes_1.scala | 19 +++++++++++- test/files/jvm/innerClassAttribute/Test.scala | 6 ++++ 11 files changed, 49 insertions(+), 85 deletions(-) (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala index 271146f623..99d4390873 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala @@ -31,8 +31,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { * True for classes generated by the Scala compiler that are considered top-level in terms of * the InnerClass / EnclosingMethod classfile attributes. See comment in BTypes. */ - def considerAsTopLevelImplementationArtifact(classSym: Symbol) = - classSym.isSpecialized + def considerAsTopLevelImplementationArtifact(classSym: Symbol) = classSym.isSpecialized /** * Cache the value of delambdafy == "inline" for each run. We need to query this value many @@ -147,10 +146,10 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { assert(classSym.isClass, classSym) def doesNotExist(method: Symbol) = { - // Value classes. Member methods of value classes exist in the generated box class. However, - // nested methods lifted into a value class are moved to the companion object and don't exist - // in the value class itself. We can identify such nested methods: the initial enclosing class - // is a value class, but the current owner is some other class (the module class). + // Value classes. Member methods of value classes exist in the generated box class. However, + // nested methods lifted into a value class are moved to the companion object and don't exist + // in the value class itself. We can identify such nested methods: the initial enclosing class + // is a value class, but the current owner is some other class (the module class). val enclCls = nextEnclosingClass(method) exitingPickler(enclCls.isDerivedValueClass) && method.owner != enclCls } @@ -172,8 +171,6 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { private def enclosingClassForEnclosingMethodAttribute(classSym: Symbol): Symbol = { assert(classSym.isClass, classSym) val r = nextEnclosingClass(nextEnclosing(classSym)) - // this should be an assertion, but we are more cautious for now as it was introduced before the 2.11.6 minor release - if (considerAsTopLevelImplementationArtifact(r)) devWarning(s"enclosing class of $classSym should not be an implementation artifact class: $r") r } @@ -188,20 +185,11 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { * on the implementation of GenASM / GenBCode, so they need to be passed in. */ def enclosingMethodAttribute(classSym: Symbol, classDesc: Symbol => String, methodDesc: Symbol => String): Option[EnclosingMethodEntry] = { - // trait impl classes are always top-level, see comment in BTypes + // specialized classes are always top-level, see comment in BTypes if (isAnonymousOrLocalClass(classSym) && !considerAsTopLevelImplementationArtifact(classSym)) { val enclosingClass = enclosingClassForEnclosingMethodAttribute(classSym) - val methodOpt = enclosingMethodForEnclosingMethodAttribute(classSym) match { - case some @ Some(m) => - if (m.owner != enclosingClass) { - // This should never happen. In case it does, it prevents emitting an invalid - // EnclosingMethod attribute: if the attribute specifies an enclosing method, - // it needs to exist in the specified enclosing class. - devWarning(s"the owner of the enclosing method ${m.locationString} should be the same as the enclosing class $enclosingClass") - None - } else some - case none => none - } + val methodOpt = enclosingMethodForEnclosingMethodAttribute(classSym) + for (m <- methodOpt) assert(m.owner == enclosingClass, s"the owner of the enclosing method ${m.locationString} should be the same as the enclosing class $enclosingClass") Some(EnclosingMethodEntry( classDesc(enclosingClass), methodOpt.map(_.javaSimpleName.toString).orNull, diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala index 20b1a52818..4c41cfc380 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala @@ -232,13 +232,6 @@ abstract class BCodeSkelBuilder extends BCodeHelpers { } def addClassFields() { - /* Non-method term members are fields, except for module members. Module - * members can only happen on .NET (no flatten) for inner traits. There, - * a module symbol is generated (transformInfo in mixin) which is used - * as owner for the members of the implementation class (so that the - * backend emits them as static). - * No code is needed for this module symbol. - */ for (f <- fieldSymbols(claszSymbol)) { val javagensig = getGenericSignature(f, claszSymbol) val flags = javaFieldFlags(f) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala index 192c2ee14e..f6bccca050 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala @@ -790,26 +790,17 @@ abstract class BTypes { * } * * - * Traits Members - * -------------- - * - * Some trait methods don't exist in the generated interface, but only in the implementation class - * (private methods in traits for example). Since EnclosingMethod expresses a source-level property, - * but the source-level enclosing method doesn't exist in the classfile, we the enclosing method - * is null (the enclosing class is still emitted). - * See BCodeAsmCommon.considerAsTopLevelImplementationArtifact - * + * Specialized Classes, Delambdafy:method closure classes + * ------------------------------------------------------ * - * Implementation Classes, Specialized Classes, Delambdafy:method closure classes - * ------------------------------------------------------------------------------ - * - * Trait implementation classes and specialized classes are always considered top-level. Again, - * the InnerClass / EnclosingMethod attributes describe a source-level properties. The impl - * classes are compilation artifacts. + * Specialized classes are always considered top-level, as the InnerClass / EnclosingMethod + * attributes describe a source-level properties. * * The same is true for delambdafy:method closure classes. These classes are generated at * top-level in the delambdafy phase, no special support is required in the backend. * + * See also BCodeHelpers.considerAsTopLevelImplementationArtifact. + * * * Mirror Classes * -------------- diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala index 759b0a615a..d10b6c8dba 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala @@ -96,11 +96,6 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { * scala.Null is mapped to scala.runtime.Null$. This is because there exist no class files * for the Nothing / Null. If used for example as a parameter type, we use the runtime classes * in the classfile method signature. - * - * Note that the referenced class symbol may be an implementation class. For example when - * compiling a mixed-in method that forwards to the static method in the implementation class, - * the class descriptor of the receiver (the implementation class) is obtained by creating the - * ClassBType. */ final def classBTypeFromSymbol(classSym: Symbol): ClassBType = { assert(classSym != NoSymbol, "Cannot create ClassBType from NoSymbol") @@ -159,9 +154,6 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { /** * This method returns the BType for a type reference, for example a parameter type. - * - * If `t` references a class, typeToBType ensures that the class is not an implementation class. - * See also comment on classBTypeFromSymbol, which is invoked for implementation classes. */ final def typeToBType(t: Type): BType = { import definitions.ArrayClass @@ -264,12 +256,12 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { * current phase, for example, after lambdalift, all local classes become member of the enclosing * class. * - * Impl classes are always considered top-level, see comment in BTypes. + * Specialized classes are always considered top-level, see comment in BTypes. */ private def memberClassesForInnerClassTable(classSymbol: Symbol): List[Symbol] = classSymbol.info.decls.collect({ case sym if sym.isClass && !considerAsTopLevelImplementationArtifact(sym) => sym - case sym if sym.isModule && !considerAsTopLevelImplementationArtifact(sym) => // impl classes get the lateMODULE flag in mixin + case sym if sym.isModule && !considerAsTopLevelImplementationArtifact(sym) => val r = exitingPickler(sym.moduleClass) assert(r != NoSymbol, sym.fullLocationString) r @@ -317,7 +309,6 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { ) } - // Check for isImplClass: trait implementation classes have NoSymbol as superClass // Check for hasAnnotationFlag for SI-9393: the classfile / java source parsers add // scala.annotation.Annotation as superclass to java annotations. In reality, java // annotation classfiles have superclass Object (like any interface classfile). @@ -380,8 +371,8 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { } val companionModuleMembers = if (considerAsTopLevelImplementationArtifact(classSym)) Nil else { - // If this is a top-level non-impl (*) class, the member classes of the companion object are - // added as members of the class. For example: + // If this is a top-level class, the member classes of the companion object are added as + // members of the class. For example: // class C { } // object C { // class D @@ -392,11 +383,6 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { // (done by buildNestedInfo). See comment in BTypes. // For consistency, the InnerClass entry for D needs to be present in C - to Java it looks // like D is a member of C, not C$. - // - // (*) We exclude impl classes: if the classfile for the impl class exists on the classpath, - // a linkedClass symbol is found for which isTopLevelModule is true, so we end up searching - // members of that weird impl-class-module-class-symbol. that search probably cannot return - // any classes, but it's better to exclude it. val javaCompatMembers = { if (linkedClass != NoSymbol && isTopLevelModuleClass(linkedClass)) // phase travel to exitingPickler: this makes sure that memberClassesForInnerClassTable only sees member @@ -454,7 +440,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { assert(innerClassSym.isClass, s"Cannot build NestedInfo for non-class symbol $innerClassSym") val isTopLevel = innerClassSym.rawowner.isPackageClass - // impl classes are considered top-level, see comment in BTypes + // specialized classes are considered top-level, see comment in BTypes if (isTopLevel || considerAsTopLevelImplementationArtifact(innerClassSym)) None else if (innerClassSym.rawowner.isTerm) { // This case should never be reached: the lambdalift phase mutates the rawowner field of all @@ -592,17 +578,11 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { /** * True for module classes of modules that are top-level or owned only by objects. Module classes - * for such objects will get a MODULE$ flag and a corresponding static initializer. + * for such objects will get a MODULE$ field and a corresponding static initializer. */ final def isStaticModuleClass(sym: Symbol): Boolean = { - /* (1) Phase travel to to pickler is required to exclude implementation classes; they have the - * lateMODULEs after mixin, so isModuleClass would be true. - * (2) isStaticModuleClass is a source-level property. See comment on isOriginallyStaticOwner. - */ - exitingPickler { // (1) - sym.isModuleClass && - isOriginallyStaticOwner(sym.originalOwner) // (2) - } + sym.isModuleClass && + isOriginallyStaticOwner(sym.originalOwner) // isStaticModuleClass is a source-level property, see comment on isOriginallyStaticOwner } // legacy, to be removed when the @remote annotation gets removed diff --git a/src/reflect/scala/reflect/api/Symbols.scala b/src/reflect/scala/reflect/api/Symbols.scala index 9e9fe5d67b..b9fb323a4c 100644 --- a/src/reflect/scala/reflect/api/Symbols.scala +++ b/src/reflect/scala/reflect/api/Symbols.scala @@ -260,9 +260,6 @@ trait Symbols { self: Universe => * with an object definition (module class in scala compiler parlance)? * If yes, `isType` is also guaranteed to be true. * - * Note to compiler developers: During the "mixin" phase, trait implementation class symbols - * receive the `lateMODULE` flag, hence `isImplClass && isModuleClass` becomes true. - * * @group Tests */ def isModuleClass: Boolean = false @@ -354,8 +351,7 @@ trait Symbols { self: Universe => /******************* tests *******************/ /** Does this symbol represent a synthetic (i.e. a compiler-generated) entity? - * Examples of synthetic entities are accessors for vals and vars - * or mixin constructors in trait implementation classes. + * Examples of synthetic entities are accessors for vals and vars. * * @group Tests */ diff --git a/src/reflect/scala/reflect/internal/Flags.scala b/src/reflect/scala/reflect/internal/Flags.scala index 35c927a5c3..f058acb7c0 100644 --- a/src/reflect/scala/reflect/internal/Flags.scala +++ b/src/reflect/scala/reflect/internal/Flags.scala @@ -50,7 +50,7 @@ package internal // 34: LIFTED // 35: EXISTENTIAL MIXEDIN // 36: EXPANDEDNAME -// 37: IMPLCLASS PRESUPER/M +// 37: PRESUPER/M // 38: TRANS_FLAG // 39: LOCKED // 40: SPECIALIZED diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index ee763df849..ed51414382 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -956,10 +956,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => * method `owner` returns the class C. * * Why not make a stable version of `isStatic`? Maybe some parts of the compiler depend on the - * current implementation. For example - * trait T { def foo = 1 } - * The method `foo` in the implementation class T$impl will be `isStatic`, because trait - * impl classes get the `lateMODULE` flag (T$impl.isStaticOwner is true). + * current implementation. */ def isStatic = (this hasFlag STATIC) || owner.isStaticOwner @@ -2780,7 +2777,6 @@ trait Symbols extends api.Symbols { self: SymbolTable => case DEFAULTPARAM => "" // TRAIT case MIXEDIN => "" // EXISTENTIAL case LABEL => "