Align unpickled Scala 2 accessors encoding with Scala 3 (#18874)

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.
scala · Nov 16, 2023 · fdf8de3 · fdf8de3
2 parents 74791e0 + d04b3c7
commit fdf8de3
Show file tree

Hide file tree

Showing 23 changed files with 124 additions and 401 deletions.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -142,6 +142,9 @@ jobs:
         run: |
           ./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; 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]
     if: "(
@@ -209,10 +212,6 @@ jobs:
         run: sbt ";dist/pack ;scala3-bootstrapped/compile ;scala3-bootstrapped/test"
         shell: cmd
 
-      - name: Test with Scala 2 library TASTy
-        run: sbt ";set ThisBuild/Build.useScala2LibraryTasty := true ;scala3-bootstrapped/testCompilation i5" # only test a subset of test to avoid doubling the CI execution time
-        shell: cmd
-
       - name: Scala.js Test
         run: sbt ";sjsJUnitTests/test ;set sjsJUnitTests/scalaJSLinkerConfig ~= switchToESModules ;sjsJUnitTests/test ;sjsCompilerTests/test"
         shell: cmd

diff --git a/community-build/community-projects/verify b/community-build/community-projects/verify
diff --git a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala
@@ -450,10 +450,18 @@ 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.
+    // The info of this symbol is adapted in the `LocalUnpickler`.
+    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 +626,14 @@ 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 tp1 = translateTempPoly(tp) match
+              case ExprType(resultType) if !denot.isOneOf(Param | Method) =>
+                // Adapt the flags of getters so they become like vals/vars instead.
+                // This is the `def` of an accessor that needs to be transformed into
+                // a `val`/`var`. Note that the `Method | Accessor` flags were already
+                // striped away in `readDisambiguatedSymbol`.
+                resultType
+              case tp1 => tp1
             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
@@ -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
@@ -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
@@ -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/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala
@@ -39,7 +39,7 @@ class CompletionTest {
     code"class Foo { val foo: BigD${m1} }"
       .completion(
         ("BigDecimal", Field, "BigDecimal"),
-        ("BigDecimal", Method, "=> scala.math.BigDecimal.type"),
+        ("BigDecimal", Field, "scala.math.BigDecimal"),
       )
   }
 

diff --git 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
@@ -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
@@ -194,7 +194,7 @@ class SemanticTokensSuite extends BaseSemanticTokensSuite:
       s"""|package <<example>>/*namespace*/
           |
           |object <<A>>/*class*/ {
-          |  val <<x>>/*variable,definition,readonly*/ = <<List>>/*class*/(1,2,3)
+          |  val <<x>>/*variable,definition,readonly*/ = <<List>>/*variable,readonly*/(1,2,3)
           |  val <<s>>/*variable,definition,readonly*/ = <<Some>>/*class*/(1)
           |  val <<Some>>/*class*/(<<s1>>/*variable,definition,readonly*/) = <<s>>/*variable,readonly*/
           |  val <<Some>>/*class*/(<<s2>>/*variable,definition,readonly*/) = <<s>>/*variable,readonly*/
@@ -269,7 +269,7 @@ class SemanticTokensSuite extends BaseSemanticTokensSuite:
           |object <<A>>/*class*/ {
           |  val <<a>>/*variable,definition,readonly*/ = 1
           |  var <<b>>/*variable,definition*/ = 2
-          |  val <<c>>/*variable,definition,readonly*/ = <<List>>/*class*/(1,<<a>>/*variable,readonly*/,<<b>>/*variable*/)
+          |  val <<c>>/*variable,definition,readonly*/ = <<List>>/*variable,readonly*/(1,<<a>>/*variable,readonly*/,<<b>>/*variable*/)
           |  <<b>>/*variable*/ = <<a>>/*variable,readonly*/
           |""".stripMargin
     )
