SI-5732 members and derivatives now return Scope

Firstly this unifies the reflection API - now both decls and members
return Scope (not Scope and List[Symbol] as it were before).

Secondly this fixes SI-5732 without having to sort the result of members.
Type.members now returns Scope, a distinguished type, which has the
`sorted` method, which does the required sorting if necessary.

Also removes nonPrivateMembers and nonPrivateDeclarations to keep the API
minimalistic (as can be seen from their implementation in internal.Types
they are just members and decls with bridges and private members removed).

5 files changed, 69 insertions, 40 deletions

diff --git a/src/reflect/scala/reflect/api/Types.scala b/src/reflect/scala/reflect/api/Types.scala
index 01de5fa9a7..33248fcbee 100644
--- a/src/reflect/scala/reflect/api/Types.scala
+++ b/src/reflect/scala/reflect/api/Types.scala
@@ -24,33 +24,26 @@ trait Types extends base.Types { self: Universe =>
      */
     def declaration(name: Name): Symbol
 
-    /** The collection of declarations in this type
-     *  [Eugene++] why not List?
+    /** A `Scope` containing directly declared members of this type.
+     *  Unlike `members` this method doesn't returns inherited members.
+     *
+     *  Members in the returned scope might appear in arbitrary order.
+     *  Use `declarations.sorted` to get an ordered list of members.
      */
-    def declarations: Iterable[Symbol]
+    def declarations: MemberScope
 
     /** The member with given name, either directly declared or inherited,
      *  an OverloadedSymbol if several exist, NoSymbol if none exist.
      */
     def member(name: Name): Symbol
 
-    /** The non-private member with given name, either directly declared or inherited,
-     *  an OverloadedSymbol if several exist, NoSymbol if none exist.
-     */
-    def nonPrivateMember(name: Name): Symbol
-
-    /** An iterable containing all members of this type (directly declared or inherited)
-     *  Members appear in the linearization order of their owners.
-     *  Members with the same owner appear in reverse order of their declarations.
-     *  [Eugene++] the order needs to be reversed back, at least in the public API
-     */
-    def members: Iterable[Symbol]
-
-    /** An iterable containing all non-private members of this type (directly declared or inherited)
-     *  Members appear in the linearization order of their owners.
-     *  Members with the same owner appear in reverse order of their declarations.
+    /** A `Scope` containing all members of this type (directly declared or inherited).
+     *  Unlike `declarations` this method also returns inherited members.
+     *
+     *  Members in the returned scope might appear in arbitrary order.
+     *  Use `declarations.sorted` to get an ordered list of members.
      */
-    def nonPrivateMembers: Iterable[Symbol]
+    def members: MemberScope
 
     /** Substitute symbols in `to` for corresponding occurrences of references to
      *  symbols `from` in this type.
diff --git a/src/reflect/scala/reflect/internal/Scopes.scala b/src/reflect/scala/reflect/internal/Scopes.scala
index 89e3c52de6..ed390b5a3b 100644
--- a/src/reflect/scala/reflect/internal/Scopes.scala
+++ b/src/reflect/scala/reflect/internal/Scopes.scala
@@ -41,9 +41,9 @@ trait Scopes extends api.Scopes { self: SymbolTable =>
    *  This is necessary because when run from reflection every scope needs to have a
    *  SynchronizedScope as mixin.
    */
-  class Scope protected[Scopes] (initElems: ScopeEntry = null, initFingerPrints: Long = 0L) extends Iterable[Symbol] {
-    
-    /** A bitset containing the last 6 bits of the start value of every name 
+  class Scope protected[Scopes] (initElems: ScopeEntry = null, initFingerPrints: Long = 0L) extends ScopeBase with MemberScopeBase {
+
+    /** A bitset containing the last 6 bits of the start value of every name
      *  stored in this scope.
      */
     var fingerPrints: Long = initFingerPrints
@@ -118,10 +118,10 @@ trait Scopes extends api.Scopes { self: SymbolTable =>
      *
      *  @param sym ...
      */
-    def enter[T <: Symbol](sym: T): T = { 
+    def enter[T <: Symbol](sym: T): T = {
       fingerPrints |= sym.name.fingerPrint
-      enterEntry(newScopeEntry(sym, this)) 
-      sym 
+      enterEntry(newScopeEntry(sym, this))
+      sym
     }
 
     /** enter a symbol, asserting that no symbol with same name exists in scope
@@ -282,6 +282,10 @@ trait Scopes extends api.Scopes { self: SymbolTable =>
       elemsCache
     }
 
+    /** Vanilla scope - symbols are stored in declaration order.
+     */
+    def sorted: List[Symbol] = toList
+
     /** Return the nesting level of this scope, i.e. the number of times this scope
      *  was nested in another */
     def nestingLevel = nestinglevel
@@ -324,14 +328,46 @@ trait Scopes extends api.Scopes { self: SymbolTable =>
       toList.map(_.defString).mkString(start, sep, end)
 
     override def toString(): String = mkString("Scope{\n  ", ";\n  ", "\n}")
-
   }
 
   implicit val ScopeTag = ClassTag[Scope](classOf[Scope])
 
+  type MemberScope = Scope
+
+  implicit val MemberScopeTag = ClassTag[MemberScope](classOf[MemberScope])
+
   /** Create a new scope */
   def newScope: Scope = new Scope()
 
+  /** Create a new scope to be used in `findMembers`.
+   *
+   *  But why do we need a special scope for `findMembers`?
+   *  Let me tell you a story.
+   *
+   * `findMembers` creates a synthetic scope and then iterates over
+   *  base classes in linearization order, and for every scrutinized class
+   *  iterates over `decls`, the collection of symbols declared in that class.
+   *  Declarations that fit the filter get appended to the created scope.
+   *
+   *  The problem is that `decls` returns a Scope, and to iterate a scope performantly
+   *  one needs to go from its end to its beginning.
+   *
+   *  Hence the `findMembers` scope is populated in a wicked order:
+   *  symbols that belong to the same declaring class come in reverse order of their declaration,
+   *  however, the scope itself is ordered w.r.t the linearization of the target type.
+   *
+   *  Once `members` became a public API, this has been confusing countless numbers of users.
+   *  Therefore we introduce a special flavor of scopes to accommodate this quirk of `findMembers`
+   */
+  private[scala] def newFindMemberScope: Scope = new Scope() {
+    override def sorted = {
+      val members = toList
+      val owners = members.map(_.owner).distinct
+      val grouped = members groupBy (_.owner)
+      owners.flatMap(owner => grouped(owner).reverse)
+    }
+  }
+
   /** Create a new scope nested in another one with which it shares its elements */
   def newNestedScope(outer: Scope): Scope = new Scope(outer)
 
diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
index e6a9cb46c6..05ead8d1ac 100644
--- a/src/reflect/scala/reflect/internal/Symbols.scala
+++ b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -2890,7 +2890,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     private[this] var typeOfThisCache: Type = _
     private[this] var typeOfThisPeriod      = NoPeriod
 
-    private var implicitMembersCacheValue: List[Symbol] = Nil
+    private var implicitMembersCacheValue: Scope = EmptyScope
     private var implicitMembersCacheKey1: Type = NoType
     private var implicitMembersCacheKey2: ScopeEntry = null
 
@@ -2909,7 +2909,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
       typeOfThisCache
     }
 
-    def implicitMembers: List[Symbol] = {
+    def implicitMembers: Scope = {
       val tp = info
       if ((implicitMembersCacheKey1 ne tp) || (implicitMembersCacheKey2 ne tp.decls.elems)) {
         // Skip a package object class, because the members are also in
diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala
index 8972dfa828..1386cca42f 100644
--- a/src/reflect/scala/reflect/internal/Types.scala
+++ b/src/reflect/scala/reflect/internal/Types.scala
@@ -612,21 +612,21 @@ trait Types extends api.Types { self: SymbolTable =>
      *  Members appear in linearization order of their owners.
      *  Members with the same owner appear in reverse order of their declarations.
      */
-    def members: List[Symbol] = membersBasedOnFlags(0, 0)
+    def members: Scope = membersBasedOnFlags(0, 0)
 
     /** A list of all non-private members of this type (defined or inherited) */
-    def nonPrivateMembers: List[Symbol] = membersBasedOnFlags(BridgeAndPrivateFlags, 0)
+    def nonPrivateMembers: Scope = membersBasedOnFlags(BridgeAndPrivateFlags, 0)
 
     /** A list of all non-private members of this type  (defined or inherited),
      *  admitting members with given flags `admit`
      */
-    def nonPrivateMembersAdmitting(admit: Long): List[Symbol] = membersBasedOnFlags(BridgeAndPrivateFlags & ~admit, 0)
+    def nonPrivateMembersAdmitting(admit: Long): Scope = membersBasedOnFlags(BridgeAndPrivateFlags & ~admit, 0)
 
     /** A list of all implicit symbols of this type  (defined or inherited) */
-    def implicitMembers: List[Symbol] = membersBasedOnFlags(BridgeFlags, IMPLICIT)
+    def implicitMembers: Scope = membersBasedOnFlags(BridgeFlags, IMPLICIT)
 
     /** A list of all deferred symbols of this type  (defined or inherited) */
-    def deferredMembers: List[Symbol] = membersBasedOnFlags(BridgeFlags, DEFERRED)
+    def deferredMembers: Scope = membersBasedOnFlags(BridgeFlags, DEFERRED)
 
     /** The member with given name,
      *  an OverloadedSymbol if several exist, NoSymbol if none exist */
@@ -642,12 +642,12 @@ trait Types extends api.Types { self: SymbolTable =>
 
     /** All members with the given flags, excluding bridges.
      */
-    def membersWithFlags(requiredFlags: Long): List[Symbol] =
+    def membersWithFlags(requiredFlags: Long): Scope =
       membersBasedOnFlags(BridgeFlags, requiredFlags)
 
     /** All non-private members with the given flags, excluding bridges.
      */
-    def nonPrivateMembersWithFlags(requiredFlags: Long): List[Symbol] =
+    def nonPrivateMembersWithFlags(requiredFlags: Long): Scope =
       membersBasedOnFlags(BridgeAndPrivateFlags, requiredFlags)
 
     /** The non-private member with given name, admitting members with given flags `admit`.
@@ -668,7 +668,7 @@ trait Types extends api.Types { self: SymbolTable =>
     /** Members excluding and requiring the given flags.
      *  Note: unfortunately it doesn't work to exclude DEFERRED this way.
      */
-    def membersBasedOnFlags(excludedFlags: Long, requiredFlags: Long): List[Symbol] =
+    def membersBasedOnFlags(excludedFlags: Long, requiredFlags: Long): Scope =
       findMembers(excludedFlags, requiredFlags)
 //      findMember(nme.ANYNAME, excludedFlags, requiredFlags, false).alternatives
 
@@ -1020,7 +1020,7 @@ trait Types extends api.Types { self: SymbolTable =>
       else (baseClasses.head.newOverloaded(this, alts))
     }
 
-    def findMembers(excludedFlags: Long, requiredFlags: Long): List[Symbol] = {
+    def findMembers(excludedFlags: Long, requiredFlags: Long): Scope = {
       // if this type contains type variables, put them to sleep for a while -- don't just wipe them out by
       // replacing them by the corresponding type parameter, as that messes up (e.g.) type variables in type refinements
       // without this, the matchesType call would lead to type variables on both sides
@@ -1054,7 +1054,7 @@ trait Types extends api.Types { self: SymbolTable =>
                    (bcs eq bcs0) ||
                    (flags & PrivateLocal) != PrivateLocal ||
                    (bcs0.head.hasTransOwner(bcs.head)))) {
-                if (members eq null) members = newScope
+                if (members eq null) members = newFindMemberScope
                 var others: ScopeEntry = members.lookupEntry(sym.name)
                 var symtpe: Type = null
                 while ((others ne null) && {
@@ -1083,7 +1083,7 @@ trait Types extends api.Types { self: SymbolTable =>
       } // while (continue)
       Statistics.popTimer(typeOpsStack, start)
       if (suspension ne null) suspension foreach (_.suspended = false)
-      if (members eq null) Nil else members.toList
+      if (members eq null) EmptyScope else members
     }
 
     /**
diff --git a/src/reflect/scala/reflect/runtime/SynchronizedSymbols.scala b/src/reflect/scala/reflect/runtime/SynchronizedSymbols.scala
index 3b28ddf42c..6a3501b1dd 100644
--- a/src/reflect/scala/reflect/runtime/SynchronizedSymbols.scala
+++ b/src/reflect/scala/reflect/runtime/SynchronizedSymbols.scala
@@ -134,7 +134,7 @@ trait SynchronizedSymbols extends internal.Symbols { self: SymbolTable =>
     override def sourceModule = synchronized { super.sourceModule }
     // [Eugene++ to Martin] doesn't override anything. no longer necessary?
     // def sourceModule_=(module: ModuleSymbol) = synchronized { super.sourceModule_=(module) }
-    override def implicitMembers: List[Symbol] = synchronized { super.implicitMembers }
+    override def implicitMembers: Scope = synchronized { super.implicitMembers }
   }
 }