Skip to content

Commit c5ed740

Browse files
retronymretronym
committed
SI-7475 Private members are not inheritable
Exclude them from superclasses in `findMember` and in `OverridingPairs`. The odd logic in `findMember` that considered whether the selector class was owned by the owner of the candidate private symbol dates back to 2007 (bff4268), but does not appear to have any relationship to the spec. Refinement types are still able to inherit private members from all direct parents, as was needed in pos/t2399.scala. More tests are included for this scenario. In short, the logic now: - includes direct parents of refinements, - otherwise, excludes privates after the first class in the base class sequence TODO: Swathes of important logic are duplicated between `findMember` and `findMembers` after this run of optimization. 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 reflection. Als 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
1 parent 0f56d6c commit c5ed740

15 files changed

+124
-13
lines changed

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

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,7 +1055,7 @@ trait Types
10551055
* Find member(s) in this type. If several members matching criteria are found, they are
10561056
* returned in an OverloadedSymbol
10571057
*
1058-
* @param name The member's name, where nme.ANYNAME means `unspecified`
1058+
* @param name The member's name
10591059
* @param excludedFlags Returned members do not have these flags
10601060
* @param requiredFlags Returned members do have these flags
10611061
* @param stableOnly If set, return only members that are types or stable values
@@ -1072,21 +1072,22 @@ trait Types
10721072
//Console.println("find member " + name.decode + " in " + this + ":" + this.baseClasses)//DEBUG
10731073
var membertpe: Type = null
10741074
var required = requiredFlags
1075-
var excluded = excludedFlags | DEFERRED
1075+
var excluded: Long = excludedFlags | DEFERRED
10761076
var continue = true
10771077
var self: Type = null
1078+
var seenFirstNonRefinementClass: Boolean = false
1079+
var refinementParents: List[Symbol] = Nil
10781080

