Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The meaning of this.field in class body #15723

Closed
liufengyun opened this issue Jul 21, 2022 · 12 comments · Fixed by #15737
Closed

The meaning of this.field in class body #15723

liufengyun opened this issue Jul 21, 2022 · 12 comments · Fixed by #15737

Comments

@liufengyun
Copy link
Contributor

Compiler version

Scala 2 and Scala 3 (master)

Minimized code

class B(val y: Int):
  println(this.y)
  foo()
  def foo() = println(this.y)

class C(override val y: Int) extends B(10)

@main
def Test = new C(20)

Output

10
20

Expectation

I would expect this.y to have the same semantics both in the class body and method body.

It's up to discussion whether an unqualified y in class body and in method body should have the same semantics.

@liufengyun liufengyun added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Jul 21, 2022
@odersky
Copy link
Contributor

odersky commented Jul 21, 2022

Yes, there are two copies of y and inside the constructor body y refers to the local copy. Here's the code of the example after phase Constructors:

  @SourceFile("test.scala") class B extends Object {
    def <init>(y: Int): Unit = 
      {
        this.y = y
        super()
        println(scala.Int.box(y))
        this.foo()
        ()
      }
    private val y: Int
    def y(): Int = this.y
    def foo(): Unit = println(scala.Int.box(this.y()))
  }
  @SourceFile("test.scala") class C extends B {
    def <init>(y: Int): Unit = 
      {
        this.y = y
        super(10)
        ()
      }
    private val y: Int
    override def y(): Int = this.y
  }

This might indeed be surprising. On the other hand, Scala 2 does the same thing, so any change here would be an incompatible runtime change. Is it worth the cost of doing this?

@liufengyun
Copy link
Contributor Author

liufengyun commented Jul 21, 2022

On the other hand, Scala 2 does the same thing, so any change here would be an incompatible runtime change. Is it worth the cost of doing this?

My conjecture is that no real-world code depends on such subtle semantics. Otherwise, people would have complained about it for Scala 2. If a real-world program does depend on such subtle semantics, then it should be a bug in the program.

I can understand that some magic must happen for class parameters used in super constructor calls. However, if we can make the semantics more consistent in the class body, at least for qualified select like this.y, that seems to be better.

@smarter
Copy link
Member

smarter commented Jul 21, 2022

class C(override val y: Int) extends B(10)

Could we disallow overriding a class parameter unless the overridden val is passed to the supercall (as in B(y))?

@som-snytt
Copy link
Contributor

Don't laugh, but I was surprised we can reference a simple class param with this.x.

The spec calls the getter member introduced by val an alias of the parameter.

To my limited thinking, that class param is not a class member, but simply a constructor parameter, by analogy to ordinary use of method param x versus the member they shadow this.x.

There was an ancient ticket wondering why x wasn't subject to implicit conversion as though the user had written this.x, and this is one case where the distinction should actually matter.

@liufengyun
Copy link
Contributor Author

class C(override val y: Int) extends B(10)

Could we disallow overriding a class parameter unless the overridden val is passed to the supercall (as in B(y))?

I think such kind of restriction will be helpful. Is it possible to go one step further to disallow overridding class parameters --- if the values are required to be the same, then there is no need to override, just pass it to super class.

@szymon-rd szymon-rd added itype:enhancement area:transform and removed stat:needs triage Every issue needs to have an "area" and "itype" label itype:bug labels Jul 22, 2022
@odersky
Copy link
Contributor

odersky commented Jul 22, 2022

Can someone try this and evaluate what the breakage would be?

@som-snytt
Copy link
Contributor

There is a longstanding feature request for "pass-thru" parameters, to express class C(y: Int) extends B(y) without fear of capture in C, such as C(_) extends B(_). That is up the same alley or in the same ballpark or neck of the woods.

@smarter
Copy link
Member

smarter commented Jul 22, 2022

without fear of capture in C

That can be accomplised by writing @constructorOnly y: Int (https://github.com/lampepfl/dotty/blob/main/library/src/scala/annotation/constructorOnly.scala)

@som-snytt
Copy link
Contributor

without fear of capture shadows

@odersky
Copy link
Contributor

odersky commented Jul 22, 2022

I think as a first step, it's probably OK to treat this.y differently from y in class bodies. We must do that in the supercall in any case. I'm going to try that and see what happens.

@liufengyun
Copy link
Contributor Author

Now the usage of this.y in the constructor is consistent after #15737. For the usage of unqualified y, one thing we could do is issue a warning when unqualified y is used in the constructor but this.y is overridden in a subclass.

This check can be implemented in the initialization checker.

WDYT? /cc: @olhotak

@olhotak
Copy link
Contributor

olhotak commented Jul 26, 2022

Yes, I think we could add a warning for the unqualified y with another y in a subclass. It seems it's not really an initialization issue but the initialization checker is where we look at constructors of superclasses, so it would be a natural place to do the check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants