Skip to content

Commit 17a6080

Browse files
smartersmarter
committed
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.
1 parent dbc1186 commit 17a6080

File tree

22 files changed

+256
-68
lines changed

22 files changed

+256
-68
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -918,7 +918,7 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
918918
// (one that doesn't erase to the actual signature). See run/t3452b for a test case.
919919

920920
val memberTpe = atPhase(erasurePhase) { moduleClass.denot.thisType.memberInfo(sym) }
921-
val erasedMemberType = TypeErasure.erasure(memberTpe)
921+
val erasedMemberType = TypeErasure.fullErasure(memberTpe)
922922
if (erasedMemberType =:= sym.denot.info)
923923
getGenericSignatureHelper(sym, moduleClass, memberTpe).orNull
924924
else null

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
341341
case TypeErasure.ErasedValueType(tycon1, underlying2) =>
342342
def compareErasedValueType = tp1 match {
343343
case TypeErasure.ErasedValueType(tycon2, underlying1) =>
344-
(tycon1.symbol eq tycon2.symbol) && isSameType(underlying1, underlying2)
344+
(tycon1.symbol eq tycon2.symbol) && isSubType(underlying1, underlying2)
345345
case _ =>
346346
secondTry
347347
}

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

Lines changed: 51 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import scala.annotation.tailrec
3636
object TypeErasure {
3737

3838
private def erasureDependsOnArgs(sym: Symbol)(using Context) =
39-
sym == defn.ArrayClass || sym == defn.PairClass
39+
sym == defn.ArrayClass || sym == defn.PairClass || isDerivedValueClass(sym)
4040

4141
def normalizeClass(cls: ClassSymbol)(using Context): ClassSymbol = {
4242
if (cls.owner == defn.ScalaPackageClass) {
@@ -59,7 +59,7 @@ object TypeErasure {
5959
case tp: TypeRef =>
6060
val sym = tp.symbol
6161
sym.isClass &&
62-
!erasureDependsOnArgs(sym) &&
62+
(!erasureDependsOnArgs(sym) || isDerivedValueClass(sym)) &&
6363
!defn.specialErasure.contains(sym) &&
6464
!defn.isSyntheticFunctionClass(sym)
6565
case _: TermRef =>
@@ -157,7 +157,7 @@ object TypeErasure {
157157

158158
def sigName(tp: Type, isJava: Boolean)(using Context): TypeName = {
159159
val normTp = tp.translateFromRepeated(toArray = isJava)
160-
val erase = erasureFn(isJava, semiEraseVCs = false, isConstructor = false, wildcardOK = true)
160+
val erase = erasureFn(isJava, semiEraseVCs = true, isConstructor = false, wildcardOK = true)
161161
erase.sigName(normTp)(using preErasureCtx)
162162
}
163163

@@ -444,14 +444,15 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
444444
case tp: TypeRef =>
445445
val sym = tp.symbol
446446
if (!sym.isClass) this(tp.translucentSuperType)
447-
else if (semiEraseVCs && isDerivedValueClass(sym)) eraseDerivedValueClassRef(tp)
447+
else if (semiEraseVCs && isDerivedValueClass(sym)) eraseDerivedValueClass(tp)
448448
else if (defn.isSyntheticFunctionClass(sym)) defn.erasedFunctionType(sym)
449449
else eraseNormalClassRef(tp)
450450
case tp: AppliedType =>
451451
val tycon = tp.tycon
452452
if (tycon.isRef(defn.ArrayClass)) eraseArray(tp)
453453
else if (tycon.isRef(defn.PairClass)) erasePair(tp)
454454
else if (tp.isRepeatedParam) apply(tp.translateFromRepeated(toArray = isJava))
455+
else if (semiEraseVCs && isDerivedValueClass(tycon.classSymbol)) eraseDerivedValueClass(tp)
455456
else apply(tp.translucentSuperType)
456457
case _: TermRef | _: ThisType =>
457458
this(tp.widen)
@@ -551,12 +552,34 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
551552
case rt => MethodType(Nil, Nil, rt)
552553
case tp1 => this(tp1)
553554

554-
private def eraseDerivedValueClassRef(tref: TypeRef)(using Context): Type = {
555-
val cls = tref.symbol.asClass
556-
val underlying = underlyingOfValueClass(cls)
557-
if underlying.exists && !isCyclic(cls) then
558-
val erasedValue = valueErasure(underlying)
559-
if erasedValue.exists then ErasedValueType(tref, erasedValue)
555+
private def eraseDerivedValueClass(tp: Type)(using Context): Type = {
556+
val cls = tp.classSymbol.asClass
557+
val unbox = valueClassUnbox(cls)
558+
if unbox.exists then
559+
val genericUnderlying = unbox.info.resultType
560+
val underlying = tp.select(unbox).widen.resultType
561+
562+
val erasedUnderlying = erasure(underlying)
563+
564+
// Ideally, we would just use `erasedUnderlying` as the erasure of `tp`, but to
565+
// be binary-compatible with Scala 2 we need two special cases for polymorphic
566+
// value classes:
567+
// - Given `class Foo[A](x: A) extends AnyVal`, `Foo[X]` should erase like
568+
// `X`, except if its a primitive in which case it erases to the boxed
569+
// version of this primitive.
570+
// - Given `class Bar[A](x: Array[A]) extends AnyVal`, `Bar[X]` will be
571+
// erased like `Array[A]` as seen from its definition site, no matter
572+
// the `X` (same if `A` is bounded).
573+
//
574+
// The binary compatibility is checked by sbt-dotty/sbt-test/scala2-compat/i8001
575+
val erasedValueClass =
576+
if erasedUnderlying.isPrimitiveValueType && !genericUnderlying.isPrimitiveValueType then
577+
defn.boxedType(erasedUnderlying)
578+
else if genericUnderlying.derivesFrom(defn.ArrayClass) then
579+
erasure(genericUnderlying)
580+
else erasedUnderlying
581+
582+
if erasedValueClass.exists then ErasedValueType(cls.typeRef, erasedValueClass)
560583
else
561584
assert(ctx.reporter.errorsReported, i"no erasure for $underlying")
562585
NoType
@@ -569,22 +592,23 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
569592
}
570593

571594
/** The erasure of a function result type. */
572-
private def eraseResult(tp: Type)(using Context): Type = tp match {
573-
case tp: TypeRef =>
574-
val sym = tp.symbol
575-
if (sym eq defn.UnitClass) sym.typeRef
576-
// For a value class V, "new V(x)" should have type V for type adaptation to work
577-
// correctly (see SIP-15 and [[Erasure.Boxing.adaptToType]]), so the return type of a
578-
// constructor method should not be semi-erased.
579-
else if (isConstructor && isDerivedValueClass(sym)) eraseNormalClassRef(tp)
580-
else this(tp)
581-
case tp: AppliedType =>
582-
val sym = tp.tycon.typeSymbol
583-
if (sym.isClass && !erasureDependsOnArgs(sym)) eraseResult(tp.tycon)
584-
else this(tp)
585-
case _ =>
586-
this(tp)
587-
}
595+
def eraseResult(tp: Type)(using Context): Type =
596+
// For a value class V, "new V(x)" should have type V for type adaptation to work
597+
// correctly (see SIP-15 and [[Erasure.Boxing.adaptToType]]), so the result type of a
598+
// constructor method should not be semi-erased.
599+
if semiEraseVCs && isConstructor && !tp.isInstanceOf[MethodOrPoly] then
600+
erasureFn(isJava, semiEraseVCs = false, isConstructor, wildcardOK).eraseResult(tp)
601+
else tp match
602+
case tp: TypeRef =>
603+
val sym = tp.symbol
604+
if (sym eq defn.UnitClass) sym.typeRef
605+
else this(tp)
606+
case tp: AppliedType =>
607+
val sym = tp.tycon.typeSymbol
608+
if (sym.isClass && !erasureDependsOnArgs(sym)) eraseResult(tp.tycon)
609+
else this(tp)
610+
case _ =>
611+
this(tp)
588612

589613
/** The name of the type as it is used in `Signature`s.
590614
* Need to ensure correspondence with erasure!
@@ -602,7 +626,7 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
602626
return sigName(info)
603627
}
604628
if (isDerivedValueClass(sym)) {
605-
val erasedVCRef = eraseDerivedValueClassRef(tp)
629+
val erasedVCRef = eraseDerivedValueClass(tp)
606630
if (erasedVCRef.exists) return sigName(erasedVCRef)
607631
}
608632
if (defn.isSyntheticFunctionClass(sym))

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
8484
ValueClassesMayNotContainInitalizationID,
8585
ValueClassesMayNotBeAbstractID,
8686
ValueClassesMayNotBeContaintedID,
87-
ValueClassesMayNotWrapItselfID,
87+
ValueClassesMayNotWrapAnotherValueClassID,
8888
ValueClassParameterMayNotBeAVarID,
8989
ValueClassNeedsExactlyOneValParamID,
9090
OnlyCaseClassOrCaseObjectAllowedID,

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1617,9 +1617,9 @@ import transform.SymUtils._
16171617
def explain = ""
16181618
}
16191619

1620-
class ValueClassesMayNotWrapItself(valueClass: Symbol)(using Context)
1621-
extends SyntaxMsg(ValueClassesMayNotWrapItselfID) {
1622-
def msg = """A value class may not wrap itself"""
1620+
class ValueClassesMayNotWrapAnotherValueClass(valueClass: Symbol)(using Context)
1621+
extends SyntaxMsg(ValueClassesMayNotWrapAnotherValueClassID) {
1622+
def msg = """A value class may not wrap another user-defined value class"""
16231623
def explain = ""
16241624
}
16251625

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,6 @@ object Erasure {
302302
* in ExtensionMethods#transform.
303303
*/
304304
def cast(tree: Tree, pt: Type)(using Context): Tree = trace(i"cast ${tree.tpe.widen} --> $pt", show = true) {
305-
306305
def wrap(tycon: TypeRef) =
307306
ref(u2evt(tycon.typeSymbol.asClass)).appliedTo(tree)
308307
def unwrap(tycon: TypeRef) =
@@ -357,7 +356,10 @@ object Erasure {
357356
if (pt.isInstanceOf[ProtoType] || tree.tpe <:< pt)
358357
tree
359358
else if (tpw.isErasedValueType)
360-
adaptToType(box(tree), pt)
359+
if (pt.isErasedValueType) then
360+
tree.asInstance(pt)
361+
else
362+
adaptToType(box(tree), pt)
361363
else if (pt.isErasedValueType)
362364
adaptToType(unbox(tree, pt), pt)
363365
else if (tpw.isPrimitiveValueType && !pt.isPrimitiveValueType)

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import NameOps._
1818
import NameKinds.OuterSelectName
1919
import StdNames._
2020
import NullOpsDecorator._
21+
import TypeUtils.isErasedValueType
2122

2223
object FirstTransform {
2324
val name: String = "firstTransform"
@@ -67,7 +68,7 @@ class FirstTransform extends MiniPhase with InfoTransformer { thisPhase =>
6768
qual.tpe
6869
}
6970
assert(
70-
qualTpe.derivesFrom(tree.symbol.owner) ||
71+
qualTpe.isErasedValueType || qualTpe.derivesFrom(tree.symbol.owner) ||
7172
tree.symbol.is(JavaStatic) && qualTpe.derivesFrom(tree.symbol.enclosingClass),
7273
i"non member selection of ${tree.symbol.showLocated} from ${qualTpe} in $tree")
7374
case _: TypeTree =>

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,12 +212,11 @@ object GenericSignatures {
212212
else if (sym == defn.UnitClass) jsig(defn.BoxedUnitClass.typeRef)
213213
else builder.append(defn.typeTag(sym.info))
214214
else if (ValueClasses.isDerivedValueClass(sym)) {
215-
val unboxed = ValueClasses.valueClassUnbox(sym.asClass).info.finalResultType
216-
val unboxedSeen = tp.memberInfo(ValueClasses.valueClassUnbox(sym.asClass)).finalResultType
217-
if (unboxedSeen.isPrimitiveValueType && !primitiveOK)
215+
val erasedUnderlying = core.TypeErasure.fullErasure(tp)
216+
if (erasedUnderlying.isPrimitiveValueType && !primitiveOK)
218217
classSig(sym, pre, args)
219218
else
220-
jsig(unboxedSeen, toplevel, primitiveOK)
219+
jsig(erasedUnderlying, toplevel, primitiveOK)
221220
}
222221
else if (defn.isSyntheticFunctionClass(sym)) {
223222
val erasedSym = defn.erasedFunctionClass(sym)

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

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import SymUtils._
1313
/** Methods that apply to user-defined value classes */
1414
object ValueClasses {
1515

16-
def isDerivedValueClass(sym: Symbol)(using Context): Boolean = {
16+
def isDerivedValueClass(sym: Symbol)(using Context): Boolean = sym.isClass && {
1717
val d = sym.denot
1818
!d.isRefinementClass &&
1919
d.isValueClass &&
@@ -54,15 +54,4 @@ object ValueClasses {
5454
/** The unboxed type that underlies a derived value class */
5555
def underlyingOfValueClass(sym: ClassSymbol)(using Context): Type =
5656
valueClassUnbox(sym).info.resultType
57-
58-
/** Whether a value class wraps itself */
59-
def isCyclic(cls: ClassSymbol)(using Context): Boolean = {
60-
def recur(seen: Set[Symbol], clazz: ClassSymbol)(using Context): Boolean =
61-
(seen contains clazz) || {
62-
val unboxed = underlyingOfValueClass(clazz).typeSymbol
63-
(isDerivedValueClass(unboxed)) && recur(seen + clazz, unboxed.asClass)
64-
}
65-
66-
recur(Set[Symbol](), cls)
67-
}
6857
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -610,8 +610,8 @@ object Checking {
610610
report.error(ValueClassesMayNotBeAbstract(clazz), clazz.srcPos)
611611
if (!clazz.isStatic)
612612
report.error(ValueClassesMayNotBeContainted(clazz), clazz.srcPos)
613-
if (isCyclic(clazz.asClass))
614-
report.error(ValueClassesMayNotWrapItself(clazz), clazz.srcPos)
613+
if (isDerivedValueClass(underlyingOfValueClass(clazz.asClass).classSymbol))
614+
report.error(ValueClassesMayNotWrapAnotherValueClass(clazz), clazz.srcPos)
615615
else {
616616
val clParamAccessors = clazz.asClass.paramAccessors.filter { param =>
617617
param.isTerm && !param.is(Flags.Accessor)

0 commit comments

Comments
 (0)