private[this] fields: too much fly, not enough ointment

I think the bug here is that the private[this] y in B overrides the val y in A right? How can that be? Is it somehow not marked private?

Cheers

 -- Martin

On Sat, Oct 15, 2011 at 5:55 AM, Paul Phillips <paulp@improving.org> wrote:

I had forgotten about SI-4762, and not because it stopped being
horrifying.  Stumbling across it again, my horror is renewed.  Can we
please change something somewhere so that what one sees here is not
what one gets? It's going to be hard to say there isn't some horrible
bug somewhere given that I can elicit 75% of the possible permutations
of 1 and 99 altering nothing but type ascriptions.  And for good
measure I threw in a runtime failure.

There are no warnings at all issued when you compile the below.  That
much at least I can do something about.

// https://issues.scala-lang.org/browse/SI-4762

// In A, x and y are -1.
class A(val x: Int) {
 val y: Int = -1
}

// In B, x and y are 99 and private[this], implicitly so
// in the case of x.
class B(x: Int) extends A(-1) {
 private[this] def y: Int = 99

 // Three distinct results.
 def f = List(
   /* (99,99) */  (this.x, this.y),
   /* (-1,99) */  ((this: B).x, (this: B).y),
   /* (-1,-1) */  ((this: A).x, (this: A).y)
 )

 // The 99s tell us we are reading the private[this]
 // data of a different instance.
 def g(b: B) = List(
   /* (-1,99) */  (b.x, b.y),
   /* (-1,99) */  ((b: B).x, (b: B).y),
   /* (-1,-1) */  ((b: A).x, (b: A).y)
 )
}

object Test {
 def f(x: A)  = /* -2 */  x.x + x.y
 def g1(x: B) = /* -2 */  (x: A).x + (x: A).y
 def g2(x: B) = (x: B).x + (x: B).y
 // java.lang.IllegalAccessError: tried to access method B.y()I from
class Test$

 def main(args: Array[String]): Unit = {
   val b = new B(99)
   b.f foreach println
   b.g(new B(99)) foreach println

   println(f(b))
   println(g1(b))
   println(g2(b))
 }
}

--
Martin Odersky
Prof., EPFL and Chairman, Typesafe
PSED, 1015 Lausanne, Switzerland
Tel. EPFL: +41 21 693 6863
Tel. Typesafe: +41 21 691 4967

On Mon, Oct 17, 2011 at 12:38 PM, martin odersky wrote:
> I think the bug here is that the private[this] y in B overrides the val y in
> A right? How can that be? Is it somehow not marked private?

There has to be more than one bug, so I can't say what THE bug is. y
is marked private, but within B it is given precedence over the one in
A, whether it is addressed as "x" or as "this.x".

Here are some candidates for bugs.

1) This does not compile:

class A { val x: Int = 5 }
class B extends A { private val x: Int = 10 }
// ./a.scala:2: error: overriding value x in class A of type Int;
// value x has weaker access privileges; it should not be private
// class B extends A { private val x: Int = 10 }
// ^

But this does:

class A { val x: Int = 5 }
class B extends A { private[this] val x: Int = 10 }

I think I understand the thinking behind that, but if it's going to be
allowed, at least "this.x" needs to refer to the one in A. You can't
address a val with super, so the only way to get at the one in A is
(this: A).x. Which leads me to

2) Static type ascriptions affecting virtual method dispatch.

Things which are expected, or should be:

- type ascriptions influence overloading resolution
- type ascriptions influence implicit search

Things which are not, at least by me:

- type ascriptions influence the result of a not-overloaded selection
on either "this" or some other instance.

3) private[this] is not enforced.

We can see in the original example that one instance of B is reaching
into another and reading its private[this] field. If I make a simple
private[this] check it fails as expected, so it's not doing nothing,
it's just not completing its assigned mission.

4) There is no way to distinguish a constructor parameter from a
declared private[this] val.

I would not expect the same semantics from a field I have explicitly
declared and one which became a field via silent scalac magic because
I (intentionally or otherwise) referenced it from outside the
constructor. In particular, I *definitely* would not expect "this.x"
to ever refer to the auto-fieldized, private[this] constructor
parameter in the subclass as opposed to the public, explicitly
declared field in the superclass. But it does.

scala> class A(var x: Int)
defined class A

scala> class B(x: Int) extends A(x) { def y = this.x }
defined class B

scala> new B(10)
res0: B = B@434e54d8

scala> res0.x = 9123
res0.x: Int = 9123

scala> res0.x
res1: Int = 9123

scala> res0.y
res2: Int = 10

5) IllegalAccessError. Well, we know that one's a bug.

In summary: if you could address everything via "super.X" then this
situation might be salvageable. Since you can't address anything but
defs that way, I do not think it is. If "x", "this.x", "(this: B).x",
and "(this: A).x" are supposed to have four different semantics, I
would be interested to hear the rationale for that. Actually I'd like
to hear the rationale for any semantics except "all four are
equivalent" - that is the one I would vote for.

So how do you address private[this] x in a class which inherits x? You
*don't*. Name it something else. We issue a warning like the
"permanently hidden" warning - "private[this] field x is permanently
hidden by val x inherited from A" - and if they want to talk to that
field, give it another name. Plenty of names in the sea. It's the
vastly more common case where you want to use the same name to pass
the constructor parameter through - and definitely do not want to have
to hand name-mangle it - that we should be optimizing for.