From 9383fd6e0c47d27d7deb5df27c8160e247bf30cb Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 22 Jul 2022 21:42:07 +0200 Subject: [PATCH 1/3] Fix treatment of parameter selections via this in constructors. Fixes #15723 --- .../tools/dotc/transform/Constructors.scala | 20 +++++++++++-------- tests/run/i15723.check | 2 ++ tests/run/i15723.scala | 10 ++++++++++ 3 files changed, 24 insertions(+), 8 deletions(-) create mode 100644 tests/run/i15723.check create mode 100644 tests/run/i15723.scala diff --git a/compiler/src/dotty/tools/dotc/transform/Constructors.scala b/compiler/src/dotty/tools/dotc/transform/Constructors.scala index d8cc76973233..d3dad820dfc2 100644 --- a/compiler/src/dotty/tools/dotc/transform/Constructors.scala +++ b/compiler/src/dotty/tools/dotc/transform/Constructors.scala @@ -149,17 +149,21 @@ class Constructors extends MiniPhase with IdentityDenotTransformer { thisPhase = // (2) If the parameter accessor reference was to an alias getter, // drop the () when replacing by the parameter. object intoConstr extends TreeMap { - private var isSuperCall = false + private var inSuperCall = false override def transform(tree: Tree)(using Context): Tree = tree match { case Ident(_) | Select(This(_), _) => var sym = tree.symbol - if sym.is(ParamAccessor) && (!sym.is(Mutable) || isSuperCall) - // Variables need to go through the getter since they might have been updated, - // except if we are in a super call, since then the virtual getter call would - // be illegal. - then + def isOverridableSelect = tree.isInstanceOf[Select] && !sym.isEffectivelyFinal + def keepUnlessInSuperCall = sym.is(Mutable) || isOverridableSelect + // If true, switch to constructor parameters only in the super call. + // Variables need to go through the getter since they might have been updated. + // References via this need to use the getter as well as long as that getter + // can be overriddem. This is needed to handle overrides correctly. See run/i15723.scala. + // But in a supercall we need to switch to parameters in any case since then + // calling the virtual getter would be illegal. + if sym.is(ParamAccessor) && (!keepUnlessInSuperCall || inSuperCall) then sym = sym.subst(accessors, paramSyms) - if (sym.maybeOwner.isConstructor) ref(sym).withSpan(tree.span) else tree + if sym.maybeOwner.isConstructor then ref(sym).withSpan(tree.span) else tree case Apply(fn, Nil) => val fn1 = transform(fn) if ((fn1 ne fn) && fn1.symbol.is(Param) && fn1.symbol.owner.isPrimaryConstructor) @@ -170,7 +174,7 @@ class Constructors extends MiniPhase with IdentityDenotTransformer { thisPhase = } def apply(tree: Tree, prevOwner: Symbol)(using Context): Tree = - isSuperCall = isSuperConstrCall(tree) + inSuperCall = isSuperConstrCall(tree) transform(tree).changeOwnerAfter(prevOwner, constr.symbol, thisPhase) } diff --git a/tests/run/i15723.check b/tests/run/i15723.check new file mode 100644 index 000000000000..b611ed20cf6c --- /dev/null +++ b/tests/run/i15723.check @@ -0,0 +1,2 @@ +20 +20 diff --git a/tests/run/i15723.scala b/tests/run/i15723.scala new file mode 100644 index 000000000000..2169857eace9 --- /dev/null +++ b/tests/run/i15723.scala @@ -0,0 +1,10 @@ +class B(val y: Int) { + println(this.y) + foo() + def foo() = println(this.y) +} +class C(override val y: Int) extends B(10) + +object Test extends App { + new C(20) +} \ No newline at end of file From 3d505964566e9f3653fbac8808470e38523f8c1d Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 23 Jul 2022 10:46:53 +0200 Subject: [PATCH 2/3] Fix comment --- .../tools/dotc/transform/Constructors.scala | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Constructors.scala b/compiler/src/dotty/tools/dotc/transform/Constructors.scala index d3dad820dfc2..59b90ff7f084 100644 --- a/compiler/src/dotty/tools/dotc/transform/Constructors.scala +++ b/compiler/src/dotty/tools/dotc/transform/Constructors.scala @@ -154,14 +154,19 @@ class Constructors extends MiniPhase with IdentityDenotTransformer { thisPhase = case Ident(_) | Select(This(_), _) => var sym = tree.symbol def isOverridableSelect = tree.isInstanceOf[Select] && !sym.isEffectivelyFinal - def keepUnlessInSuperCall = sym.is(Mutable) || isOverridableSelect - // If true, switch to constructor parameters only in the super call. + def switchOutsideSupercall = !sym.is(Mutable) && !isOverridableSelect + // If true, switch to constructor parameters also in the constructor body + // that follows the super call. // Variables need to go through the getter since they might have been updated. // References via this need to use the getter as well as long as that getter - // can be overriddem. This is needed to handle overrides correctly. See run/i15723.scala. - // But in a supercall we need to switch to parameters in any case since then - // calling the virtual getter would be illegal. - if sym.is(ParamAccessor) && (!keepUnlessInSuperCall || inSuperCall) then + // can be overridden. This is needed to handle overrides correctly. See run/i15723.scala. + // Note that in a supercall we need to switch to parameters in any case since then + // calling the virtual getter call would be illegal. + // + // Note: We intentionally treat references via this and identifiers differently + // here. Identifiers in a constructor always bind to the parameter. This is + // done for backwards compatbility. + if sym.is(ParamAccessor) && (switchOutsideSupercall || inSuperCall) then sym = sym.subst(accessors, paramSyms) if sym.maybeOwner.isConstructor then ref(sym).withSpan(tree.span) else tree case Apply(fn, Nil) => From 4710941134276278287e8767cd09f1965f733524 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 23 Jul 2022 10:47:26 +0200 Subject: [PATCH 3/3] Unrelated tests I piggyback these here to not go through the ceremony of another PR --- tests/pos/fewer-braces.scala | 12 ++++++++-- tests/run/patmat.check | 4 ++++ tests/run/patmat.scala | 45 ++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 tests/run/patmat.check create mode 100644 tests/run/patmat.scala diff --git a/tests/pos/fewer-braces.scala b/tests/pos/fewer-braces.scala index 28fd62def2eb..3fdebc068f58 100644 --- a/tests/pos/fewer-braces.scala +++ b/tests/pos/fewer-braces.scala @@ -1,5 +1,13 @@ import language.experimental.fewerBraces -object Test: +def Test = + + val xo: Option[Int] = Some(1) + + val y = + xo.fold: + 22 + .apply: x => + x + 1 + println(y) - assert((new Object: Any).isInstanceOf[Object]) \ No newline at end of file diff --git a/tests/run/patmat.check b/tests/run/patmat.check new file mode 100644 index 000000000000..c3b06e0f9d8a --- /dev/null +++ b/tests/run/patmat.check @@ -0,0 +1,4 @@ +Bob is 22 years old and lives in Paris +Hello Peter +Bob is 22 years old and lives in Paris +Hello PersonExtractor(Peter) diff --git a/tests/run/patmat.scala b/tests/run/patmat.scala new file mode 100644 index 000000000000..d12271bddebc --- /dev/null +++ b/tests/run/patmat.scala @@ -0,0 +1,45 @@ +object Test1: + class User(val name: String, val age: Int, val city: String) + object User: + def unapply(user: User) = UserExtractor(user.name, user.age, user.city) + + case class UserExtractor(name: String, age: Int, city: String) + + class Person(val name: String) + object Person: + def unapply(person: Person) = PersonExtractor(person.name) + + case class PersonExtractor(name: String) + + def test = + val user = User("Bob", 22, "Paris") + (user: Any) match + case User(name, age, city) => println(s"$name is $age years old and lives in $city") + val p = Person("Peter") + (p: Any) match + case Person(n) => println(s"Hello $n") + +object Test2: + class User(val name: String, val age: Int, val city: String) + object User: + def unapply(user: User): Option[UserExtractor] = Some(UserExtractor(user.name, user.age, user.city)) + + case class UserExtractor(name: String, age: Int, city: String) + + class Person(val name: String) + object Person: + def unapply(person: Person): Some[PersonExtractor] = Some(PersonExtractor(person.name)) + + case class PersonExtractor(name: String) + + def test = + val user = User("Bob", 22, "Paris") + (user: Any) match + case User(name, age, city) => println(s"$name is $age years old and lives in $city") + val p = Person("Peter") + (p: Any) match + case Person(n) => println(s"Hello $n") + +@main def Test = + Test1.test + Test2.test