10791081
while (continue) {
10801082
continue = false
10811083
val bcs0 = baseClasses
10821084
var bcs = bcs0
1083-
// omit PRIVATE LOCALS unless selector class is contained in class owning the def.
10841085
def admitPrivateLocal(owner: Symbol): Boolean = {
10851086
val selectorClass = this match {
10861087
case tt: ThisType => tt.sym // SI-7507 the first base class is not necessarily the selector class.
10871088
case _ => bcs0.head
10881089
}
1089-
selectorClass.hasTransOwner(owner)
1090+
selectorClass == owner
10901091
}
10911092
while (!bcs.isEmpty) {
10921093
val decls = bcs.head.info.decls
@@ -1098,11 +1099,19 @@ trait Types
10981099
val excl = flags & excluded
10991100
val isMember = (
11001101
excl == 0L
1101-
&& ( (bcs eq bcs0)
1102-
|| (flags & PrivateLocal) != PrivateLocal
1103-
|| admitPrivateLocal(bcs.head)
1102+
&& (
1103+
(flags & PRIVATE) != PRIVATE // non-privates are always members
1104+
|| (
1105+
!seenFirstNonRefinementClass // classes don't inherit privates
1106+
|| refinementParents.contains(bcs.head) // refinements inherit privates of direct parents
1107+
)
1108+
|| (
1109+
(flags & PrivateLocal) == PrivateLocal
1110+
&& admitPrivateLocal(bcs.head)
1111+
)
11041112
)
11051113
)
1114+
11061115
if (isMember) {
11071116
if (name.isTypeName || (stableOnly && sym.isStable && !sym.hasVolatileType)) {
11081117
if (Statistics.canEnable) Statistics.popTimer(typeOpsStack, start)
@@ -1155,7 +1164,13 @@ trait Types
11551164
}
11561165
entry = decls lookupNextEntry entry
11571166
} // while (entry ne null)
1158-
// excluded = excluded | LOCAL
1167+
1168+
val sym = bcs.head
1169+
if (sym.isRefinementClass)
1170+
refinementParents :::= bcs.head.parentSymbols // keep track of direct parents of refinements
1171+
else if (sym.isClass)
1172+
seenFirstNonRefinementClass = true
1173+
11591174
bcs = if (name == nme.CONSTRUCTOR) Nil else bcs.tail
11601175
} // while (!bcs.isEmpty)
11611176
required |= DEFERRED

‎test/files/neg/forgot-interpolator.check

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,6 @@ forgot-interpolator.scala:30: warning: `$beppo` looks like an interpolated ident
1010
forgot-interpolator.scala:34: warning: `$aleppo` looks like an interpolated identifier! Did you forget the interpolator?
1111
def f = "$aleppo is a pepper and a city." // warn 4
1212
^
13-
forgot-interpolator.scala:42: warning: `$bar` looks like an interpolated identifier! Did you forget the interpolator?
14-
def f = "$bar is private, shall we warn just in case?" // warn 5
15-
^
1613
forgot-interpolator.scala:47: warning: `$hippo` looks like an interpolated identifier! Did you forget the interpolator?
1714
def h = "$hippo takes an implicit" // warn 6
1815
^
@@ -26,5 +23,5 @@ forgot-interpolator.scala:90: warning: `$calico` looks like an interpolated iden
2623
def f4 = "I also salute $calico" // warn 9
2724
^
2825
error: No warnings can be incurred under -Xfatal-warnings.
29-
9 warnings found
26+
8 warnings found
3027
one error found

‎test/files/neg/forgot-interpolator.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ package test {
3939
if (bar > 8) ??? // use it to avoid extra warning
4040
}
4141
class Baz extends Bar {
42-
def f = "$bar is private, shall we warn just in case?" // warn 5
42+
def f = "$bar is private, shall we warn just in case?" // no longer a warning, private members aren't inherited!
4343
}
4444
class G {
4545
def g = "$greppo takes an arg" // no warn

‎test/files/neg/t7475c.check

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
t7475c.scala:6: error: value a is not a member of A.this.B
2+
println(this.a) // wait, what?
3+
^
4+
t7475c.scala:7: error: value b is not a member of A.this.B
5+
println(this.b) // wait, what?
6+
^
7+
two errors found

‎test/files/neg/t7475c.scala

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
class A {
2+
private val a: Int = 0
3+
private[this] val b: Int = 0
4+
class B extends A {
5+
def foo(a: A) = a.a // okay
6+
println(this.a) // wait, what?
7+
println(this.b) // wait, what?
8+
}
9+
}

‎test/files/neg/t7475d.check

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
t7475d.scala:4: error: value priv is not a member of T.this.TT
2+
(??? : TT).priv
3+
^
4+
t7475d.scala:10: error: value priv is not a member of U.this.UU
5+
(??? : UU).priv
6+
^
7+
two errors found

‎test/files/neg/t7475e.check

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
t7475e.scala:8: error: value priv is not a member of Base.this.TT
2+
(??? : TT).priv
3+
^
4+
one error found

‎test/files/neg/t7475e.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
trait U {
2+
}
3+
4+
trait Base {
5+
private val priv = 0
6+
7+
type TT = U with T // should exclude `priv`
8+
(??? : TT).priv
9+
}
10+
11+
trait T extends Base {
12+
}

‎test/files/pos/t7475a.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
trait AbstractPublic {
2+
def queue: Any
3+
}
4+
trait ConcretePrivate {
5+
private val queue: Any = ()
6+
}
7+
8+
abstract class Mix
9+
extends ConcretePrivate with AbstractPublic {
10+
final def queue: Any = ()
11+
}

‎test/files/pos/t7475b.scala

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
trait U {
2+
}
3+
4+
trait T {
5+
type TT = Any with T with U
6+
private val priv = 0
7+
(??? : TT).priv
8+
}

‎test/files/pos/t7475d.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
trait T {
2+
type TT = T with Any
3+
private val priv = 0
4+
(??? : TT).priv
5+
}
6+
7+
trait U {
8+
type UU = Any with U
9+
private val priv = 0
10+
(??? : UU).priv
11+
}

‎test/files/pos/t7475e.scala

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
trait U {
2+
private val priv = 0
3+
type TT = U with T // should allow `priv`
4+
(??? : TT).priv
5+
}
6+
7+
trait Base {
8+
9+
}
10+
11+
trait T extends Base {
12+
13+
}

‎test/files/run/t7475b.check

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
2
2+
2

‎test/files/run/t7475b.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
trait A { private val x = 1 }
2+
trait B { val x = 2 }
3+
trait C1 extends B with A { println(x) }
4+
trait C2 extends A with B { println(x) }
5+
6+
object Test {
7+
def main(args: Array[String]): Unit = {
8+
new C1 { }
9+
new C2 { }
10+
}
11+
}

‎test/files/run/t7507.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ trait Cake extends Slice
44
trait Slice { self: Cake => // must have self type that extends `Slice`
55
private[this] val bippy = () // must be private[this]
66
locally(bippy)
7+
class C1 {
8+
locally(bippy)
9+
locally(self.bippy)
10+
}
711
}
812

913
// Originally reported bug:

0 commit comments

Comments
 (0)