Skip to content

Commit

Permalink
Align unpickled Scala 2 accessors encoding with Scala 3
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nicolasstucki committed Nov 10, 2023
1 parent 032f6cd commit 6da4d4a
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 20 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/inlines/InlineReducer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Synthesizer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -647,7 +647,7 @@ class CompletionSuite extends BaseCompletionSuite:
|}
|""".stripMargin,
"""|None scala
|NoManifest scala.reflect
|NoManifest: scala.reflect
|""".stripMargin,
topLines = Some(2)
)
Expand All @@ -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)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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*/
Expand Down Expand Up @@ -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
)
Expand All @@ -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
)

Expand Down Expand Up @@ -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*/
|}
Expand Down
4 changes: 2 additions & 2 deletions tests/neg/constructor-proxy-shadowing.check
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
|--------------------------------------------------------------------------------------------------------------------
Expand All @@ -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(...)
|
Expand Down

0 comments on commit 6da4d4a

Please sign in to comment.