[SI-7475] Private member shadows inherited member Created: 12/May/13  Updated: 20/Mar/14  Resolved: 15/Feb/14

Status: CLOSED
Project: Scala Programming Language
Component/s: Compiler (Misc), Repl / Interpreter
Affects Version/s: Scala 2.10.1
Fix Version/s: Scala 2.11.0-RC1

Type: Bug Priority: Blocker
Reporter: Rob Norris Assignee: Jason Zaugg
Resolution: Fixed Votes: 0
Labels: access, has-pull-request

Issue Links:
Relates
relates to SI-6794 Can we please reach some kind of deci... Open
relates to SI-2568 qualified modifiers, private[C] and p... Open
relates to SI-261 private vals in traits depend on comp... CLOSED
relates to SI-261 private vals in traits depend on comp... CLOSED
relates to SI-8426 Private field in class cannot be acce... CLOSED
relates to SI-8432 Cannot access private members in sub-... CLOSED
relates to SI-7699 private members create mixin order de... CLOSED

 Description   

Depending on order of traits, private members can shadow inherited members and prevent compilation. This does not happen with separate compilation.

scala> :pa
// Entering paste mode (ctrl-D to finish)
 
trait A { private val x = 1 }
trait B { val x = 2 }
trait C extends B with A { println(x) }
 
// Exiting paste mode, now interpreting.
 
<console>:9: error: value x in trait A cannot be accessed in C
       trait C extends B with A { println(x) }
                                          ^
 
scala> :reset
Resetting interpreter state.
 
scala> trait A { private val x = 1 }
defined trait A
 
scala> trait B { val x = 2 }
defined trait B
 
scala> trait C extends B with A { println(x) }
defined trait C

Expected behavior (to me anyway) is that private members (which are not inherited) should not shadow inherited members when resolving symbols in subclasses. But in either case behavior should be consistent between combined and separate compilation.

Edit: damn this editor/formatter.



 Comments   
Comment by Paul Phillips [ 12/May/13 ]

The differing behavior is some sort of repl artifact. It doesn't compile together or separately outside the repl.

Here's a test. This should compile and when run print 2 twice.

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 { }
  }
}

Comment by Rob Norris [ 13/May/13 ]

Note the same problem occurs if A is a self-type.

trait C3 extends B { self: A => println(x) }

Not sure if this is surprising or not. Another data point.

Comment by Jason Zaugg [ 06/Jun/13 ]

The name lookup for x searches the base classes C and B.

In regular compilation (joint or separate); the member A::x is found, but is inaccessible, which leads into:

      def searchPrefix = {
        cx = cx.enclClass
        val found0 = lookupInPrefix(name)
        val found1 = found0 filter accessibleInPrefix
        if (found0.exists && !found1.exists && inaccessible == null)
          inaccessible = LookupInaccessible(found0, analyzer.lastAccessCheckDetails)
 
        found1
      }

In the resident compilation context (e.g. REPL with classes defined line-by-line), the name mangling of A::x is visible:

(baseClasses(1), baseClasses(1).info.decls.toList(1), baseClasses(1).info.decls.toList(1).name)
result = {scala.Tuple3@7304}"(trait A#45776,value x#45779,$line4$$read$$iw$$iw$A$$x)"

The lookup walks further up the base class sequence, finds B::x and happily compiles.

We're all to well aware of the proliferation of this sort of subtle bug in resident compilers; that's why we aren't supporting FSC anymore. But what do to with the beloved REPL. Could we rig it up to reload symbols from classfiles after they have compiled, rather than hanging onto the renmants of the later phases of the original run?

Back to the question of whether the error is valid or not under regular conditions. I hope not.

The private modifier can be used with any definition or declaration in a template. Such members can be accessed only from within the directly enclosing template and its companion module or companion class (§5.4). They are not inherited by subclasses and they may not override definitions in parent classes.

So the following is a bug, no? B should not have inherited `a`.

scala> class A {
     |   private val a: Int = 0
     |   class B extends A {
     |     def foo(a: A) = a.a // okay
     |     println(this.a)     // wait, what?
     |   }
     | }
defined class A

Comment by Jason Zaugg [ 18/Jun/13 ]

Here's a similar example distilled from Akka + JDK8 (which introduced an private field into PriorityBlockingQueue).

