SI-7475 findMember and findMembers: estranged no more

Swathes of important logic are duplicated between `findMember`
and `findMembers` after they separated on grounds of irreconcilable
differences about how fast they should run:

    d905558 Variation #10 to optimze findMember
    fcb0c01 Attempt #9 to opimize findMember.
    71d2ceb Attempt #8 to opimize findMember.
    77e5692 Attempty #7 to optimize findMember
    275115e Fixing problem that caused fingerprints to fail in
    e94252e Attemmpt #6 to optimize findMember
    73e61b8 Attempt #5 to optimize findMember.
    04f0b65 Attempt #4 to optimize findMember
    0e3c70f Attempt #3 to optimize findMember
    41f4497 Attempt #2 to optimize findMember
    1a73aa0 Attempt #1 to optimize findMember

This didn't actually bear fruit, and the intervening years have
seen the implementations drift.

Now is the time to reunite them under the banner of `FindMemberBase`.

Each has a separate subclass to customise the behaviour. This is
primarily used by `findMember` to cache member types and to assemble
the resulting list of symbols in an low-allocation manner.

While there I have introduced some polymorphic calls, the call sites
are only bi-morphic, and our typical pattern of compilation involves
far more `findMember` calls, so I expect that JIT will keep the
virtual call cost to an absolute minimum.

Test results have been updated now that `findMembers` correctly
excludes constructors and doesn't inherit privates.

Coming up next: we can actually fix SI-7475!

Author: retronym

src/reflect/scala/reflect/internal/Types.scala

@@ -990,65 +990,7 @@ trait Types
      *
      */
     def findMembers(excludedFlags: Long, requiredFlags: Long): Scope = {
-      // TODO refactor to use `FindMemberBase`
-      def findMembersInternal: Scope = {
-        var members: Scope = null
-        if (Statistics.canEnable) Statistics.incCounter(findMembersCount)
-        val start = if (Statistics.canEnable) Statistics.pushTimer(typeOpsStack, findMembersNanos) else null
-
-        //Console.println("find member " + name.decode + " in " + this + ":" + this.baseClasses)//DEBUG
-        var required = requiredFlags
-        var excluded = excludedFlags | DEFERRED
-        var retryForDeferred = true
-        var self: Type = null
-        while (retryForDeferred) {
-          retryForDeferred = false
-          val bcs0 = baseClasses
-          var bcs = bcs0
-          while (!bcs.isEmpty) {
-            val decls = bcs.head.info.decls
-            var entry = decls.elems
-            while (entry ne null) {
-              val sym = entry.sym
-              val flags = sym.flags
-              if ((flags & required) == required) {
-                val excl = flags & excluded
-                if (excl == 0L &&
-                    (// omit PRIVATE LOCALS unless selector class is contained in class owning the def.
-                     (bcs eq bcs0) ||
-                     (flags & PrivateLocal) != PrivateLocal ||
-                     (bcs0.head.hasTransOwner(bcs.head)))) {
-                  if (members eq null) members = newFindMemberScope
-                  var others: ScopeEntry = members.lookupEntry(sym.name)
-                  var symtpe: Type = null
-                  while ((others ne null) && {
-                           val other = others.sym
-                           (other ne sym) &&
-                           ((other.owner eq sym.owner) ||
-                            (flags & PRIVATE) != 0 || {
-                               if (self eq null) self = narrowForFindMember(this)
-                               if (symtpe eq null) symtpe = self.memberType(sym)
-                               !(self.memberType(other) matches symtpe)
-                            })}) {
-                    others = members lookupNextEntry others
-                  }
-                  if (others eq null) members enter sym
-                } else if (excl == DEFERRED) {
-                  retryForDeferred = (excludedFlags & DEFERRED) == 0
-                }
-              }
-              entry = entry.next
-            } // while (entry ne null)
-            // excluded = excluded | LOCAL
-            bcs = bcs.tail
-          } // while (!bcs.isEmpty)
-          required |= DEFERRED
-          excluded &= ~(DEFERRED.toLong)
-        } // while (retryForDeferred)
-        if (Statistics.canEnable) Statistics.popTimer(typeOpsStack, start)
-        if (members eq null) EmptyScope else members
-      }
-
+      def findMembersInternal = new FindMembers(this, excludedFlags, requiredFlags).apply()
       if (this.isGround) findMembersInternal
       else suspendingTypeVars(typeVarsInType(this))(findMembersInternal)
     }
