Fix #8001: Erase polymorphic value classes like Scala 2

Previously given:

    class Poly[A](val value: A) extends AnyVal

We always erased `Poly[X]` to `Object`, no matter the value `X`, because
the erasure was the erased underlying type as seen from its definition,
and not as seen from the current prefix. But it turns out that in Scala
2, `Foo[Int]` will be erased to `Integer` instead (it would have made
more sense to use `int` but I suspect this is more accidental than
designed).

To be binary-compatible with Scala 2 and to support the same kind of
overloads we need to replicate its behavior, more precisely the rules I
was able to reverse-engineer are:

- Given `class Foo[A](x: A) extends AnyVal`, `Foo[X]` should erase like
  `X`, except if its a primitive in which case it erases to the boxed
  version of this primitive.
- Given `class Bar[A](x: Array[A]) extends AnyVal`, `Bar[X]` will be
  erased like `Array[A]` as seen from its definition site, no matter
  the `X` (same if `A` is bounded).

I was able to adapt our implementation of value class erasure to these
new rules without too much refactoring through one compromise: a value
class can no longer wrap another value class. This was never supported
by Scala 2 so we can afford to not support it either.

Author: smarter

compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala

@@ -918,7 +918,7 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
     // (one that doesn't erase to the actual signature). See run/t3452b for a test case.
 
     val memberTpe = atPhase(erasurePhase) { moduleClass.denot.thisType.memberInfo(sym) }
-    val erasedMemberType = TypeErasure.erasure(memberTpe)
+    val erasedMemberType = TypeErasure.fullErasure(memberTpe)
     if (erasedMemberType =:= sym.denot.info)
       getGenericSignatureHelper(sym, moduleClass, memberTpe).orNull
     else null

compiler/src/dotty/tools/dotc/core/TypeComparer.scala

@@ -341,7 +341,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
       case TypeErasure.ErasedValueType(tycon1, underlying2) =>
         def compareErasedValueType = tp1 match {
           case TypeErasure.ErasedValueType(tycon2, underlying1) =>
-            (tycon1.symbol eq tycon2.symbol) && isSameType(underlying1, underlying2)
+            (tycon1.symbol eq tycon2.symbol) && isSubType(underlying1, underlying2)
           case _ =>
             secondTry
         }

compiler/src/dotty/tools/dotc/core/TypeErasure.scala

