Local fields are created unnecessarily instead of using existing ones in superclass

[smarter]

We shouldn't need to define a new local field in B here:

class A(val aaa: Int)
class B(val aaa: Int) extends A(aaa)

But we do in dotty:

package <empty> {
  class A extends Object { 
    def <init>(aaa: Int): A = {
      super()
      this.aaa$$local = aaa
      ()
    }
    private val aaa$$local: Int
    <accessor> def aaa(): Int = this.aaa$$local
  }
  final lazy module val A: A$ = new A$()
  final module class A$ extends Object { this: A$ => 
    def <init>(): A$ = {
      super()
      ()
    }
  }
  class B extends A { 
    def <init>(aaa: Int): B = {
      super(aaa)
      this.aaa$$local = aaa
      ()
    }
    private val aaa$$local: Int
    override <accessor> def aaa(): Int = this.aaa$$local
    def foo(): Int = B.this.aaa().+(1)
  }
  final lazy module val B: B$ = new B$()
  final module class B$ extends Object { this: B$ => 
    def <init>(): B$ = {
      super()
      ()
    }
  }
}

Compare with scalac:

package <empty> {
  class A extends Object {
    <paramaccessor> private[this] val aaa: Int = _;
    <stable> <accessor> <paramaccessor> def aaa(): Int = A.this.aaa;
    def <init>(aaa: Int): A = {
      A.this.aaa = aaa;
      A.super.<init>();
      ()
    }
  };
  class B extends A {
    override <stable> <accessor> <paramaccessor> def aaa(): Int = (B.super.aaa(): Int);
    def foo(): Int = B.this.aaa().+(1);
    def <init>(aaa: Int): B = {
      B.super.<init>(aaa);
      ()
    }
  }
}

[smarter]

The issue is in ParamForwarding:

if (sym is (PrivateLocalParamAccessor, butNot = Mutable)) {

Replacing PrivateLocalParamAccessor by ParamAccessor avoids the field creation. @odersky : Why would the field need to be private[this] for this transformation?

[retronym]

scalac is too willing to use the super param accessors, IMO. For example:

class A(val aaa: Int)
class B(y: Int) extends A(y) {
  def yy = y
}
class C extends B(0) {
  override val aaa = ???
}

object Test {
  def main(args: Array[String]): Unit = {
    new C().yy // throws!
  }
}

I've raised a ticket. @odersky Do you agree this is a bug?

[DarkDimius]

@retronym, in your example, shouldn't constructor throw? aaa field throws an exception during initialization.

[retronym]

Oh right. The behaviour I'm trying to show is in this case, a separate field is required in B. scalac is doing the right thing.

class A(val y: Int)
class B(y: Int) extends A(y) {
  def yy = y
}
class C extends B(0) {
  override val y = -1
}

object Test {
  def main(args: Array[String]): Unit = {
    println(new C().yy) // prints 0 as expected
  }
}

[retronym]

But, in this case, we print -1. Not sure anymore if this is wrong or right.

class A(val y: Int)
class B(override val y: Int) extends A(y) {
  def yy = y
}
class C extends B(0) {
  override val y = -1
}

object Test {
  def main(args: Array[String]): Unit = {
    println(new C().yy) // prints -1
  }
}

[odersky]

Looks like it is wrong. Which is really unfortunate, because avoid
parameter fields that get passed on to the super accessor is absolutely
crucial for memory footprint. Not clear what to do about this without a
global analysis. Anyone has a better idea?

• Martin

On Wed, May 27, 2015 at 10:23 AM, Jason Zaugg notifications@github.com
wrote:

But, in this case, we print -1. Not sure anymore if this is wrong or
right.

class A(val y: Int)class B(override val y: Int) extends A(y) {
def yy = y
}class C extends B(0) {
override val y = -1
}
object Test {
def main(args: Array[String]): Unit = {
println(new C().yy) // prints -1
}
}

—
Reply to this email directly or view it on GitHub
#608 (comment).

Martin Odersky
EPFL

[DarkDimius]

@odersky why is it wrong? C overrides y to be equal to -1.

[odersky]

Correction: Everything is fine. In fact, here is the disassembled code of B
in Jason's first example

class A(val y: Int)
class B(y: Int) extends A(y) {
def yy = y
}
class C extends B(0) {
override val y = -1
}

/Users/odersky/workspace/dotty/playground> javap -c -private B
Compiled from "aliasing.scala"
public class B extends A {
public int yy();
Code:
0: aload_0
1: invokespecial #13 // Method A.y:()I
4: ireturn

public B(int);
Code:
0: aload_0
1: iload_1
2: invokespecial #19 // Method A."":(I)V
5: return
}

Note the "invokespecial". That's crucial so that we get the getter of B,
not the overriding one of C. Here's the code generated by dotty, which does
the same thing but keeps the y accessor:

public class B extends A {
public B(int);
Code:
0: aload_0
1: iload_1
2: invokespecial #9 // Method A."":(I)V
5: return

private int y();
Code:
0: aload_0
1: invokespecial #16 // Method A.y:()I
4: ireturn

public int yy();
Code:
0: aload_0
1: invokespecial #18 // Method y:()I
4: ireturn
}

I agree that if we have a val in B everything changes and we should get a
-1.

On Wed, May 27, 2015 at 11:26 AM, Dmitry Petrashko <notifications@github.com

wrote:

@odersky https://github.com/odersky why is it wrong? C overrides y to
be equal to -1.

—
Reply to this email directly or view it on GitHub
#608 (comment).

Martin Odersky
EPFL

[DarkDimius]

@odersky, so in the example from original post

class A(val aaa: Int)
class B(val aaa: Int) extends A(aaa)

Should the field be shared?

[odersky]

Yes, it should be shared.

On Wed, May 27, 2015 at 1:34 PM, Dmitry Petrashko notifications@github.com
wrote:

@odersky https://github.com/odersky, so in the example from original
post

class A(val aaa: Int)
class B(val aaa: Int) extends A(aaa)

Should the field be shared?

—
Reply to this email directly or view it on GitHub
#608 (comment).

Martin Odersky
EPFL

[smarter]

I have a fix with an extensive test case at https://github.com/smarter/dotty/commits/fix/param-forwarding unfortunately the following currently fail:

class A(val aaa: Int)
class B(override val aaa: Int) extends A(aaa)

With:

try/superarg2.scala:2: error: overriding value aaa in class A of type Int;
 method aaa of type => Int needs to be a stable, immutable value
class B(override val aaa: Int) extends A(aaa)

The failure comes from RefChecks:

      } else if (other.isStable && !member.isStable) { // (1.4)
        overrideError("needs to be a stable, immutable value")

Because at the point where RefChecks is run, the tree looks like:

class A(aaa: Int) extends Object() {
  val aaa: Int
}
class B(aaa: Int) extends A(aaa) {
  override def aaa: Int = super.aaa
}

What would be the best way to avoid this? Should I try using @uncheckedStable?
EDIT: Fixed by marking the forwarder as Stable, seemed to be the best solution.