@@ -1063,115 +1005,7 @@ trait Types
      *  @param stableOnly     If set, return only members that are types or stable values
      */
     def findMember(name: Name, excludedFlags: Long, requiredFlags: Long, stableOnly: Boolean): Symbol = {
-      def findMemberInternal = {
-        // TODO delete `findMemberInternalOld`
-        val resultOld = findMemberInternalOld
-        val resultNew = findMemberInternalNew
-        if (resultOld.alternatives != resultNew.alternatives) {
-          findMemberInternalOld
-          findMemberInternalNew
-        }
-        assert(resultOld.alternatives == resultNew.alternatives, s"\nOLD:${resultOld.alternatives}\nNEW${resultNew.alternatives}")
-        resultNew
-      }
-      def findMemberInternalNew = new FindMember(this, name, excludedFlags, requiredFlags, stableOnly).apply()
-      def findMemberInternalOld: Symbol = {
-        var member: Symbol        = NoSymbol
-        var members: List[Symbol] = null
-        var lastM: ::[Symbol]     = null
-        if (Statistics.canEnable) Statistics.incCounter(findMemberCount)
-        val start = if (Statistics.canEnable) Statistics.pushTimer(typeOpsStack, findMemberNanos) else null
-
-        //Console.println("find member " + name.decode + " in " + this + ":" + this.baseClasses)//DEBUG
-        var membertpe: Type = null
-        var required = requiredFlags
-        var excluded = excludedFlags | DEFERRED
-        var continue = true
-        var self: Type = null
-
-        while (continue) {
-          continue = false
-          val bcs0 = baseClasses
-          var bcs = bcs0
-          // omit PRIVATE LOCALS unless selector class is contained in class owning the def.
-          def admitPrivateLocal(owner: Symbol): Boolean = {
-            val selectorClass = this match {
-              case tt: ThisType => tt.sym // SI-7507 the first base class is not necessarily the selector class.
-              case _            => bcs0.head
-            }
-            selectorClass.hasTransOwner(owner)
-          }
-          while (!bcs.isEmpty) {
-            val decls = bcs.head.info.decls
-            var entry = decls.lookupEntry(name)
-            while (entry ne null) {
-              val sym = entry.sym
-              val flags = sym.flags
-              if ((flags & required) == required) {
-                val excl = flags & excluded
-                if (excl == 0L &&
-                      (
-                    (bcs eq bcs0) ||
-                    (flags & PrivateLocal) != PrivateLocal ||
-                    admitPrivateLocal(bcs.head))) {
-                  if (name.isTypeName || (stableOnly && sym.isStable && !sym.hasVolatileType)) {
-                    if (Statistics.canEnable) Statistics.popTimer(typeOpsStack, start)
-                    return sym
-                  } else if (member eq NoSymbol) {
-                    member = sym
-                  } else if (members eq null) {
-                    if ((member ne sym) &&
-                      ((member.owner eq sym.owner) ||
-                        (flags & PRIVATE) != 0 || {
-                          if (self eq null) self = narrowForFindMember(this)
-                          if (membertpe eq null) membertpe = self.memberType(member)
-                          !(membertpe matches self.memberType(sym))
-                        })) {
-                      lastM = new ::(sym, null)
-                      members = member :: lastM
-                    }
-                  } else {
-                    var others: List[Symbol] = members
-                    var symtpe: Type = null
-                    while ((others ne null) && {
-                      val other = others.head
-                      (other ne sym) &&
-                        ((other.owner eq sym.owner) ||
-                          (flags & PRIVATE) != 0 || {
-                            if (self eq null) self = narrowForFindMember(this)
-                            if (symtpe eq null) symtpe = self.memberType(sym)
-                            !(self.memberType(other) matches symtpe)
-                               })}) {
-                      others = others.tail
-                    }
-                    if (others eq null) {
-                      val lastM1 = new ::(sym, null)
-                      lastM.tl = lastM1
-                      lastM = lastM1
-                    }
-                  }
-                } else if (excl == DEFERRED) {
-                  continue = true
-                }
-              }
-              entry = decls lookupNextEntry entry
-            } // while (entry ne null)
-            // excluded = excluded | LOCAL
-            bcs = if (name == nme.CONSTRUCTOR) Nil else bcs.tail
-          } // while (!bcs.isEmpty)
-          required |= DEFERRED
-          excluded &= ~(DEFERRED.toLong)
-        } // while (continue)
-        if (Statistics.canEnable) Statistics.popTimer(typeOpsStack, start)
-        if (members eq null) {
-          if (member == NoSymbol) if (Statistics.canEnable) Statistics.incCounter(noMemberCount)
-          member
-        } else {
-          if (Statistics.canEnable) Statistics.incCounter(multMemberCount)
-          lastM.tl = Nil
-          baseClasses.head.newOverloaded(this, members)
-        }
-      }
+      def findMemberInternal = new FindMember(this, name, excludedFlags, requiredFlags, stableOnly).apply()
 
       if (this.isGround) findMemberInternal
       else suspendingTypeVars(typeVarsInType(this))(findMemberInternal)

src/reflect/scala/reflect/internal/tpe/FindMembers.scala

@@ -150,6 +150,31 @@ trait FindMembers {
     protected def memberTypeLow(sym: Symbol): Type = self.memberType(sym)
   }
 
+  private[reflect] final class FindMembers(tpe: Type, excludedFlags: Long, requiredFlags: Long)
+    extends FindMemberBase[Scope](tpe, nme.ANYname, excludedFlags, requiredFlags) {
+    private[this] var _membersScope: Scope   = null
+    private def membersScope: Scope = {
+      if (_membersScope eq null) _membersScope = newFindMemberScope
+      _membersScope
+    }
+
+    protected def shortCircuit(sym: Symbol): Boolean = false
+    protected def result: Scope = membersScope
+
+    protected def addMemberIfNew(sym: Symbol): Unit = {
+      val members = membersScope
+      var others = members.lookupEntry(sym.name)
+      var isNew = true
+      while (others ne null) {
+        val member = others.sym
+        if (!isNewMember(member, sym))
+          isNew = false
+        others = members lookupNextEntry others // next existing member with the same name.
+      }
+      if (isNew) members.enter(sym)
+    }
+  }
+
   private[reflect] final class FindMember(tpe: Type, name: Name, excludedFlags: Long, requiredFlags: Long, stableOnly: Boolean)
     extends FindMemberBase[Symbol](tpe, name, excludedFlags, requiredFlags) {
     // Gathering the results into a hand rolled ListBuffer

src/reflect/scala/reflect/runtime/JavaUniverseForce.scala

@@ -27,8 +27,29 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
 
     this.settings
     this.treeInfo
+    // inaccessible: this.scala$reflect$runtime$Gil$$gil
+    // inaccessible: this.uniqueLock
+    // inaccessible: this._skolemizationLevel
+    // inaccessible: this._undoLog
+    // inaccessible: this._intersectionWitness
+    // inaccessible: this._volatileRecursions
+    // inaccessible: this._pendingVolatiles
+    // inaccessible: this._subsametypeRecursions
+    // inaccessible: this._pendingSubTypes
+    // inaccessible: this._basetypeRecursions
+    // inaccessible: this._pendingBaseTypes
+    // inaccessible: this._lubResults
+    // inaccessible: this._glbResults
+    // inaccessible: this._indent
+    // inaccessible: this._toStringRecursions
+    // inaccessible: this._toStringSubjects
+    // inaccessible: this.atomicIds
+    // inaccessible: this.atomicExistentialIds
+    // inaccessible: this._recursionTable
+    // inaccessible: this.mirrors
     this.rootMirror
     this.treeBuild
+    // inaccessible: this.SimpleNameOrdering
     this.traceSymbols
     this.perRunCaches
     this.FreshNameExtractor
@@ -40,22 +61,27 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.SubpatternsAttachment
     this.noPrint
     this.typeDebug
+    // inaccessible: this.maxFree
     this.Range
     // inaccessible: this.posAssigner
     this.ConsoleWriter
     this.RefTree
     this.PackageDef
+    this.ClassDef
     this.ModuleDef
     this.ValOrDefDef
     this.ValDef
+    this.DefDef
     this.TypeDef
     this.LabelDef
     this.ImportSelector
     this.Import
     this.Template
+    this.Block
     this.CaseDef
     this.Alternative
     this.Star
+    this.Bind
     this.UnApply
     this.ArrayValue
     this.Function
@@ -64,12 +90,16 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.If
     this.Match
     this.Return
+    this.Try
     this.Throw
     this.New
     this.Typed
     this.TypeApply
+    this.Apply
     this.ApplyDynamic
+    this.Super
     this.This
+    this.Select
     this.Ident
     this.ReferenceToBoxed
     this.Literal
@@ -78,13 +108,16 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.SelectFromTypeTree
     this.CompoundTypeTree
     this.AppliedTypeTree
+    this.TypeBoundsTree
     this.ExistentialTypeTree
+    this.TypeTree
     this.Modifiers
     this.EmptyTree
     this.noSelfType
     this.pendingSuperCall
     this.emptyValDef
     this.EmptyTreeTypeSubstituter
+    // inaccessible: this.duplicator
     this.UnmappableAnnotArg
     this.LiteralAnnotArg
     this.ArrayAnnotArg
@@ -95,6 +128,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.UnmappableAnnotation
     this.ErroneousAnnotation
     this.ThrownException
+    // inaccessible: this.compactify
     this.tpnme
     this.fulltpnme
     this.binarynme
@@ -114,6 +148,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.ProperTypeKind
     this.TypeConKind
     this.inferKind
+    // inaccessible: this.substTypeMapCache
     this.UnmappableTree
     this.ErrorType
     this.WildcardType
@@ -150,6 +185,9 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.unwrapToStableClass
     this.unwrapWrapperTypes
     this.RecoverableCyclicReference
+    // inaccessible: this._undoLog
+    // inaccessible: this.numericLoBound
+    // inaccessible: this.numericHiBound
     this.TypeConstraint
     this.normalizeAliases
     this.dropSingletonType
@@ -161,15 +199,18 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.typeVarToOriginMap
     this.ErroneousCollector
     this.adaptToNewRunMap
+    // inaccessible: this.commonOwnerMapObj
     this.SubTypePair
     this.SymbolKind
     this.NoSymbol
     this.CyclicReference
+    // inaccessible: this.TypeHistory
     this.TermName
     this.TypeName
     this.Liftable
     this.Unliftable
     this.BooleanFlag
+    // inaccessible: this.CachedNames
     this.WeakTypeTag
     this.TypeTag
     this.Expr
@@ -386,12 +427,14 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     definitions.languageFeatureModule
     definitions.metaAnnotations
     definitions.AnnotationDefaultAttr
+    // inaccessible: definitions.erasurePhase
     definitions.isPhantomClass
     definitions.syntheticCoreClasses
     definitions.syntheticCoreMethods
     definitions.hijackedCoreClasses
     definitions.symbolsNotPresentInBytecode
     definitions.isPossibleSyntheticParent
+    // inaccessible: definitions.boxedValueClassesSet
     definitions.abbrvTag
     definitions.numericWeight
     definitions.boxedModule

test/files/run/reflection-magicsymbols-invoke.check

@@ -82,7 +82,6 @@ Array
 it's important to print the list of Array's members
 if some of them change (possibly, adding and/or removing magic symbols), we must update this test
 constructor Array: (_length: Int)Array[T]
-constructor Cloneable: ()java.lang.Cloneable
 method !=: (x$1: Any)Boolean
 method !=: (x$1: AnyRef)Boolean
 method ##: ()Int