@@ -278,10 +278,10 @@ class SemanticTokensSuite extends BaseSemanticTokensSuite:
     check(
       """
         |object <<Main>>/*class*/ {
-        |  val <<a>>/*variable,definition,readonly*/ = <<List>>/*class*/(1,2,3)
-        |  val <<y>>/*variable,definition,readonly*/ = <<Vector>>/*class*/(1,2)
-        |  val <<z>>/*variable,definition,readonly*/ = <<Set>>/*class*/(1,2,3)
-        |  val <<w>>/*variable,definition,readonly*/ = <<Right>>/*class*/(1)
+        |val <<a>>/*variable,definition,readonly*/ = <<List>>/*variable,readonly*/(1,2,3)
+        |val <<y>>/*variable,definition,readonly*/ = <<Vector>>/*variable,readonly*/(1,2)
+        |val <<z>>/*variable,definition,readonly*/ = <<Set>>/*variable,readonly*/(1,2,3)
+        |val <<w>>/*variable,definition,readonly*/ = <<Right>>/*variable,readonly*/(1)
         |}""".stripMargin
     )
 
@@ -326,7 +326,7 @@ class SemanticTokensSuite extends BaseSemanticTokensSuite:
          |
          |object <<B>>/*class*/ {
          |  val <<a>>/*variable,definition,readonly*/ = for {
-         |    <<foo>>/*variable,definition,readonly*/ <- <<List>>/*class*/("a", "b", "c")
+         |    <<foo>>/*variable,definition,readonly*/ <- <<List>>/*variable,readonly*/("a", "b", "c")
          |    <<_>>/*class,abstract*/ = <<println>>/*method*/("print!")
          |  } yield <<foo>>/*variable,readonly*/
          |}

diff --git a/tests/coverage/pos/For.scoverage.check b/tests/coverage/pos/For.scoverage.check
@@ -262,23 +262,6 @@ covtest
 For$package$
 Object
 covtest.For$package$
-testForeach
-301
-304
-13
-Nil
-Ident
-false
-0
-false
-Nil
-
-15
-For.scala
-covtest
-For$package$
-Object
-covtest.For$package$
 $anonfun
 318
 343
@@ -290,7 +273,7 @@ false
 false
 println("user code here")
 
-16
+15
 For.scala
 covtest
 For$package$

diff --git a/tests/coverage/pos/Inlined.scoverage.check b/tests/coverage/pos/Inlined.scoverage.check
@@ -94,23 +94,6 @@ Object
 covtest.Inlined$package$
 testInlined
 155
-159
-6
-List
-Ident
-false
-0
-false
-List
-
-5
-Inlined.scala
-covtest
-Inlined$package$
-Object
-covtest.Inlined$package$
-testInlined
-155
 169
 6
 length
@@ -120,7 +103,7 @@ false
 false
 List(l).length
 
-6
+5
 Inlined.scala
 covtest
 Inlined$package$
@@ -137,7 +120,7 @@ false
 false
 scala.runtime.Scala3RunTime.assertFailed()
 
-7
+6
 Inlined.scala
 covtest
 Inlined$package$
@@ -154,7 +137,7 @@ true
 false
 scala.runtime.Scala3RunTime.assertFailed()
 
-8
+7
 Inlined.scala
 covtest
 Inlined$package$
@@ -171,7 +154,7 @@ true
 false
 
 
-9
+8
 Inlined.scala
 covtest
 Inlined$package$
@@ -188,24 +171,7 @@ false
 false
 List(l)
 
-10
-Inlined.scala
-covtest
-Inlined$package$
-Object
-covtest.Inlined$package$
-testInlined
-180
-184
-7
-List
-Ident
-false
-0
-false
-List
-
-11
+9
 Inlined.scala
 covtest
 Inlined$package$
@@ -222,7 +188,7 @@ false
 false
 List(l).length
 
-12
+10
 Inlined.scala
 covtest
 Inlined$package$
@@ -239,7 +205,7 @@ false
 false
 scala.runtime.Scala3RunTime.assertFailed()
 
-13
+11
 Inlined.scala
 covtest
 Inlined$package$
@@ -256,7 +222,7 @@ true
 false
 scala.runtime.Scala3RunTime.assertFailed()
 
-14
+12
 Inlined.scala
 covtest
 Inlined$package$
@@ -273,7 +239,7 @@ true
 false
 
 
-15
+13
 Inlined.scala
 covtest
 Inlined$package$