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

test/files/presentation/ide-bug-1000531.check

@@ -3,7 +3,7 @@ reload: CrashOnLoad.scala
 askTypeCompletion at CrashOnLoad.scala(6,12)
 ================================================================================
 [response] askTypeCompletion at (6,12)
-retrieved 120 members
+retrieved 119 members
 [inaccessible] protected[package lang] def clone(): Object
 [inaccessible] protected[package lang] def finalize(): Unit
 [inaccessible] protected[this] def reversed: List[B]

test/files/presentation/ping-pong.check

@@ -3,7 +3,7 @@ reload: PingPong.scala
 askTypeCompletion at PingPong.scala(10,23)
 ================================================================================
 [response] askTypeCompletion at (10,23)
-retrieved 35 members
+retrieved 34 members
 [inaccessible] private[this] val ping: Ping
 [inaccessible] protected[package lang] def clone(): Object
 [inaccessible] protected[package lang] def finalize(): Unit

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