@@ -36,7 +36,7 @@ import scala.annotation.tailrec
 object TypeErasure {
 
   private def erasureDependsOnArgs(sym: Symbol)(using Context) =
-    sym == defn.ArrayClass || sym == defn.PairClass
+    sym == defn.ArrayClass || sym == defn.PairClass || isDerivedValueClass(sym)
 
   def normalizeClass(cls: ClassSymbol)(using Context): ClassSymbol = {
     if (cls.owner == defn.ScalaPackageClass) {
@@ -59,7 +59,7 @@ object TypeErasure {
     case tp: TypeRef =>
       val sym = tp.symbol
       sym.isClass &&
-      !erasureDependsOnArgs(sym) &&
+      (!erasureDependsOnArgs(sym) || isDerivedValueClass(sym)) &&
       !defn.specialErasure.contains(sym) &&
       !defn.isSyntheticFunctionClass(sym)
     case _: TermRef =>
@@ -157,7 +157,7 @@ object TypeErasure {
 
   def sigName(tp: Type, isJava: Boolean)(using Context): TypeName = {
     val normTp = tp.translateFromRepeated(toArray = isJava)
-    val erase = erasureFn(isJava, semiEraseVCs = false, isConstructor = false, wildcardOK = true)
+    val erase = erasureFn(isJava, semiEraseVCs = true, isConstructor = false, wildcardOK = true)
     erase.sigName(normTp)(using preErasureCtx)
   }
 
@@ -444,14 +444,15 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
     case tp: TypeRef =>
       val sym = tp.symbol
       if (!sym.isClass) this(tp.translucentSuperType)
-      else if (semiEraseVCs && isDerivedValueClass(sym)) eraseDerivedValueClassRef(tp)
+      else if (semiEraseVCs && isDerivedValueClass(sym)) eraseDerivedValueClass(tp)
       else if (defn.isSyntheticFunctionClass(sym)) defn.erasedFunctionType(sym)
       else eraseNormalClassRef(tp)
     case tp: AppliedType =>
       val tycon = tp.tycon
       if (tycon.isRef(defn.ArrayClass)) eraseArray(tp)
       else if (tycon.isRef(defn.PairClass)) erasePair(tp)
       else if (tp.isRepeatedParam) apply(tp.translateFromRepeated(toArray = isJava))
+      else if (semiEraseVCs && isDerivedValueClass(tycon.classSymbol)) eraseDerivedValueClass(tp)
       else apply(tp.translucentSuperType)
     case _: TermRef | _: ThisType =>
       this(tp.widen)
@@ -551,12 +552,34 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
           case rt => MethodType(Nil, Nil, rt)
       case tp1 => this(tp1)
 
-  private def eraseDerivedValueClassRef(tref: TypeRef)(using Context): Type = {
-    val cls = tref.symbol.asClass
-    val underlying = underlyingOfValueClass(cls)
-    if underlying.exists && !isCyclic(cls) then
-      val erasedValue = valueErasure(underlying)
-      if erasedValue.exists then ErasedValueType(tref, erasedValue)
+  private def eraseDerivedValueClass(tp: Type)(using Context): Type = {
+    val cls = tp.classSymbol.asClass
+    val unbox = valueClassUnbox(cls)
+    if unbox.exists then
+      val genericUnderlying = unbox.info.resultType
+      val underlying = tp.select(unbox).widen.resultType
+
+      val erasedUnderlying = erasure(underlying)
+
+      // Ideally, we would just use `erasedUnderlying` as the erasure of `tp`, but to
+      // be binary-compatible with Scala 2 we need two special cases for polymorphic
+      // value classes:
+      // - Given `class Foo[A](x: A) extends AnyVal`, `Foo[X]` should erase like
+      //   `X`, except if its a primitive in which case it erases to the boxed
+      //   version of this primitive.
+      // - Given `class Bar[A](x: Array[A]) extends AnyVal`, `Bar[X]` will be
+      //   erased like `Array[A]` as seen from its definition site, no matter
+      //   the `X` (same if `A` is bounded).
+      //
+      // The binary compatibility is checked by sbt-dotty/sbt-test/scala2-compat/i8001
+      val erasedValueClass =
+        if erasedUnderlying.isPrimitiveValueType && !genericUnderlying.isPrimitiveValueType then
+          defn.boxedType(erasedUnderlying)
+        else if genericUnderlying.derivesFrom(defn.ArrayClass) then
+          erasure(genericUnderlying)
+        else erasedUnderlying
+
+      if erasedValueClass.exists then ErasedValueType(cls.typeRef, erasedValueClass)
       else
         assert(ctx.reporter.errorsReported, i"no erasure for $underlying")
         NoType
@@ -569,22 +592,23 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
   }
 
   /** The erasure of a function result type. */
-  private def eraseResult(tp: Type)(using Context): Type = tp match {
-    case tp: TypeRef =>
-      val sym = tp.symbol
-      if (sym eq defn.UnitClass) sym.typeRef
-      // For a value class V, "new V(x)" should have type V for type adaptation to work
-      // correctly (see SIP-15 and [[Erasure.Boxing.adaptToType]]), so the return type of a
-      // constructor method should not be semi-erased.
-      else if (isConstructor && isDerivedValueClass(sym)) eraseNormalClassRef(tp)
-      else this(tp)
-    case tp: AppliedType =>
-      val sym = tp.tycon.typeSymbol
-      if (sym.isClass && !erasureDependsOnArgs(sym)) eraseResult(tp.tycon)
-      else this(tp)
-    case _ =>
-      this(tp)
-  }
+  def eraseResult(tp: Type)(using Context): Type =
+    // For a value class V, "new V(x)" should have type V for type adaptation to work
+    // correctly (see SIP-15 and [[Erasure.Boxing.adaptToType]]), so the result type of a
+    // constructor method should not be semi-erased.
+    if semiEraseVCs && isConstructor && !tp.isInstanceOf[MethodOrPoly] then
+      erasureFn(isJava, semiEraseVCs = false, isConstructor, wildcardOK).eraseResult(tp)
+    else tp match
+      case tp: TypeRef =>
+        val sym = tp.symbol
+        if (sym eq defn.UnitClass) sym.typeRef
+        else this(tp)
+      case tp: AppliedType =>
+        val sym = tp.tycon.typeSymbol
+        if (sym.isClass && !erasureDependsOnArgs(sym)) eraseResult(tp.tycon)
+        else this(tp)
+      case _ =>
+        this(tp)
 
   /** The name of the type as it is used in `Signature`s.
    *  Need to ensure correspondence with erasure!
@@ -602,7 +626,7 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
           return sigName(info)
         }
         if (isDerivedValueClass(sym)) {
-          val erasedVCRef = eraseDerivedValueClassRef(tp)
+          val erasedVCRef = eraseDerivedValueClass(tp)
           if (erasedVCRef.exists) return sigName(erasedVCRef)
         }
         if (defn.isSyntheticFunctionClass(sym))

compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

@@ -84,7 +84,7 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
     ValueClassesMayNotContainInitalizationID,
     ValueClassesMayNotBeAbstractID,
     ValueClassesMayNotBeContaintedID,
-    ValueClassesMayNotWrapItselfID,
+    ValueClassesMayNotWrapAnotherValueClassID,
     ValueClassParameterMayNotBeAVarID,
     ValueClassNeedsExactlyOneValParamID,
     OnlyCaseClassOrCaseObjectAllowedID,

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -1617,9 +1617,9 @@ import transform.SymUtils._
     def explain = ""
   }
 
-  class ValueClassesMayNotWrapItself(valueClass: Symbol)(using Context)
-    extends SyntaxMsg(ValueClassesMayNotWrapItselfID) {
-    def msg = """A value class may not wrap itself"""
+  class ValueClassesMayNotWrapAnotherValueClass(valueClass: Symbol)(using Context)
+    extends SyntaxMsg(ValueClassesMayNotWrapAnotherValueClassID) {
+    def msg = """A value class may not wrap another user-defined value class"""
     def explain = ""
   }
 

compiler/src/dotty/tools/dotc/transform/Erasure.scala

@@ -302,7 +302,6 @@ object Erasure {
      *  in ExtensionMethods#transform.
      */
     def cast(tree: Tree, pt: Type)(using Context): Tree = trace(i"cast ${tree.tpe.widen} --> $pt", show = true) {
-
       def wrap(tycon: TypeRef) =
         ref(u2evt(tycon.typeSymbol.asClass)).appliedTo(tree)
       def unwrap(tycon: TypeRef) =
@@ -357,7 +356,10 @@ object Erasure {
           if (pt.isInstanceOf[ProtoType] || tree.tpe <:< pt)
             tree
           else if (tpw.isErasedValueType)
-            adaptToType(box(tree), pt)
+            if (pt.isErasedValueType) then
+              tree.asInstance(pt)
+            else
+              adaptToType(box(tree), pt)
           else if (pt.isErasedValueType)
             adaptToType(unbox(tree, pt), pt)
           else if (tpw.isPrimitiveValueType && !pt.isPrimitiveValueType)

compiler/src/dotty/tools/dotc/transform/FirstTransform.scala

@@ -18,6 +18,7 @@ import NameOps._
 import NameKinds.OuterSelectName
 import StdNames._
 import NullOpsDecorator._
+import TypeUtils.isErasedValueType
 
 object FirstTransform {
   val name: String = "firstTransform"
@@ -67,7 +68,7 @@ class FirstTransform extends MiniPhase with InfoTransformer { thisPhase =>
           qual.tpe
         }
         assert(
-          qualTpe.derivesFrom(tree.symbol.owner) ||
+          qualTpe.isErasedValueType || qualTpe.derivesFrom(tree.symbol.owner) ||
             tree.symbol.is(JavaStatic) && qualTpe.derivesFrom(tree.symbol.enclosingClass),
           i"non member selection of ${tree.symbol.showLocated} from ${qualTpe} in $tree")
       case _: TypeTree =>

compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala

@@ -212,12 +212,11 @@ object GenericSignatures {
             else if (sym == defn.UnitClass) jsig(defn.BoxedUnitClass.typeRef)
             else builder.append(defn.typeTag(sym.info))
           else if (ValueClasses.isDerivedValueClass(sym)) {
-            val unboxed     = ValueClasses.valueClassUnbox(sym.asClass).info.finalResultType
-            val unboxedSeen = tp.memberInfo(ValueClasses.valueClassUnbox(sym.asClass)).finalResultType
-            if (unboxedSeen.isPrimitiveValueType && !primitiveOK)
+            val erasedUnderlying = core.TypeErasure.fullErasure(tp)
+            if (erasedUnderlying.isPrimitiveValueType && !primitiveOK)
               classSig(sym, pre, args)
             else
-              jsig(unboxedSeen, toplevel, primitiveOK)
+              jsig(erasedUnderlying, toplevel, primitiveOK)
           }
           else if (defn.isSyntheticFunctionClass(sym)) {
             val erasedSym = defn.erasedFunctionClass(sym)

compiler/src/dotty/tools/dotc/transform/ValueClasses.scala

@@ -13,7 +13,7 @@ import SymUtils._
 /** Methods that apply to user-defined value classes */
 object ValueClasses {
 
-  def isDerivedValueClass(sym: Symbol)(using Context): Boolean = {
+  def isDerivedValueClass(sym: Symbol)(using Context): Boolean = sym.isClass && {
     val d = sym.denot
     !d.isRefinementClass &&
     d.isValueClass &&
@@ -54,15 +54,4 @@ object ValueClasses {
   /** The unboxed type that underlies a derived value class */
   def underlyingOfValueClass(sym: ClassSymbol)(using Context): Type =
     valueClassUnbox(sym).info.resultType
-
-  /** Whether a value class wraps itself */
-  def isCyclic(cls: ClassSymbol)(using Context): Boolean = {
-    def recur(seen: Set[Symbol], clazz: ClassSymbol)(using Context): Boolean =
-      (seen contains clazz) || {
-        val unboxed = underlyingOfValueClass(clazz).typeSymbol
-        (isDerivedValueClass(unboxed)) && recur(seen + clazz, unboxed.asClass)
-      }
-
-    recur(Set[Symbol](), cls)
-  }
 }

compiler/src/dotty/tools/dotc/typer/Checking.scala

@@ -610,8 +610,8 @@ object Checking {
         report.error(ValueClassesMayNotBeAbstract(clazz), clazz.srcPos)
       if (!clazz.isStatic)
         report.error(ValueClassesMayNotBeContainted(clazz), clazz.srcPos)
-      if (isCyclic(clazz.asClass))
-        report.error(ValueClassesMayNotWrapItself(clazz), clazz.srcPos)
+      if (isDerivedValueClass(underlyingOfValueClass(clazz.asClass).classSymbol))
+        report.error(ValueClassesMayNotWrapAnotherValueClass(clazz), clazz.srcPos)
       else {
         val clParamAccessors = clazz.asClass.paramAccessors.filter { param =>
           param.isTerm && !param.is(Flags.Accessor)

sbt-dotty/sbt-test/scala2-compat/i8001/build.sbt

@@ -0,0 +1,15 @@
+val scala3Version = sys.props("plugin.scalaVersion")
+val scala2Version = "2.13.4"
+
+lazy val lib = (project in file ("lib"))
+  .settings(scalaVersion := scala2Version)
+
+lazy val test = (project in file ("main"))
+  .dependsOn(lib)
+  .settings(
+    scalaVersion := scala3Version,
+    // https://github.com/sbt/sbt/issues/5369
+    projectDependencies := {
+      projectDependencies.value.map(_.withDottyCompat(scalaVersion.value))
+    }
+  )

sbt-dotty/sbt-test/scala2-compat/i8001/lib/lib.scala

@@ -0,0 +1,43 @@
+class Poly[A](val value: A) extends AnyVal
+
+class Arr[A](val value: Array[A]) extends AnyVal
+
+class ArrRef[A <: AnyRef](val value: Array[A]) extends AnyVal
+
+class A {
+  def poly1(x: Poly[Int]): Poly[Int] =
+    new Poly(x.value)
+
+  def poly2(x: Poly[String]): Poly[String] =
+    new Poly(x.value)
+
+  def poly3(x: Poly[Array[Int]]): Poly[Array[Int]] =
+    new Poly(x.value)
+
+  def poly4(x: Poly[Array[String]]): Poly[Array[String]] =
+    new Poly(x.value)
+
+  def arr1(x: Arr[Int]): Arr[Int] =
+    new Arr(x.value)
+
+  def arr2(x: Arr[String]): Arr[String] =
+    new Arr(x.value)
+
+  def arr3(x: Arr[Array[Int]]): Arr[Array[Int]] =
+    new Arr(x.value)
+
+  def arr4(x: Arr[Array[String]]): Arr[Array[String]] =
+    new Arr(x.value)
+
+  def arrRef1(x: ArrRef[Integer]): ArrRef[Integer] =
+    new ArrRef(x.value)
+
+  def arrRef2(x: ArrRef[String]): ArrRef[String] =
+    new ArrRef(x.value)
+
+  def arrRef3(x: ArrRef[Array[Int]]): ArrRef[Array[Int]] =
+    new ArrRef(x.value)
+
+  def arrRef4(x: ArrRef[Array[String]]): ArrRef[Array[String]] =
+    new ArrRef(x.value)
+}

sbt-dotty/sbt-test/scala2-compat/i8001/main/test.scala

@@ -0,0 +1,20 @@
+object B {
+  def main(args: Array[String]): Unit = {
+    val a = new A
+
+    a.poly1(new Poly(1))
+    a.poly2(new Poly(""))
+    a.poly3(new Poly(Array(1)))
+    a.poly4(new Poly(Array("")))
+
+    a.arr1(new Arr(Array(1)))
+    a.arr2(new Arr(Array("")))
+    a.arr3(new Arr(Array(Array(1))))
+    a.arr4(new Arr(Array(Array(""))))
+
+    a.arrRef1(new ArrRef(Array(1)))
+    a.arrRef2(new ArrRef(Array("")))
+    a.arrRef3(new ArrRef(Array(Array(1))))
+    a.arrRef4(new ArrRef(Array(Array(""))))
+  }
+}

sbt-dotty/sbt-test/scala2-compat/i8001/project/plugins.sbt

@@ -0,0 +1 @@
+addSbtPlugin("ch.epfl.lamp" % "sbt-dotty" % sys.props("plugin.version"))

sbt-dotty/sbt-test/scala2-compat/i8001/test

@@ -0,0 +1 @@
+> test/run

tests/neg/i1642.scala

@@ -1 +1,4 @@
-class Test2(val valueVal: Test2) extends AnyVal // error: value class cannot wrap itself
+class Test0(val valueVal: Test0) extends AnyVal // error: value class cannot wrap itself
+
+class Test1(val x: Int) extends AnyVal
+class Test2(val y: Test1) extends AnyVal // error: value class may not wrap another user-defined value class

tests/neg/i5005.scala

@@ -1,5 +1,5 @@
 
-case class i0 (i0: i1) extends AnyVal
+case class i0 (i0: i1) extends AnyVal // error
 trait i1 extends i0   // error
 
 trait F[x] extends AnyVal    // error