Skip to content

Commit 779f08d

Browse files
committed
SI-7475 Refactor findMember computation into a class
This affords us: - a better chance to share code with `findMembers` again - the ability to break up the logic into smaller methods for scrutability and JIT-tability. The cost is the use of the heap rather than the stack for the working area of the computation. But I didn't notice a difference between `locker.lib` and `quick.lib` after this change. I've left the old implementation in place with an assertion to verify identical results. In the next commit, this stepping stone will be removed, and we'll update `findMembers` to share the code. Some problem areas have been highlighted and will be addressed after the refactoring phase is complete.
1 parent 1e9dcc2 commit 779f08d

File tree

3 files changed

+240
-47
lines changed

3 files changed

+240
-47
lines changed

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ trait Types
8282
with tpe.GlbLubs
8383
with tpe.TypeMaps
8484
with tpe.TypeConstraints
85+
with tpe.FindMembers
8586
with util.Collections { self: SymbolTable =>
8687

8788
import definitions._
@@ -248,7 +249,7 @@ trait Types
248249
* type instead of the proxy. This gives buried existentials a
249250
* chance to make peace with the other types. See SI-5330.
250251
*/
251-
private def narrowForFindMember(tp: Type): Type = {
252+
private[internal] def narrowForFindMember(tp: Type): Type = {
252253
val w = tp.widen
253254
// Only narrow on widened type when we have to -- narrow is expensive unless the target is a singleton type.
254255
if ((tp ne w) && containsExistential(w)) w.narrow
@@ -989,6 +990,7 @@ trait Types
989990
*
990991
*/
991992
def findMembers(excludedFlags: Long, requiredFlags: Long): Scope = {
993+
// TODO refactor to use `FindMemberBase`
992994
def findMembersInternal: Scope = {
993995
var members: Scope = null
994996
if (Statistics.canEnable) Statistics.incCounter(findMembersCount)
@@ -1055,14 +1057,25 @@ trait Types
10551057
* Find member(s) in this type. If several members matching criteria are found, they are
10561058
* returned in an OverloadedSymbol
10571059
*
1058-
* @param name The member's name, where nme.ANYNAME means `unspecified`
1060+
* @param name The member's name
10591061
* @param excludedFlags Returned members do not have these flags
10601062
* @param requiredFlags Returned members do have these flags
10611063
* @param stableOnly If set, return only members that are types or stable values
10621064
*/
1063-
//TODO: use narrow only for modules? (correct? efficiency gain?)
10641065
def findMember(name: Name, excludedFlags: Long, requiredFlags: Long, stableOnly: Boolean): Symbol = {
1065-
def findMemberInternal: Symbol = {
1066+
def findMemberInternal = {
1067+
// TODO delete `findMemberInternalOld`
1068+
val resultOld = findMemberInternalOld
1069+
val resultNew = findMemberInternalNew
1070+
if (resultOld.alternatives != resultNew.alternatives) {
1071+
findMemberInternalOld
1072+
findMemberInternalNew
1073+
}
1074+
assert(resultOld.alternatives == resultNew.alternatives, s"\nOLD:${resultOld.alternatives}\nNEW${resultNew.alternatives}")
1075+
resultNew
1076+
}
1077+
def findMemberInternalNew = new FindMember(this, name, excludedFlags, requiredFlags, stableOnly).apply()
1078+
def findMemberInternalOld: Symbol = {
10661079
var member: Symbol = NoSymbol
10671080
var members: List[Symbol] = null
10681081
var lastM: ::[Symbol] = null
Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
/* NSC -- new Scala compiler
2+
* Copyright 2005-2014 LAMP/EPFL
3+
* @author Jason Zaugg
4+
*/
5+
package scala.reflect.internal
6+
package tpe
7+
8+
import Flags._
9+
import util.Statistics
10+
import TypesStats._
11+
12+
trait FindMembers {
13+
this: SymbolTable =>
14+
15+
/** Implementatation of `Type#{findMember, findMembers}` */
16+
private[internal] abstract class FindMemberBase[T](tpe: Type, name: Name, excludedFlags: Long, requiredFlags: Long) {
17+
protected val initBaseClasses: List[Symbol] = tpe.baseClasses
18+
19+
// The first base class, or the symbol of the ThisType
20+
private[this] var _selectorClass: Symbol = null
21+
private def selectorClass: Symbol = {
22+
if (_selectorClass eq null) {
23+
_selectorClass = tpe match {
24+
case tt: ThisType => tt.sym // SI-7507 the first base class is not necessarily the selector class.
25+
case _ => initBaseClasses.head
26+
}
27+
}
28+
_selectorClass
29+
}
30+
31+
// Cache for the narrowed type of `tp` (in `tp.findMember`).
32+
// This is needed to avoid mismatched existential types are reported in SI-5330.
33+
private[this] var _self: Type = null
34+
protected def self: Type = {
35+
// TODO: use narrow only for modules? (correct? efficiency gain?) (<-- Note: this comment predates SI-5330)
36+
if (_self eq null) _self = narrowForFindMember(tpe)
37+
_self
38+
}
39+
40+
// Main entry point
41+
def apply(): T = {
42+
if (Statistics.canEnable) Statistics.incCounter(findMemberCount)
43+
val start = if (Statistics.canEnable) Statistics.pushTimer(typeOpsStack, findMemberNanos) else null
44+
try searchConcreteThenDeferred
45+
finally if (Statistics.canEnable) Statistics.popTimer(typeOpsStack, start)
46+
}
47+
48+
protected def result: T
49+
50+
// SLS 5.1.3 First, a concrete definition always overrides an abstract definition
51+
private def searchConcreteThenDeferred: T = {
52+
val deferredSeen = walkBaseClasses(requiredFlags, excludedFlags | DEFERRED)
53+
if (deferredSeen) // OPT: the `if` avoids a second pass if the first pass didn't spot any candidates.
54+
walkBaseClasses(requiredFlags | DEFERRED, excludedFlags & ~(DEFERRED.toLong))
55+
result
56+
}
57+
58+
/*
59+
* Walk up through the decls of each base class.
60+
*
61+
* Called in two passes: first excluding deferred, then mandating it.
62+
*
63+
* @return true, if a potential deferred member was seen on the first pass that calls for a second pass.
64+
*/
65+
private def walkBaseClasses(required: Long, excluded: Long): Boolean = {
66+
var bcs = initBaseClasses
67+
68+
// Has we seen a candidate deferred member?
69+
var deferredSeen = false
70+
71+
while (!bcs.isEmpty) {
72+
val currentBaseClass = bcs.head
73+
val decls = currentBaseClass.info.decls
74+
val findAll = name == nme.ANYname
75+
var entry = if (findAll) decls.elems else decls.lookupEntry(name)
76+
while (entry ne null) {
77+
val sym = entry.sym
78+
val flags = sym.flags
79+
val meetsRequirements = (flags & required) == required
80+
if (meetsRequirements) {
81+
val excl: Long = flags & excluded
82+
val isExcluded: Boolean = excl != 0L
83+
if (!isExcluded && isPotentialMember(sym, flags, currentBaseClass)) {
84+
if (shortCircuit(sym)) return false
85+
else addMemberIfNew(sym)
86+
} else if (excl == DEFERRED) {
87+
deferredSeen = true
88+
}
89+
}
90+
entry = if (findAll) entry.next else decls lookupNextEntry entry
91+
}
92+
93+
bcs = bcs.tail
94+
}
95+
deferredSeen
96+
}
97+
98+
/* Should this symbol be returned immediately as the sole result? */
99+
protected def shortCircuit(sym: Symbol): Boolean
100+
101+
/* Add this member to the final result, unless an already-found member matches it. */
102+
protected def addMemberIfNew(sym: Symbol): Unit
103+
104+
// Is `sym` a potentially member of `baseClass`?
105+
//
106+
// Q. When does a potential member fail to be a an actual member?
107+
// A. if it is subsumed by an member in a subclass.
108+
private def isPotentialMember(sym: Symbol, flags: Long, owner: Symbol): Boolean = {
109+
// conservatively (performance wise) doing this with flags masks rather than `sym.isPrivate`
110+
// to avoid multiple calls to `Symbol#flags`.
111+
val isPrivateLocal = (flags & PrivateLocal) == PrivateLocal
112+
113+
def admitPrivateLocal(sym: Symbol): Boolean =
114+
(
115+
(selectorClass == owner)
116+
|| !isPrivateLocal
117+
|| selectorClass.hasTransOwner(owner) // private[this] only a member from within the class (incorrect!)
118+
)
119+
120+
// TODO first condition incorrect wrt self types! In neg/t7507, bippy should not be a member!
121+
// The condition, or a close variant thereof, will still be needed to prevent inheritance of constructors.
122+
owner == initBaseClasses.head || admitPrivateLocal(sym) && sym.name != nme.CONSTRUCTOR
123+
}
124+
125+
// True unless the already-found member of type `memberType` matches the candidate symbol `other`.
126+
protected def isNewMember(member: Symbol, other: Symbol): Boolean =
127+
( (other ne member)
128+
&& ( (member.owner eq other.owner)
129+
|| (other.flags & PRIVATE) != 0
130+
|| !(memberTypeLow(member) matches memberTypeHi(other))
131+
)
132+
)
133+
134+
// Cache for the member type of a candidate member when comparing against multiple, already-found existing members
135+
//
136+
// TODO this cache is probably unnecessary, `tp.memberType(sym: MethodSymbol)` is already cached internally.
137+
private[this] var _memberTypeHiCache: Type = null
138+
private[this] var _memberTypeHiCacheSym: Symbol = null
139+
140+
protected def memberTypeHi(sym: Symbol): Type = {
141+
if (_memberTypeHiCacheSym ne sym) {
142+
_memberTypeHiCache = self.memberType(sym)
143+
_memberTypeHiCacheSym = sym
144+
}
145+
_memberTypeHiCache
146+
}
147+
148+
// member type of the LHS of `matches` call. This is an extension point to enable a cache in
149+
// FindMember.
150+
protected def memberTypeLow(sym: Symbol): Type = self.memberType(sym)
151+
}
152+
153+
private[reflect] final class FindMember(tpe: Type, name: Name, excludedFlags: Long, requiredFlags: Long, stableOnly: Boolean)
154+
extends FindMemberBase[Symbol](tpe, name, excludedFlags, requiredFlags) {
155+
// Gathering the results into a hand rolled ListBuffer
156+
// TODO Try just using a ListBuffer to see if this low-level-ness is worth it.
157+
private[this] var member0: Symbol = NoSymbol
158+
private[this] var members: List[Symbol] = null
159+
private[this] var lastM: ::[Symbol] = null
160+
161+
private def clearAndAddResult(sym: Symbol): Unit = {
162+
member0 = sym
163+
members = null
164+
lastM = null
165+
}
166+
167+
protected def shortCircuit(sym: Symbol): Boolean = (name.isTypeName || (stableOnly && sym.isStable && !sym.hasVolatileType)) && {
168+
clearAndAddResult(sym)
169+
true
170+
}
171+
172+
protected def addMemberIfNew(sym: Symbol): Unit =
173+
if (member0 eq NoSymbol) {
174+
member0 = sym // The first found member
175+
} else if (members eq null) {
176+
// We've found exactly one member so far...
177+
if (isNewMember(member0, sym)) {
178+
// ... make that two.
179+
lastM = new ::(sym, null)
180+
members = member0 :: lastM
181+
}
182+
} else {
183+
// Already found 2 or more members
184+
var ms: List[Symbol] = members
185+
186+
var isNew = true
187+
while ((ms ne null) && isNew) {
188+
val member = ms.head
189+
if (!isNewMember(member, sym))
190+
isNew = false
191+
ms = ms.tail
192+
}
193+
if (isNew) {
194+
val lastM1 = new ::(sym, null)
195+
lastM.tl = lastM1
196+
lastM = lastM1
197+
}
198+
}
199+
200+
// Cache for the member type of the first member we find.
201+
private[this] var _member0Tpe: Type = null
202+
private[this] def member0Tpe: Type = {
203+
assert(member0 != null)
204+
if (_member0Tpe eq null) _member0Tpe = self.memberType(member0)
205+
_member0Tpe
206+
}
207+
208+
override protected def memberTypeLow(sym: Symbol): Type =
209+
if (sym eq member0) member0Tpe else super.memberTypeLow(sym)
210+
211+
// Assemble the result from the hand-rolled ListBuffer
212+
protected def result: Symbol = if (members eq null) {
213+
if (member0 == NoSymbol) {
214+
if (Statistics.canEnable) Statistics.incCounter(noMemberCount)
215+
NoSymbol
216+
} else member0
217+
} else {
218+
if (Statistics.canEnable) Statistics.incCounter(multMemberCount)
219+
lastM.tl = Nil
220+
initBaseClasses.head.newOverloaded(tpe, members)
221+
}
222+
}
223+
}

0 commit comments

Comments
 (0)