trait AbstractPublic {
  def queue: Any
}
trait ConcretePrivate {
  private val queue: Any = ()
}
 
abstract class Mix
  extends ConcretePrivate with AbstractPublic {
  final def queue: Any = ()
}

Comment by Jason Zaugg [ 19/Jun/13 ]

https://github.com/retronym/scala/compare/ticket/7475?expand=1

Comment by Jason Zaugg [ 25/Jun/13 ]

Martin noted that in the early days of findMember, it used to exclude private members beyond the first base class. This was removed in 88a54be3877. He can't remember why. Theories: 1) to make sure that private members are intersected in refinement types (see example below that doesn't compile under my patch), or 2) to make sure private members are visible in classes with a self type.

trait T {
  type TT = T with Any
  private val priv = 0
  (??? : TT).priv // we currently allow this, and should continue to do so.
}

Collateral work: I just noticed that findMember is duplicated with findMembers. It was an attempted performance optimization: 04f0b659efe8. Needless to say, they have started to drift apart.

Comment by Paul Phillips [ 25/Jun/13 ]

If only it were needless to say. That copy-paste job is only a year old.

Comment by Paul Phillips [ 28/Jul/13 ]

See also recently reopened SI-2568, where it was clearly pointed out that private members were inherited in violation of the specification, and it was closed wontfix anyway.

Comment by Grzegorz Kossakowski [ 15/Oct/13 ]

Unassigning and rescheduling to M7 as previous deadline was missed.

Comment by A. P. Marki [ 19/Nov/13 ]

I haven't seen SI-261 mentioned as an early duplicate, though bugs, like features, evolve over time.

Comment by Jason Zaugg [ 16/Jan/14 ]

Another one that shouldn't be allowed, but it:

class A {
  private val foo = 1
  (new B).foo // refchecks, but should not
}
 
class B extends A

Comment by Jason Zaugg [ 29/Jan/14 ]

https://github.com/scala/scala/pull/3440

Comment by Jason Zaugg [ 14/Feb/14 ]

followup discussion regarding the interaction of this change with pattern typing: https://groups.google.com/forum/#!topic/scala-internals/h8ztMqw-3Xg

Comment by Adriaan Moors [ 14/Feb/14 ]

Optimization == evil.root. I'm happy to submit a PR to remove it from inferTypedPattern and just use intersectionType.

Comment by Jason Zaugg [ 14/Feb/14 ]

I just checked and it does leak into error messages.

% git diff
diff --git a/test/files/pos/t7475f.scala b/test/files/pos/t7475f.scala
index 45a4000..275d997 100644
--- a/test/files/pos/t7475f.scala
+++ b/test/files/pos/t7475f.scala
@@ -6,6 +6,7 @@ class C {
   private def foo: Any = 0
   this match {
     case d: D =>
+      d: String
       d.foo
   }
 }
 
% qbin/scalac test/files/pos/t7475f.scala
test/files/pos/t7475f.scala:9: error: type mismatch;
 found   : C with D
 required: String
      d: String

But that's less of a WTF than not being able to access the private member of the refined type of a binder, so if we're after a simple change that's okay with me.

Comment by Adriaan Moors [ 14/Feb/14 ]

As always, there's another catch: test/files/run/t2755.scala CCEs if you don't eliminate subtypes… it's probably related to another issue with Array.
If we take the glb instead of the intersection, this problem doesn't arise.

Comment by Adriaan Moors [ 14/Feb/14 ]

We already do this for extractor patterns btw:

      def makeTypedUnApply() = {
        // the union of the expected type and the inferred type of the argument to unapply
        val glbType        = glb(ensureFullyDefined(pt) :: unapplyArg.tpe_* :: Nil)

Comment by Adriaan Moors [ 14/Feb/14 ]

well, duh – glb removes the upper type as well of course

Comment by Adriaan Moors [ 15/Feb/14 ]

Shooting at dusk: https://github.com/scala/scala/pull/3535

Comment by Adriaan Moors [ 15/Feb/14 ]

I think this issue can be closed, opening a new one for the pattern typing. That will have to wait until 2.12, unfortunately.

Generated at Fri Dec 14 03:46:17 CET 2018 using JIRA 7.9.1#79001-sha1:60970b42586a2ec2760ed6cfe825b26961e62b9e.