diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2014-01-31 14:29:19 +0100 |
---|---|---|
committer | Adriaan Moors <adriaan.moors@typesafe.com> | 2014-02-10 14:39:18 -0800 |
commit | 8d96380caeb1f03da8916d494e878f57a363f459 (patch) | |
tree | c297c3e0e1348ecffd1cbb8d4b642e2dbf01a2d6 /test/files | |
parent | aebe379a14ab7b2a3bce35a6faa9d9232c748154 (diff) | |
download | scala-8d96380caeb1f03da8916d494e878f57a363f459.tar.gz scala-8d96380caeb1f03da8916d494e878f57a363f459.tar.bz2 scala-8d96380caeb1f03da8916d494e878f57a363f459.zip |
SI-7475 Private members are not inheritable
It turns out `findMembers` has been a bit sloppy in recent
years and has returned private members from *anywhere* up the
base class sequence. Access checks usually pick up the slack
and eliminate the unwanted privates.
But, in concert with the "concrete beats abstract" rule in
`findMember`, the following mishap appeared:
scala> :paste
// Entering paste mode (ctrl-D to finish)
trait T { def a: Int }
trait B { private def a: Int = 0 }
trait C extends T with B { a }
// Exiting paste mode, now interpreting.
<console>:9: error: method a in trait B cannot be accessed in C
trait C extends T with B { a }
^
I noticed this when compiling Akka against JDK 8; a new private method
in the bowels of the JDK was enough to break the build!
It turns out that some finesse in needed to interpret SLS 5.2:
> The private modifier can be used with any definition or declaration
> in a template. They are not inherited by subclasses [...]
So, can we simply exclude privates from all but the first base class?
No, as that might be a refinement class! The following must be
allowed:
trait A { private def foo = 0; trait T { self: A => this.foo } }
This commit:
- tracks when the walk up the base class sequence passes the
first non-refinement class, and excludes private members
- ... except, if we are at a direct parent of a refinement
class itself
- Makes a corresponding change to OverridingPairs, to only consider
private members if they are owned by the `base` Symbol under
consideration. We don't need to deal with the subtleties of
refinements there as that code is only used for bona-fide classes.
- replaces use of `hasTransOwner` when considering whether a
private[this] symbol is a member.
The last condition was not grounded in the spec at all. The change
is visible in cases like:
// Old
scala> trait A { private[this] val x = 0; class B extends A { this.x } }
<console>:7: error: value x in trait A cannot be accessed in A.this.B
trait A { private[this] val x = 0; class B extends A { this.x } }
^
// New
scala> trait A { private[this] val x = 0; class B extends A { this.x } }
<console>:8: error: value x is not a member of A.this.B
trait A { private[this] val x = 0; class B extends A { this.x } }
^
Furthermore, we no longer give a `private[this]` member a free pass
if it is sourced from the very first base class.
trait Cake extends Slice {
private[this] val bippy = ()
}
trait Slice { self: Cake =>
bippy // BCS: Cake, Slice, AnyRef, Any
}
The different handling between `private` and `private[this]`
still seems a bit dubious.
The spec says:
> An different form of qualification is private[this]. A member M
> marked with this modifier can be accessed only from within the
> object in which it is defined. That is, a selection p.M is only
> legal if the prefix is this or O.this, for some class O enclosing
> the reference. In addition, the restrictions for unqualified
> private apply.
This sounds like a question of access, not membership. If so, we
should admit `private[this]` members from parents of refined types
in `FindMember`.
AFAICT, not too much rests on the distinction: do we get a
"no such member", or "member foo inaccessible" error? I welcome
scrutinee of the checkfile of `neg/t7475f.scala` to help put
this last piece into the puzzle.
One more thing: findMember does not have *any* code the corresponds
to the last sentence of:
> SLS 5.2 The modifier can be qualified with an identifier C
> (e.g. private[C]) that must denote a class or package enclosing
> the definition. Members labeled with such a modifier are accessible
> respectively only from code inside the package C or only from code
> inside the class C and its companion module (ยง5.4).
> Such members are also inherited only from templates inside C.
When I showed Martin this, he suggested it was an error in the
spec, and we should leave the access checking to callers of
that inherited qualified-private member.
Diffstat (limited to 'test/files')
-rw-r--r-- | test/files/neg/forgot-interpolator.check | 5 | ||||
-rw-r--r-- | test/files/neg/forgot-interpolator.scala | 2 | ||||
-rw-r--r-- | test/files/neg/t7475c.check | 7 | ||||
-rw-r--r-- | test/files/neg/t7475c.scala | 9 | ||||
-rw-r--r-- | test/files/neg/t7475d.check | 7 | ||||
-rw-r--r-- | test/files/neg/t7475e.check | 4 | ||||
-rw-r--r-- | test/files/neg/t7475e.scala | 12 | ||||
-rw-r--r-- | test/files/neg/t7475f.check | 10 | ||||
-rw-r--r-- | test/files/neg/t7475f.scala | 28 | ||||
-rw-r--r-- | test/files/neg/t7507.check | 2 | ||||
-rw-r--r-- | test/files/pos/t7475a.scala | 11 | ||||
-rw-r--r-- | test/files/pos/t7475b.scala | 8 | ||||
-rw-r--r-- | test/files/pos/t7475d.scala | 11 | ||||
-rw-r--r-- | test/files/pos/t7475e.scala | 13 | ||||
-rw-r--r-- | test/files/presentation/scope-completion-3.check | 28 | ||||
-rw-r--r-- | test/files/presentation/scope-completion-import.check | 16 | ||||
-rw-r--r-- | test/files/presentation/visibility.check | 3 | ||||
-rw-r--r-- | test/files/run/reflection-sorted-members.check | 1 | ||||
-rw-r--r-- | test/files/run/t7475b.check | 2 | ||||
-rw-r--r-- | test/files/run/t7475b.scala | 11 | ||||
-rw-r--r-- | test/files/run/t7507.scala | 4 |
21 files changed, 147 insertions, 47 deletions
diff --git a/test/files/neg/forgot-interpolator.check b/test/files/neg/forgot-interpolator.check index 157cbb4802..8988458982 100644 --- a/test/files/neg/forgot-interpolator.check +++ b/test/files/neg/forgot-interpolator.check @@ -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 ^ @@ -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 diff --git a/test/files/neg/forgot-interpolator.scala b/test/files/neg/forgot-interpolator.scala index 34a7c7aef4..a53054d890 100644 --- a/test/files/neg/forgot-interpolator.scala +++ b/test/files/neg/forgot-interpolator.scala @@ -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 diff --git a/test/files/neg/t7475c.check b/test/files/neg/t7475c.check new file mode 100644 index 0000000000..472808131a --- /dev/null +++ b/test/files/neg/t7475c.check @@ -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 diff --git a/test/files/neg/t7475c.scala b/test/files/neg/t7475c.scala new file mode 100644 index 0000000000..cd4a8762ca --- /dev/null +++ b/test/files/neg/t7475c.scala @@ -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? + } +} diff --git a/test/files/neg/t7475d.check b/test/files/neg/t7475d.check new file mode 100644 index 0000000000..6bd1da0d44 --- /dev/null +++ b/test/files/neg/t7475d.check @@ -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 diff --git a/test/files/neg/t7475e.check b/test/files/neg/t7475e.check new file mode 100644 index 0000000000..48af2be51a --- /dev/null +++ b/test/files/neg/t7475e.check @@ -0,0 +1,4 @@ +t7475e.scala:8: error: value priv is not a member of Base.this.TT + (??? : TT).priv + ^ +one error found diff --git a/test/files/neg/t7475e.scala b/test/files/neg/t7475e.scala new file mode 100644 index 0000000000..e5c4877d6e --- /dev/null +++ b/test/files/neg/t7475e.scala @@ -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 { +} diff --git a/test/files/neg/t7475f.check b/test/files/neg/t7475f.check new file mode 100644 index 0000000000..a07a4480e2 --- /dev/null +++ b/test/files/neg/t7475f.check @@ -0,0 +1,10 @@ +t7475f.scala:12: error: method c1 in class C cannot be accessed in C[T] + c1 // a member, but inaccessible. + ^ +t7475f.scala:13: error: not found: value c2 + c2 // a member, but inaccessible. + ^ +t7475f.scala:26: error: value d2 is not a member of D[Any] + other.d2 // not a member + ^ +three errors found diff --git a/test/files/neg/t7475f.scala b/test/files/neg/t7475f.scala new file mode 100644 index 0000000000..6c5feadf19 --- /dev/null +++ b/test/files/neg/t7475f.scala @@ -0,0 +1,28 @@ +class C[T] extends D[T] { + private def c1 = 0 + private[this] def c2 = 0 +} + +trait D[T] { + self: C[T] => + + private def d1 = 0 + private[this] def d2 = 0 + + c1 // a member, but inaccessible. + c2 // a member, but inaccessible. + + d1 // okay + d2 // okay + + + class C { + d1 + d2 + } + + def x(other: D[Any]) { + other.d1 + other.d2 // not a member + } +} diff --git a/test/files/neg/t7507.check b/test/files/neg/t7507.check index d402869fd4..de30fc7057 100644 --- a/test/files/neg/t7507.check +++ b/test/files/neg/t7507.check @@ -1,4 +1,4 @@ -t7507.scala:6: error: value bippy in trait Cake cannot be accessed in Cake +t7507.scala:6: error: not found: value bippy locally(bippy) ^ one error found diff --git a/test/files/pos/t7475a.scala b/test/files/pos/t7475a.scala new file mode 100644 index 0000000000..810ce9a05c --- /dev/null +++ b/test/files/pos/t7475a.scala @@ -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 = () +} diff --git a/test/files/pos/t7475b.scala b/test/files/pos/t7475b.scala new file mode 100644 index 0000000000..a34743b8be --- /dev/null +++ b/test/files/pos/t7475b.scala @@ -0,0 +1,8 @@ +trait U { +} + +trait T { + type TT = Any with T with U + private val priv = 0 + (??? : TT).priv +} diff --git a/test/files/pos/t7475d.scala b/test/files/pos/t7475d.scala new file mode 100644 index 0000000000..497c2bf443 --- /dev/null +++ b/test/files/pos/t7475d.scala @@ -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 +} diff --git a/test/files/pos/t7475e.scala b/test/files/pos/t7475e.scala new file mode 100644 index 0000000000..fbc965c4ca --- /dev/null +++ b/test/files/pos/t7475e.scala @@ -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 { + +} diff --git a/test/files/presentation/scope-completion-3.check b/test/files/presentation/scope-completion-3.check index df3007ab4e..b70a7d5c6b 100644 --- a/test/files/presentation/scope-completion-3.check +++ b/test/files/presentation/scope-completion-3.check @@ -3,19 +3,7 @@ reload: Completions.scala askScopeCompletion at Completions.scala(75,2) ================================================================================ [response] askScopeCompletion at (75,2) -retrieved 49 members -[inaccessible] private class Cb2 extends AnyRef -[inaccessible] private class Ct2 extends AnyRef -[inaccessible] private def fb2: Int -[inaccessible] private def ft2: Int -[inaccessible] private object Ob2 -[inaccessible] private object Ot2 -[inaccessible] private type tb2 = Completion1.this.tb2 -[inaccessible] private type tt2 = Completion1.this.tt2 -[inaccessible] private[this] val vb2: Int -[inaccessible] private[this] val vt2: Int -[inaccessible] private[this] var rb2: Int -[inaccessible] private[this] var rt2: Int +retrieved 37 members abstract class Base1 extends AnyRef abstract trait Trait1 extends AnyRef class Cb1 extends AnyRef @@ -58,19 +46,7 @@ type tt1 = Completion1.this.tt1 askScopeCompletion at Completions.scala(104,2) ================================================================================ [response] askScopeCompletion at (104,2) -retrieved 49 members -[inaccessible] private class Cb2 extends AnyRef -[inaccessible] private class Ct2 extends AnyRef -[inaccessible] private def fb2: Int -[inaccessible] private def ft2: Int -[inaccessible] private object Ob2 -[inaccessible] private object Ot2 -[inaccessible] private type tb2 = test.Completion2.tb2 -[inaccessible] private type tt2 = test.Completion2.tt2 -[inaccessible] private[this] val vb2: Int -[inaccessible] private[this] val vt2: Int -[inaccessible] private[this] var rb2: Int -[inaccessible] private[this] var rt2: Int +retrieved 37 members abstract class Base1 extends AnyRef abstract trait Trait1 extends AnyRef class Cb1 extends AnyRef diff --git a/test/files/presentation/scope-completion-import.check b/test/files/presentation/scope-completion-import.check index 220ffc399b..50197e5822 100644 --- a/test/files/presentation/scope-completion-import.check +++ b/test/files/presentation/scope-completion-import.check @@ -3,10 +3,8 @@ reload: Completions.scala askScopeCompletion at Completions.scala(23,4) ================================================================================ [response] askScopeCompletion at (23,4) -retrieved 18 members -[inaccessible] private[this] val pVCCC: Int +retrieved 16 members [inaccessible] private[this] val pVOOO: Int -[inaccessible] private[this] var pRCCC: Int [inaccessible] private[this] var pROOO: Int class C extends AnyRef class Foo extends AnyRef @@ -27,10 +25,8 @@ val o: test.O.type askScopeCompletion at Completions.scala(27,4) ================================================================================ [response] askScopeCompletion at (27,4) -retrieved 17 members -[inaccessible] private[this] val pVCCC: Int +retrieved 15 members [inaccessible] private[this] val pVOOO: Int -[inaccessible] private[this] var pRCCC: Int [inaccessible] private[this] var pROOO: Int class C extends AnyRef class Foo extends AnyRef @@ -126,10 +122,8 @@ val c: test.C askScopeCompletion at Completions.scala(49,4) ================================================================================ [response] askScopeCompletion at (49,4) -retrieved 18 members -[inaccessible] private[this] val pVCCC: Int +retrieved 16 members [inaccessible] private[this] val pVOOO: Int -[inaccessible] private[this] var pRCCC: Int [inaccessible] private[this] var pROOO: Int class C extends AnyRef class Foo extends AnyRef @@ -150,10 +144,8 @@ private[this] var rOOO: Int askScopeCompletion at Completions.scala(59,4) ================================================================================ [response] askScopeCompletion at (59,4) -retrieved 19 members -[inaccessible] private[this] val pVCCC: Int +retrieved 17 members [inaccessible] private[this] val pVOOO: Int -[inaccessible] private[this] var pRCCC: Int [inaccessible] private[this] var pROOO: Int class C extends AnyRef class Foo extends AnyRef diff --git a/test/files/presentation/visibility.check b/test/files/presentation/visibility.check index 1ae1213f2d..b77887f8f7 100644 --- a/test/files/presentation/visibility.check +++ b/test/files/presentation/visibility.check @@ -79,8 +79,7 @@ protected[package lang] def finalize(): Unit askTypeCompletion at Completions.scala(22,11) ================================================================================ [response] askTypeCompletion at (22,11) -retrieved 35 members -[inaccessible] private def secretPrivate(): Unit +retrieved 34 members def +(other: String): String def ->[B](y: B): (accessibility.AccessibilityChecks, B) def ensuring(cond: Boolean): accessibility.AccessibilityChecks diff --git a/test/files/run/reflection-sorted-members.check b/test/files/run/reflection-sorted-members.check index c148e19e69..415e073149 100644 --- a/test/files/run/reflection-sorted-members.check +++ b/test/files/run/reflection-sorted-members.check @@ -1,4 +1,3 @@ value a value b value c -value x diff --git a/test/files/run/t7475b.check b/test/files/run/t7475b.check new file mode 100644 index 0000000000..51993f072d --- /dev/null +++ b/test/files/run/t7475b.check @@ -0,0 +1,2 @@ +2 +2 diff --git a/test/files/run/t7475b.scala b/test/files/run/t7475b.scala new file mode 100644 index 0000000000..a205602b6d --- /dev/null +++ b/test/files/run/t7475b.scala @@ -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 { } + } +} diff --git a/test/files/run/t7507.scala b/test/files/run/t7507.scala index 6c1959ddac..a5eab6248f 100644 --- a/test/files/run/t7507.scala +++ b/test/files/run/t7507.scala @@ -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: |