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

Non-stable lazy val pattern bindings #18878

Closed
nicolasstucki opened this issue Nov 8, 2023 · 9 comments
Closed

Non-stable lazy val pattern bindings #18878

nicolasstucki opened this issue Nov 8, 2023 · 9 comments
Assignees
Labels
area:desugar Desugaring happens after parsing but before typing, see desugar.scala itype:bug

Comments

@nicolasstucki
Copy link
Contributor

Compiler version

3.0 - 3.3

Minimized code

object Foo:
  lazy val (a, (b, c), d) = (1, (2, 3), 4)
  def a2: a.type = a

Output

3 |  def a2: a.type = a
  |          ^^^^^^
  |(Foo.a : => Int) is not a valid singleton type, since it is not an immutable path

Note that the syntax tree creates def for the bindings instead of lazy vals.

[[syntax trees at end of                     typer]] //
package <empty> {
  final lazy module val Foo: Foo = new Foo()
  final module class Foo() extends Object() { this: Foo.type =>
    private[this] lazy val $1$: (Int, Int, Int, Int) =
      Tuple3.apply[Int, (Int, Int), Int](1, Tuple2.apply[Int, Int](2, 3), 4):
        (Int, (Int, Int), Int) @unchecked match 
        {
          case 
            Tuple3.unapply[Int, (Int, Int), Int](a @ _,
              Tuple2.unapply[Int, Int](b @ _, c @ _), d @ _)
           => Tuple4.apply[Int, Int, Int, Int](a, b, c, d)
        }
    def a: Int = Foo.$1$._1
    def b: Int = Foo.$1$._2
    def c: Int = Foo.$1$._3
    def d: Int = Foo.$1$._4
    def a2: Foo.a.type = Foo.a
  }
}
Scala 2.13 desugaring
object Foo extends scala.AnyRef {
      def <init>(): Playground.Foo.type = {
        Foo.super.<init>();
        ()
      };
      <synthetic> <stable> <accessor> lazy <artifact> private[this] val x$1: (Int, Int, Int, Int) = (scala.Tuple3.apply[Int, (Int, Int), Int](1, scala.Tuple2.apply[Int, Int](2, 3), 4): (Int, (Int, Int), Int) @unchecked) match {
        case (_1: Int, _2: (Int, Int), _3: Int): (Int, (Int, Int), Int)((a @ _), (_1: Int, _2: Int): (Int, Int)((b @ _), (c @ _)), (d @ _)) => scala.Tuple4.apply[Int, Int, Int, Int](a, b, c, d)
      };
      <stable> <accessor> lazy val a: Int = Foo.this.x$1._1;
      <stable> <accessor> lazy val b: Int = Foo.this.x$1._2;
      <stable> <accessor> lazy val c: Int = Foo.this.x$1._3;
      <stable> <accessor> lazy val d: Int = Foo.this.x$1._4;
      def a2: Playground.Foo.a.type = Foo.this.a
    }
  };

Expectation

This should compile

@nicolasstucki nicolasstucki added itype:bug area:desugar Desugaring happens after parsing but before typing, see desugar.scala labels Nov 8, 2023
@nicolasstucki
Copy link
Contributor Author

It works wth vals

object Foo {
  val (a, (b, c), d) = (1, (2, 3), 4)
  def a2: a.type = a
}
[[syntax trees at end of                     typer]] //
package <empty> {
  final lazy module val Foo: Foo = new Foo()
  final module class Foo() extends Object() { this: Foo.type =>
    private[this] val $1$: (Int, Int, Int, Int) =
      Tuple3.apply[Int, (Int, Int), Int](1, Tuple2.apply[Int, Int](2, 3), 4):
        (Int, (Int, Int), Int) @unchecked match 
        {
          case 
            Tuple3.unapply[Int, (Int, Int), Int](a @ _,
              Tuple2.unapply[Int, Int](b @ _, c @ _), d @ _)
           => Tuple4.apply[Int, Int, Int, Int](a, b, c, d)
        }
    val a: Int = Foo.$1$._1
    val b: Int = Foo.$1$._2
    val c: Int = Foo.$1$._3
    val d: Int = Foo.$1$._4
    def a2: Foo.a.type = Foo.a
  }
}

@nicolasstucki
Copy link
Contributor Author

This issue was found when trying by using TASTy MiMa on the Scala 2 library TASTy.

https://github.com/lampepfl/dotty/pull/18877/files#diff-1fc2a36702124d57c13b45d49cabd0d061048f980c3f66e4e01b4d2d207e03d0R63-R68

@nicolasstucki nicolasstucki self-assigned this Nov 8, 2023
@nicolasstucki
Copy link
Contributor Author

This was behaviour was added in 54f6399.

Fix desugaring of lazy patterns.

Selectors should be defs, not lazy vals.

@odersky do you remember why these binding were considered broken at the time?

@nicolasstucki
Copy link
Contributor Author

Maybe the defs where used as an optimization to avoid the extra initialization of each individual lazy vals.

nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Nov 8, 2023
Fixes scala#18878

Reverts change in 54f6399

Example:
```
lazy val (a, b) = ...
```
desugars into
```
private lazy val t$1 = ...
lazy val a = t$1._1 // before: def a = t$1._1
lazy val b = t$1._2 // before: def b = t$1._2
```
Where `a` and `b` should be stable identifiers.
@sjrd
Copy link
Member

sjrd commented Nov 8, 2023

That seems expected to me. lazy vals are not stable to begin with, whether or not they are defined as patterns:

scala> class Foo { lazy val a = "foo"; def b: a.type = a }
-- Error: ----------------------------------------------------------------------
1 |class Foo { lazy val a = "foo"; def b: a.type = a }
  |                                       ^
  |                                 (Foo.this.a : String) is not a legal path
  |                                 since it refers to nonfinal lazy value a
1 error found

@nicolasstucki
Copy link
Contributor Author

I the case I showed I have an object, not a class. If we use an object in your example, it works.

object Bar { lazy val a = "foo"; def b: a.type = a }

@odersky
Copy link
Contributor

odersky commented Nov 8, 2023

As I wrote in my comment to #18879, maybe that should not work either.

@nicolasstucki
Copy link
Contributor Author

nicolasstucki commented Nov 8, 2023

Then I will only focus on what we should do in the Scala 2 library case. It seems I should apply the fix in #18879 on the when compiling. But maybe we could just leave it as it is if it does not cause incompatibilities in practice. I will investigate.

@nicolasstucki
Copy link
Contributor Author

The problematic case in the library is protected in a private[process]. It should be fine as it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:desugar Desugaring happens after parsing but before typing, see desugar.scala itype:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants