From 6da4d4aa6a95434e2d95c083c6870e2b82596cdf Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Wed, 8 Nov 2023 15:51:59 +0100 Subject: [PATCH] Align unpickled Scala 2 accessors encoding with Scala 3 Adapt the flags of getters so they become like vals/vars instead. The getters flags and info are modified. The private fields for case accessors are not unpickled anymore. We need these getters to be aligned with Scala 3 if we want to be able to use the Scala 2 library TASTy. Otherwise library classfiles would not behave in the same way as their TASTy counterpart. This change may cause some macros to fail if they relied on the old style accessors. This should be adapted to work with the old and new representation. We observed that such a change is needed in `verify`, other might be detected with the open community build. --- .github/workflows/ci.yaml | 2 +- community-build/community-projects/verify | 2 +- .../dotc/core/unpickleScala2/Scala2Unpickler.scala | 14 +++++++++++++- .../dotty/tools/dotc/inlines/InlineReducer.scala | 2 +- .../tools/dotc/transform/PatternMatcher.scala | 2 +- .../src/dotty/tools/dotc/typer/Synthesizer.scala | 2 +- .../pc/tests/completion/CompletionDocSuite.scala | 2 +- .../pc/tests/completion/CompletionSuite.scala | 8 ++++---- .../pc/tests/tokens/SemanticTokensSuite.scala | 14 +++++++------- tests/neg/constructor-proxy-shadowing.check | 4 ++-- 10 files changed, 32 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 5f5f616b76da..1f556a290fc3 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -143,7 +143,7 @@ jobs: ./project/scripts/sbt ";sjsSandbox/run ;sjsSandbox/test ;sjsJUnitTests/test ;set sjsJUnitTests/scalaJSLinkerConfig ~= switchToESModules ;sjsJUnitTests/test ;sjsCompilerTests/test" - name: Test with Scala 2 library TASTy - run: ./project/scripts/sbt ";set ThisBuild/Build.useScala2LibraryTasty := true ;scala3-bootstrapped/testCompilation i5" # only test a subset of test to avoid doubling the CI execution time + run: ./project/scripts/sbt ";set ThisBuild/Build.useScala2LibraryTasty := true ;scala3-bootstrapped/testCompilation i5; scala3-bootstrapped/testCompilation tests/run/typelevel-peano.scala" # only test a subset of test to avoid doubling the CI execution time test_windows_fast: runs-on: [self-hosted, Windows] diff --git a/community-build/community-projects/verify b/community-build/community-projects/verify index 073921a373e0..ae37d7e153fc 160000 --- a/community-build/community-projects/verify +++ b/community-build/community-projects/verify @@ -1 +1 @@ -Subproject commit 073921a373e05bcfdc863769f676089ab889a002 +Subproject commit ae37d7e153fc62d64c40a72c45f810511aef2e01 diff --git a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala index fe68d22f779a..8687a713ba2c 100644 --- a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala @@ -450,10 +450,17 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas // Scala 2 sometimes pickle the same type parameter symbol multiple times // (see i11173 for an example), but we should only unpickle it once. || tag == TYPEsym && flags.is(TypeParam) && symScope(owner).lookup(name.asTypeName).exists + // We discard the private val representing a case accessor. We only load the case accessor def. + || flags.isAllOf(CaseAccessor| PrivateLocal, butNot = Method) then // skip this member return NoSymbol + // Adapt the flags of getters so they become like vals/vars instead + if flags.isAllOf(Method | Accessor) && !name.toString().endsWith("_$eq") then + flags &~= Method | Accessor + if !flags.is(StableRealizable) then flags |= Mutable + name = name.adjustIfModuleClass(flags) if (flags.is(Method)) name = @@ -618,7 +625,12 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas setClassInfo(denot, tp, fromScala2 = true, selfInfo) NamerOps.addConstructorProxies(denot.classSymbol) case denot => - val tp1 = translateTempPoly(tp) + val tp0 = translateTempPoly(tp) + val tp1 = + if !denot.is(Param) && !denot.is(Method) && tp0.isInstanceOf[ExprType] then + tp0.asInstanceOf[ExprType].resultType + else tp0 + denot.info = if (tag == ALIASsym) TypeAlias(tp1) else if (denot.isType) checkNonCyclic(denot.symbol, tp1, reportErrors = false) diff --git a/compiler/src/dotty/tools/dotc/inlines/InlineReducer.scala b/compiler/src/dotty/tools/dotc/inlines/InlineReducer.scala index 2dbfd9117f48..029616502c8c 100644 --- a/compiler/src/dotty/tools/dotc/inlines/InlineReducer.scala +++ b/compiler/src/dotty/tools/dotc/inlines/InlineReducer.scala @@ -329,7 +329,7 @@ class InlineReducer(inliner: Inliner)(using Context): val paramCls = paramType.classSymbol if (paramCls.is(Case) && unapp.symbol.is(Synthetic) && scrut <:< paramType) { val caseAccessors = - if (paramCls.is(Scala2x)) paramCls.caseAccessors.filter(_.is(Method)) + if paramCls.is(Scala2x) then paramCls.caseAccessors else paramCls.asClass.paramAccessors val selectors = for (accessor <- caseAccessors) diff --git a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala index 8f5eec693609..05ef16961698 100644 --- a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -327,7 +327,7 @@ object PatternMatcher { /** Plan for matching the result of an unapply against argument patterns `args` */ def unapplyPlan(unapp: Tree, args: List[Tree]): Plan = { def caseClass = unapp.symbol.owner.linkedClass - lazy val caseAccessors = caseClass.caseAccessors.filter(_.is(Method)) + lazy val caseAccessors = caseClass.caseAccessors def isSyntheticScala2Unapply(sym: Symbol) = sym.isAllOf(SyntheticCase) && sym.owner.is(Scala2x) diff --git a/compiler/src/dotty/tools/dotc/typer/Synthesizer.scala b/compiler/src/dotty/tools/dotc/typer/Synthesizer.scala index 6e1302c88398..ffb68a721fef 100644 --- a/compiler/src/dotty/tools/dotc/typer/Synthesizer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Synthesizer.scala @@ -409,7 +409,7 @@ class Synthesizer(typer: Typer)(using @constructorOnly c: Context): New(defn.RuntimeTupleMirrorTypeRef, Literal(Constant(arity)) :: Nil) def makeProductMirror(pre: Type, cls: Symbol, tps: Option[List[Type]]): TreeWithErrors = - val accessors = cls.caseAccessors.filterNot(_.isAllOf(PrivateLocal)) + val accessors = cls.caseAccessors val elemLabels = accessors.map(acc => ConstantType(Constant(acc.name.toString))) val typeElems = tps.getOrElse(accessors.map(mirroredType.resultType.memberInfo(_).widenExpr)) val nestedPairs = TypeOps.nestedPairs(typeElems) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionDocSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionDocSuite.scala index c419ce3946d9..6c5da76114a2 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionDocSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionDocSuite.scala @@ -214,7 +214,7 @@ class CompletionDocSuite extends BaseCompletionSuite: """.stripMargin, """ |> Found documentation for scala/package.Vector. - |Vector scala.collection.immutable + |Vector: scala.collection.immutable |""".stripMargin, includeDocs = true ) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala index 055363830a1b..b77404d4e7fc 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala @@ -25,7 +25,7 @@ class CompletionSuite extends BaseCompletionSuite: | Lis@@ |}""".stripMargin, """ - |List scala.collection.immutable + |List: scala.collection.immutable |List - java.awt |List - java.util |List - scala.collection.immutable @@ -647,7 +647,7 @@ class CompletionSuite extends BaseCompletionSuite: |} |""".stripMargin, """|None scala - |NoManifest scala.reflect + |NoManifest: scala.reflect |""".stripMargin, topLines = Some(2) ) @@ -660,8 +660,8 @@ class CompletionSuite extends BaseCompletionSuite: |} |""".stripMargin, """|Some(value) scala - |Seq scala.collection.immutable - |Set scala.collection.immutable + |Seq: scala.collection.immutable + |Set: scala.collection.immutable |""".stripMargin, topLines = Some(3) ) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/tokens/SemanticTokensSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/tokens/SemanticTokensSuite.scala index 9ef153e51da1..5d1fe1083b52 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/tokens/SemanticTokensSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/tokens/SemanticTokensSuite.scala @@ -194,7 +194,7 @@ class SemanticTokensSuite extends BaseSemanticTokensSuite: s"""|package <>/*namespace*/ | |object <>/*class*/ { - | val <>/*variable,definition,readonly*/ = <>/*class*/(1,2,3) + | val <>/*variable,definition,readonly*/ = <>/*variable,readonly*/(1,2,3) | val <>/*variable,definition,readonly*/ = <>/*class*/(1) | val <>/*class*/(<>/*variable,definition,readonly*/) = <>/*variable,readonly*/ | val <>/*class*/(<>/*variable,definition,readonly*/) = <>/*variable,readonly*/ @@ -269,7 +269,7 @@ class SemanticTokensSuite extends BaseSemanticTokensSuite: |object <>/*class*/ { | val <>/*variable,definition,readonly*/ = 1 | var <>/*variable,definition*/ = 2 - | val <>/*variable,definition,readonly*/ = <>/*class*/(1,<>/*variable,readonly*/,<>/*variable*/) + | val <>/*variable,definition,readonly*/ = <>/*variable,readonly*/(1,<>/*variable,readonly*/,<>/*variable*/) | <>/*variable*/ = <>/*variable,readonly*/ |""".stripMargin ) @@ -278,10 +278,10 @@ class SemanticTokensSuite extends BaseSemanticTokensSuite: check( """ |object <
>/*class*/ { - | val <>/*variable,definition,readonly*/ = <>/*class*/(1,2,3) - | val <>/*variable,definition,readonly*/ = <>/*class*/(1,2) - | val <>/*variable,definition,readonly*/ = <>/*class*/(1,2,3) - | val <>/*variable,definition,readonly*/ = <>/*class*/(1) + |val <>/*variable,definition,readonly*/ = <>/*variable,readonly*/(1,2,3) + |val <>/*variable,definition,readonly*/ = <>/*variable,readonly*/(1,2) + |val <>/*variable,definition,readonly*/ = <>/*variable,readonly*/(1,2,3) + |val <>/*variable,definition,readonly*/ = <>/*variable,readonly*/(1) |}""".stripMargin ) @@ -326,7 +326,7 @@ class SemanticTokensSuite extends BaseSemanticTokensSuite: | |object <>/*class*/ { | val <>/*variable,definition,readonly*/ = for { - | <>/*variable,definition,readonly*/ <- <>/*class*/("a", "b", "c") + | <>/*variable,definition,readonly*/ <- <>/*variable,readonly*/("a", "b", "c") | <<_>>/*class,abstract*/ = <>/*method*/("print!") | } yield <>/*variable,readonly*/ |} diff --git a/tests/neg/constructor-proxy-shadowing.check b/tests/neg/constructor-proxy-shadowing.check index f45b9b3205c3..091d1ed14c1e 100644 --- a/tests/neg/constructor-proxy-shadowing.check +++ b/tests/neg/constructor-proxy-shadowing.check @@ -52,7 +52,7 @@ 17 |val x = Seq(3) // error: shadowing | ^^^ | Reference to constructor proxy for class Seq - | shadows outer reference to getter Seq in package scala + | shadows outer reference to value Seq in package scala | | The instance needs to be created with an explicit `new`. |-------------------------------------------------------------------------------------------------------------------- @@ -66,7 +66,7 @@ | | new Seq(...) | - | Or it could mean calling the apply method of getter Seq in package scala as in + | Or it could mean calling the apply method of value Seq in package scala as in | | Seq.apply(...) |