Skip to content

Commit

Permalink
SI-7475 Private members are not inheritable
Browse files Browse the repository at this point in the history
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
  • Loading branch information
retronym committed Jan 29, 2014
1 parent 0f56d6c commit c5ed740
Show file tree
Hide file tree
Showing 15 changed files with 124 additions and 13 deletions.
31 changes: 23 additions & 8 deletions src/reflect/scala/reflect/internal/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1055,7 +1055,7 @@ trait Types
* Find member(s) in this type. If several members matching criteria are found, they are
* returned in an OverloadedSymbol
*
* @param name The member's name, where nme.ANYNAME means `unspecified`
* @param name The member's name
* @param excludedFlags Returned members do not have these flags
* @param requiredFlags Returned members do have these flags
* @param stableOnly If set, return only members that are types or stable values
Expand All @@ -1072,21 +1072,22 @@ trait Types
//Console.println("find member " + name.decode + " in " + this + ":" + this.baseClasses)//DEBUG
var membertpe: Type = null
var required = requiredFlags
var excluded = excludedFlags | DEFERRED
var excluded: Long = excludedFlags | DEFERRED
var continue = true
var self: Type = null
var seenFirstNonRefinementClass: Boolean = false
var refinementParents: List[Symbol] = Nil

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)
selectorClass == owner
}
while (!bcs.isEmpty) {
val decls = bcs.head.info.decls
Expand All @@ -1098,11 +1099,19 @@ trait Types
val excl = flags & excluded
val isMember = (
excl == 0L
&& ( (bcs eq bcs0)
|| (flags & PrivateLocal) != PrivateLocal
|| admitPrivateLocal(bcs.head)
&& (
(flags & PRIVATE) != PRIVATE // non-privates are always members
|| (
!seenFirstNonRefinementClass // classes don't inherit privates
|| refinementParents.contains(bcs.head) // refinements inherit privates of direct parents
)
|| (
(flags & PrivateLocal) == PrivateLocal
&& admitPrivateLocal(bcs.head)
)
)
)

if (isMember) {
if (name.isTypeName || (stableOnly && sym.isStable && !sym.hasVolatileType)) {
if (Statistics.canEnable) Statistics.popTimer(typeOpsStack, start)
Expand Down Expand Up @@ -1155,7 +1164,13 @@ trait Types
}
entry = decls lookupNextEntry entry
} // while (entry ne null)
// excluded = excluded | LOCAL

val sym = bcs.head
if (sym.isRefinementClass)
refinementParents :::= bcs.head.parentSymbols // keep track of direct parents of refinements
else if (sym.isClass)
seenFirstNonRefinementClass = true

bcs = if (name == nme.CONSTRUCTOR) Nil else bcs.tail
} // while (!bcs.isEmpty)
required |= DEFERRED
Expand Down
5 changes: 1 addition & 4 deletions test/files/neg/forgot-interpolator.check
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ forgot-interpolator.scala:30: warning: `$beppo` looks like an interpolated ident
forgot-interpolator.scala:34: warning: `$aleppo` looks like an interpolated identifier! Did you forget the interpolator?
def f = "$aleppo is a pepper and a city." // warn 4
^
forgot-interpolator.scala:42: warning: `$bar` looks like an interpolated identifier! Did you forget the interpolator?
def f = "$bar is private, shall we warn just in case?" // warn 5
^
forgot-interpolator.scala:47: warning: `$hippo` looks like an interpolated identifier! Did you forget the interpolator?
def h = "$hippo takes an implicit" // warn 6
^
Expand All @@ -26,5 +23,5 @@ forgot-interpolator.scala:90: warning: `$calico` looks like an interpolated iden
def f4 = "I also salute $calico" // warn 9
^
error: No warnings can be incurred under -Xfatal-warnings.
9 warnings found
8 warnings found
one error found
2 changes: 1 addition & 1 deletion test/files/neg/forgot-interpolator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ package test {
if (bar > 8) ??? // use it to avoid extra warning
}
class Baz extends Bar {
def f = "$bar is private, shall we warn just in case?" // warn 5
def f = "$bar is private, shall we warn just in case?" // no longer a warning, private members aren't inherited!
}
class G {
def g = "$greppo takes an arg" // no warn
Expand Down
7 changes: 7 additions & 0 deletions test/files/neg/t7475c.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
t7475c.scala:6: error: value a is not a member of A.this.B
println(this.a) // wait, what?
^
t7475c.scala:7: error: value b is not a member of A.this.B
println(this.b) // wait, what?
^
two errors found
9 changes: 9 additions & 0 deletions test/files/neg/t7475c.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class A {
private val a: Int = 0
private[this] val b: Int = 0
class B extends A {
def foo(a: A) = a.a // okay
println(this.a) // wait, what?
println(this.b) // wait, what?
}
}
7 changes: 7 additions & 0 deletions test/files/neg/t7475d.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
t7475d.scala:4: error: value priv is not a member of T.this.TT
(??? : TT).priv
^
t7475d.scala:10: error: value priv is not a member of U.this.UU
(??? : UU).priv
^
two errors found
4 changes: 4 additions & 0 deletions test/files/neg/t7475e.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
t7475e.scala:8: error: value priv is not a member of Base.this.TT
(??? : TT).priv
^
one error found
12 changes: 12 additions & 0 deletions test/files/neg/t7475e.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
trait U {
}

trait Base {
private val priv = 0

type TT = U with T // should exclude `priv`
(??? : TT).priv
}

trait T extends Base {
}
11 changes: 11 additions & 0 deletions test/files/pos/t7475a.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
trait AbstractPublic {
def queue: Any
}
trait ConcretePrivate {
private val queue: Any = ()
}

abstract class Mix
extends ConcretePrivate with AbstractPublic {
final def queue: Any = ()
}
8 changes: 8 additions & 0 deletions test/files/pos/t7475b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
trait U {
}

trait T {
type TT = Any with T with U
private val priv = 0
(??? : TT).priv
}
11 changes: 11 additions & 0 deletions test/files/pos/t7475d.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
trait T {
type TT = T with Any
private val priv = 0
(??? : TT).priv
}

trait U {
type UU = Any with U
private val priv = 0
(??? : UU).priv
}
13 changes: 13 additions & 0 deletions test/files/pos/t7475e.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
trait U {
private val priv = 0
type TT = U with T // should allow `priv`
(??? : TT).priv
}

trait Base {

}

trait T extends Base {

}
2 changes: 2 additions & 0 deletions test/files/run/t7475b.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
2
2
11 changes: 11 additions & 0 deletions test/files/run/t7475b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
trait A { private val x = 1 }
trait B { val x = 2 }
trait C1 extends B with A { println(x) }
trait C2 extends A with B { println(x) }

object Test {
def main(args: Array[String]): Unit = {
new C1 { }
new C2 { }
}
}
4 changes: 4 additions & 0 deletions test/files/run/t7507.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ trait Cake extends Slice
trait Slice { self: Cake => // must have self type that extends `Slice`
private[this] val bippy = () // must be private[this]
locally(bippy)
class C1 {
locally(bippy)
locally(self.bippy)
}
}

// Originally reported bug:
Expand Down

0 comments on commit c5ed740

Please sign in to comment.