Profiler suggested it would be worthwhile to sh...

Profiler suggested it would be worthwhile to short-circuit
allOverriddenSymbols.isEmpty, and one thing led to another. I don't
know how much I accomplished performancewise, but the cosmetology is
outstanding. Review by moors.

5 files changed, 96 insertions, 94 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Constructors.scala b/src/compiler/scala/tools/nsc/transform/Constructors.scala
index d84e754cc1..a333c7458d 100644
--- a/src/compiler/scala/tools/nsc/transform/Constructors.scala
+++ b/src/compiler/scala/tools/nsc/transform/Constructors.scala
@@ -215,7 +215,7 @@ abstract class Constructors extends Transform with ast.TreeDSL {
       //   the symbol is an outer accessor of a final class which does not override another outer accessor. )
       def maybeOmittable(sym: Symbol) = sym.owner == clazz && (
         sym.isParamAccessor && sym.isPrivateLocal ||
-        sym.isOuterAccessor && sym.owner.isFinal && sym.allOverriddenSymbols.isEmpty &&
+        sym.isOuterAccessor && sym.owner.isFinal && !sym.isOverridingSymbol &&
         !(clazz isSubClass DelayedInitClass)
       )
 
diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala
index 5452dc70a2..21bf696077 100644
--- a/src/compiler/scala/tools/nsc/transform/Erasure.scala
+++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala
@@ -1102,7 +1102,7 @@ abstract class Erasure extends AddInterfaces
           }
           else {
             def doDynamic(fn: Tree, qual: Tree): Tree = {
-              if (fn.symbol.owner.isRefinementClass && fn.symbol.allOverriddenSymbols.isEmpty)
+              if (fn.symbol.owner.isRefinementClass && !fn.symbol.isOverridingSymbol)
                 ApplyDynamic(qual, args) setSymbol fn.symbol setPos tree.pos
               else tree
             }
@@ -1118,9 +1118,9 @@ abstract class Erasure extends AddInterfaces
           val owner = tree.symbol.owner
           // println("preXform: "+ (tree, tree.symbol, tree.symbol.owner, tree.symbol.owner.isRefinementClass))
           if (owner.isRefinementClass) {
-            val overridden = tree.symbol.allOverriddenSymbols
-            assert(!overridden.isEmpty, tree.symbol)
-            tree.symbol = overridden.head
+            val overridden = tree.symbol.nextOverriddenSymbol
+            assert(overridden != NoSymbol, tree.symbol)
+            tree.symbol = overridden
           }
           def isAccessible(sym: Symbol) = localTyper.context.isAccessible(sym, sym.owner.thisType)
           if (!isAccessible(owner) && qual.tpe != null) {
diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala
index c18bfb7929..0d7f1eb5cf 100644
--- a/src/compiler/scala/tools/nsc/transform/Mixin.scala
+++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala
@@ -1063,12 +1063,8 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
                 if (sym.isSetter) {
                   val isOverriddenSetter =
                     nme.isTraitSetterName(sym.name) && {
-                      sym.allOverriddenSymbols match {
-                        case other :: _ =>
-                          isOverriddenAccessor(other.getter(other.owner), clazz.info.baseClasses)
-                        case _ =>
-                          false
-                      }
+                      val other = sym.nextOverriddenSymbol
+                      (other != NoSymbol) && isOverriddenAccessor(other.getter(other.owner), clazz.info.baseClasses)
                     }
                   if (isOverriddenSetter) UNIT
                   else accessedRef match {
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
index a434af1175..d7aa7bc527 100644
--- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -217,10 +217,7 @@ abstract class RefChecks extends InfoTransform {
       val self = clazz.thisType
 
       def isAbstractTypeWithoutFBound(sym: Symbol) = // (part of DEVIRTUALIZE)
-        sym.isAbstractType && !isFBounded(sym)
-
-      def isFBounded(tsym: Symbol) =
-        tsym.info.baseTypeSeq exists (_ contains tsym)
+        sym.isAbstractType && !sym.isFBounded
 
       def infoString(sym: Symbol) = infoString0(sym, sym.owner != clazz)
       def infoStringWithLocation(sym: Symbol) = infoString0(sym, true)
@@ -443,7 +440,7 @@ abstract class RefChecks extends InfoTransform {
       printMixinOverrideErrors()
 
       // Verifying a concrete class has nothing unimplemented.
-      if (clazz.isClass && !clazz.isTrait && !(clazz hasFlag ABSTRACT) && !typesOnly) {
+      if (clazz.isConcreteClass && !typesOnly) {
         val abstractErrors = new ListBuffer[String]
         def abstractErrorMessage =
           // a little formatting polish
@@ -470,11 +467,14 @@ abstract class RefChecks extends InfoTransform {
               atPhase(currentRun.erasurePhase.next)(tp1 matches tp2)
             })
 
-        def ignoreDeferred(member: Symbol) =
-          isAbstractTypeWithoutFBound(member) ||
-          (member.isJavaDefined &&
-           (currentRun.erasurePhase == NoPhase || // the test requires atPhase(erasurePhase.next) so shouldn't be done if the compiler has no erasure phase available
-            javaErasedOverridingSym(member) != NoSymbol))
+        def ignoreDeferred(member: Symbol) = (
+          (member.isAbstractType && !member.isFBounded) || (
+            member.isJavaDefined &&
+            // the test requires atPhase(erasurePhase.next) so shouldn't be
+            // done if the compiler has no erasure phase available
+            (currentRun.erasurePhase == NoPhase || javaErasedOverridingSym(member) != NoSymbol)
+          )
+        )
 
         // 2. Check that only abstract classes have deferred members
         def checkNoAbstractMembers() = {
@@ -566,7 +566,7 @@ abstract class RefChecks extends InfoTransform {
         //
         // (3) is violated but not (2).
         def checkNoAbstractDecls(bc: Symbol) {
-          for (decl <- bc.info.decls.iterator) {
+          for (decl <- bc.info.decls) {
             if (decl.isDeferred && !ignoreDeferred(decl)) {
               val impl = decl.matchingSymbol(clazz.thisType, admit = VBRIDGE)
               if (impl == NoSymbol || (decl.owner isSubClass impl.owner)) {
@@ -746,8 +746,7 @@ abstract class RefChecks extends InfoTransform {
               // However, if `sym` does override a type in a base class
               // we have to assume NoVariance, as there might then be
               // references to the type parameter that are not variance checked.
-              state = if (sym.allOverriddenSymbols.isEmpty) AnyVariance
-                      else NoVariance
+              state = if (sym.isOverridingSymbol) NoVariance else AnyVariance
             }
             sym = sym.owner
           }
@@ -1070,10 +1069,12 @@ abstract class RefChecks extends InfoTransform {
         }
       )
       transformTrees(cdef :: {
-        if (sym.isStatic)
-          if (sym.allOverriddenSymbols.isEmpty) Nil
-          else List(createStaticModuleAccessor())
-        else createInnerModuleAccessor(findOrCreateModuleVar)
+        if (!sym.isStatic)
+          createInnerModuleAccessor(findOrCreateModuleVar)
+        else if (sym.isOverridingSymbol)
+          List(createStaticModuleAccessor())
+        else
+          Nil
       })
     }
 
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index 84c9595e3c..a470e80449 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -1637,31 +1637,35 @@ trait Typers extends Modes {
       }
     }
 
-    /** Check if a method is defined in such a way that it can be called.
-      * A method cannot be called if it is a non-private member of a structural type
-      * and if its parameter's types are not one of
-      * - this.type
-      * - a type member of the structural type
-      * - an abstract type declared outside of the structural type. */
-    def checkMethodStructuralCompatible(meth: Symbol): Unit =
-      if (meth.owner.isStructuralRefinement && meth.allOverriddenSymbols.isEmpty && !(meth.isPrivate || meth.hasAccessBoundary)) {
-        val tp: Type = meth.tpe match {
-          case mt: MethodType => mt
-          case NullaryMethodType(res) => res
- // TODO_NMT: drop NullaryMethodType from resultType?
-          case pt: PolyType => pt.resultType
-          case _ => NoType
-        }
-        for (paramType <- tp.paramTypes)  {
-          if (paramType.typeSymbol.isAbstractType && !(paramType.typeSymbol.hasTransOwner(meth.owner)))
-            unit.error(meth.pos,"Parameter type in structural refinement may not refer to an abstract type defined outside that refinement")
-          else if (paramType.typeSymbol.isAbstractType && !(paramType.typeSymbol.hasTransOwner(meth)))
-            unit.error(meth.pos,"Parameter type in structural refinement may not refer to a type member of that refinement")
-          else if (paramType.isInstanceOf[ThisType] && paramType.typeSymbol == meth.owner)
-            unit.error(meth.pos,"Parameter type in structural refinement may not refer to the type of that refinement (self type)")
-        }
+    /** Check if a structurally defined method violates implementation restrictions.
+     *  A method cannot be called if it is a non-private member of a refinement type
+     *  and if its parameter's types are any of:
+     *    - this.type
+     *    - a type member of the refinement
+     *    - an abstract type declared outside of the refinement.
+     */
+    def checkMethodStructuralCompatible(meth: Symbol): Unit = {
+      def fail(msg: String) = unit.error(meth.pos, msg)
+      val tp: Type = meth.tpe match {
+        case mt @ MethodType(_, _)     => mt
+        case NullaryMethodType(restpe) => restpe  // TODO_NMT: drop NullaryMethodType from resultType?
+        case PolyType(_, restpe)       => restpe
+        case _                         => NoType
       }
 
+      for (paramType <- tp.paramTypes) {
+        val sym = paramType.typeSymbol
+
+        if (sym.isAbstractType) {
+          if (!sym.hasTransOwner(meth.owner))
+            fail("Parameter type in structural refinement may not refer to an abstract type defined outside that refinement")
+          else if (!sym.hasTransOwner(meth))
+            fail("Parameter type in structural refinement may not refer to a type member of that refinement")
+        }
+        if (paramType.isInstanceOf[ThisType] && sym == meth.owner)
+          fail("Parameter type in structural refinement may not refer to the type of that refinement (self type)")
+      }
+    }
     def typedUseCase(useCase: UseCase) {
       def stringParser(str: String): syntaxAnalyzer.Parser = {
         val file = new BatchSourceFile(context.unit.source.file, str) {
@@ -1774,7 +1778,8 @@ trait Typers extends Modes {
         }
       }
 
-      checkMethodStructuralCompatible(meth)
+      if (meth.isRefinementMember)
+        checkMethodStructuralCompatible(meth)
 
       treeCopy.DefDef(ddef, typedMods, ddef.name, tparams1, vparamss1, tpt1, rhs1) setType NoType
     }
@@ -1862,51 +1867,51 @@ trait Typers extends Modes {
         for (stat <- block.stats) enterLabelDef(stat)
 
         if (phaseId(currentPeriod) <= currentRun.typerPhase.id) {
-          // This is very tricky stuff, because we are navigating
-          // the Skylla and Charybdis of anonymous classes and what to return
-          // from them here. On the one hand, we cannot admit
-          // every non-private member of an anonymous class as a part of
-          // the structural type of the enclosing block. This runs afoul of
-          // the restriction that a structural type may not refer to an enclosing
-          // type parameter or abstract types (which in turn is necessitated
-          // by what can be done in Java reflection. On the other hand,
-          // making every term member private conflicts with private escape checking
-          // see ticket #3174 for an example.
-          // The cleanest way forward is if we would find a way to suppress
-          // structural type checking for these members and maybe defer
-          // type errors to the places where members are called. But that would
-          // be a big refactoring and also a  big departure from existing code.
-          // The probably safest fix for 2.8 is to keep members of an anonymous
-          // class that are not mentioned in a parent type private (as before)
-          // but to disable escape checking for code that's in the same anonymous class.
-          // That's what's done here.
-          // We really should go back and think hard whether we find a better
-          // way to address the problem of escaping idents on the one hand and well-formed
-          // structural types on the other.
+          // This is very tricky stuff, because we are navigating the Skylla and Charybdis of
+          // anonymous classes and what to return from them here. On the one hand, we cannot admit
+          // every non-private member of an anonymous class as a part of the structural type of the
+          // enclosing block. This runs afoul of the restriction that a structural type may not
+          // refer to an enclosing type parameter or abstract types (which in turn is necessitated
+          // by what can be done in Java reflection). On the other hand, making every term member
+          // private conflicts with private escape checking - see ticket #3174 for an example.
+          //
+          // The cleanest way forward is if we would find a way to suppress structural type checking
+          // for these members and maybe defer type errors to the places where members are called.
+          // But that would be a big refactoring and also a big departure from existing code. The
+          // probably safest fix for 2.8 is to keep members of an anonymous class that are not
+          // mentioned in a parent type private (as before) but to disable escape checking for code
+          // that's in the same anonymous class. That's what's done here.
+          //
+          // We really should go back and think hard whether we find a better way to address the
+          // problem of escaping idents on the one hand and well-formed structural types on the
+          // other.
           block match {
-            case block @ Block(List(classDef @ ClassDef(_, _, _, _)), newInst @ Apply(Select(New(_), _), _)) =>
-              // The block is an anonymous class definitions/instantiation pair
-              //   -> members that are hidden by the type of the block are made private
+            case Block(List(classDef @ ClassDef(_, _, _, _)), Apply(Select(New(_), _), _)) =>
+              val classDecls = classDef.symbol.info.decls
               val visibleMembers = pt match {
-                case WildcardType => classDef.symbol.info.decls.toList
-                case BoundedWildcardType(TypeBounds(lo, hi)) => lo.members
-                case _ => pt.members
+                case WildcardType                           => classDecls.toList
+                case BoundedWildcardType(TypeBounds(lo, _)) => lo.members
+                case _                                      => pt.members
               }
-              for (member <- classDef.symbol.info.decls
-                   if member.isTerm && !member.isConstructor &&
-                      member.allOverriddenSymbols.isEmpty &&
-                      (!member.isPrivate && !member.hasAccessBoundary) &&
-                      !(visibleMembers exists { visible =>
-                        visible.name == member.name &&
-                        member.tpe <:< visible.tpe.substThis(visible.owner, ThisType(classDef.symbol))
-                      })
-              ) {
-                member.resetFlag(PROTECTED)
-                member.resetFlag(LOCAL)
-                member.setFlag(PRIVATE | SYNTHETIC_PRIVATE)
-                syntheticPrivates += member
-                member.privateWithin = NoSymbol
+              def matchesVisibleMember(member: Symbol) = visibleMembers exists { vis =>
+                (member.name == vis.name) &&
+                (member.tpe <:< vis.tpe.substThis(vis.owner, ThisType(classDef.symbol)))
               }
+              // The block is an anonymous class definitions/instantiation pair
+              //   -> members that are hidden by the type of the block are made private
+              ( classDecls filter (member =>
+                     member.isTerm
+                  && member.isVisibleInRefinement
+                  && !member.hasAccessBoundary
+                  && !matchesVisibleMember(member)
+                )
+                foreach { member =>
+                  member resetFlag (PROTECTED | LOCAL)
+                  member   setFlag (PRIVATE | SYNTHETIC_PRIVATE)
+                  syntheticPrivates += member
+                  member.privateWithin = NoSymbol
+                }
+              )
             case _ =>
           }
         }
@@ -1917,7 +1922,7 @@ trait Typers extends Modes {
       } finally {
         // enable escaping privates checking from the outside and recycle
         // transient flag
-        for (sym <- syntheticPrivates) sym resetFlag SYNTHETIC_PRIVATE
+        syntheticPrivates foreach (_ resetFlag SYNTHETIC_PRIVATE)
       }
     }