From 91d174fc75eeb030c243374f4db7a70e3396e359 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 19 Mar 2021 15:12:12 +0100 Subject: [PATCH] Array erasure: Better Java and Scala 2 compat We sometimes need to erase `Array[T]` to `Object` instead of `Array[erasure(T)]` in method signatures, this happens when both primitive and reference arrays, or two different sort of primitive arrays could be passed as arguments because the lub of those types on the JVM is `Object`. But before this commit, we additionally erased Arrays whose element type is upper-bounded by a universal trait to Object (like `Array[_ <: Serializable]`), this isn't necessary since primitives do not extend those traits (except for compiler fictions like `Singleton`) and derived value classes in arrays are always boxed. Since having matching Scala 2 and 3 erasure is a lost cause (cf #11603), this commit align ourselves with Java which improves Java interop (because we can emit more accurate Java generic signatures) and should let us simplify erasure more in the future. It also turns out than even before this commit, we did not match Scala 2 erasure perfectly since we erase `Array[_ <: Int]` to `Array[Int]` whereas Scala 2 erases to `Object` (this is important since we want `IArray[Int]` to be erased to `Array[Int]`), so we need to special case Scala 2 array erasure anyway to handle this. This commit renames `isUnboundedGeneric` to `isGenericArrayElement` since the former was somewhat misleading, `T <: String | Any` is bounded, but `Array[T]` cannot be represented with a specific JVM array type. --- .../dotty/tools/dotc/core/TypeErasure.scala | 87 ++++++++++++------- .../dotc/transform/GenericSignatures.scala | 4 +- .../tools/dotc/transform/TypeTestsCasts.scala | 2 +- .../scala2-compat/erasure/dottyApp/Api.scala | 42 +++++++++ .../scala2-compat/erasure/dottyApp/Main.scala | 31 ++++++- .../scala2-compat/erasure/scala2Lib/Api.scala | 33 +++++++ tests/run/array-erasure.scala | 24 +++++ tests/run/arrays-from-java/A_1.scala | 4 + tests/run/arrays-from-java/C_2.java | 21 +++++ tests/run/arrays-from-java/Test_2.scala | 4 + 10 files changed, 217 insertions(+), 35 deletions(-) create mode 100644 tests/run/arrays-from-java/A_1.scala create mode 100644 tests/run/arrays-from-java/C_2.java create mode 100644 tests/run/arrays-from-java/Test_2.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala index 4c43e72ec4b0..5ce82de63eed 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala @@ -269,42 +269,70 @@ object TypeErasure { } } - /** Underlying type that does not contain aliases or abstract types - * at top-level, treating opaque aliases as transparent. + /** Is `Array[tp]` a generic Array that needs to be erased to `Object`? + * This is true if among the subtypes of `Array[tp]` there is either: + * - both a reference array type and a primitive array type + * (e.g. `Array[_ <: Int | String]`, `Array[_ <: Any]`) + * - or two different primitive array types (e.g. `Array[_ <: Int | Double]`) + * In both cases the erased lub of those array types on the JVM is `Object`. + * + * In addition, if `isScala2` is true, we mimic the Scala 2 erasure rules and + * also return true for element types upper-bounded by a non-reference type + * such as in `Array[_ <: Int]` or `Array[_ <: UniversalTrait]`. */ - def classify(tp: Type)(using Context): Type = - if (tp.typeSymbol.isClass) tp - else tp match { - case tp: TypeProxy => classify(tp.translucentSuperType) - case tp: AndOrType => tp.derivedAndOrType(classify(tp.tp1), classify(tp.tp2)) - case _ => tp - } + def isGenericArrayElement(tp: Type, isScala2: Boolean)(using Context): Boolean = { + /** A symbol that represents the sort of JVM array that values of type `t` can be stored in: + * - If we can always store such values in a reference array, return Object + * - If we can always store them in a specific primitive array, return the + * corresponding primitive class + * - Otherwise, return `NoSymbol`. + */ + def arrayUpperBound(t: Type): Symbol = t.dealias match + case t: TypeRef if t.symbol.isClass => + val sym = t.symbol + // Only a few classes have both primitives and references as subclasses. + if (sym eq defn.AnyClass) || (sym eq defn.AnyValClass) || (sym eq defn.MatchableClass) || (sym eq defn.SingletonClass) + || isScala2 && !(t.derivesFrom(defn.ObjectClass) || t.isNullType) then + NoSymbol + // We only need to check for primitives because derived value classes in arrays are always boxed. + else if sym.isPrimitiveValueClass then + sym + else + defn.ObjectClass + case tp: TypeProxy => + arrayUpperBound(tp.translucentSuperType) + case tp: AndOrType => + val repr1 = arrayUpperBound(tp.tp1) + val repr2 = arrayUpperBound(tp.tp2) + if repr1 eq repr2 then + repr1 + else if tp.isAnd then + repr1.orElse(repr2) + else + NoSymbol + case _ => + NoSymbol - /** Is `tp` an abstract type or polymorphic type parameter that has `Any`, `AnyVal`, `Null`, - * or a universal trait as upper bound and that is not Java defined? Arrays of such types are - * erased to `Object` instead of `Object[]`. - */ - def isUnboundedGeneric(tp: Type)(using Context): Boolean = { - def isBoundedType(t: Type): Boolean = t match { - case t: OrType => isBoundedType(t.tp1) && isBoundedType(t.tp2) - case _ => t.derivesFrom(defn.ObjectClass) || t.isNullType - } + /** Can one of the JVM Array type store all possible values of type `t`? */ + def fitsInJVMArray(t: Type): Boolean = arrayUpperBound(t).exists tp.dealias match { case tp: TypeRef if !tp.symbol.isOpaqueAlias => !tp.symbol.isClass && - !isBoundedType(classify(tp)) && - !tp.symbol.is(JavaDefined) + !tp.symbol.is(JavaDefined) && // In Java code, Array[T] can never erase to Object + !fitsInJVMArray(tp) case tp: TypeParamRef => - !isBoundedType(classify(tp)) - case tp: TypeAlias => isUnboundedGeneric(tp.alias) + !fitsInJVMArray(tp) + case tp: TypeAlias => + isGenericArrayElement(tp.alias, isScala2) case tp: TypeBounds => - val upper = classify(tp.hi) - !isBoundedType(upper) && - !upper.isPrimitiveValueType - case tp: TypeProxy => isUnboundedGeneric(tp.translucentSuperType) - case tp: AndType => isUnboundedGeneric(tp.tp1) && isUnboundedGeneric(tp.tp2) - case tp: OrType => isUnboundedGeneric(tp.tp1) || isUnboundedGeneric(tp.tp2) + !fitsInJVMArray(tp.hi) + case tp: TypeProxy => + isGenericArrayElement(tp.translucentSuperType, isScala2) + case tp: AndType => + isGenericArrayElement(tp.tp1, isScala2) && isGenericArrayElement(tp.tp2, isScala2) + case tp: OrType => + isGenericArrayElement(tp.tp1, isScala2) || isGenericArrayElement(tp.tp2, isScala2) case _ => false } } @@ -642,8 +670,7 @@ class TypeErasure(sourceLanguage: SourceLanguage, semiEraseVCs: Boolean, isConst private def eraseArray(tp: Type)(using Context) = { val defn.ArrayOf(elemtp) = tp - if (classify(elemtp).derivesFrom(defn.NullClass)) JavaArrayType(defn.ObjectType) - else if (isUnboundedGeneric(elemtp) && !sourceLanguage.isJava) defn.ObjectType + if (isGenericArrayElement(elemtp, isScala2 = sourceLanguage.isScala2)) defn.ObjectType else JavaArrayType(erasureFn(sourceLanguage, semiEraseVCs = false, isConstructor, isSymbol, wildcardOK)(elemtp)) } diff --git a/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala b/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala index 9d48df564296..e2977a03e804 100644 --- a/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala +++ b/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala @@ -10,7 +10,7 @@ import core.Flags._ import core.Names.{DerivedName, Name, SimpleName, TypeName} import core.Symbols._ import core.TypeApplications.TypeParamInfo -import core.TypeErasure.{erasedGlb, erasure, isUnboundedGeneric} +import core.TypeErasure.{erasedGlb, erasure, isGenericArrayElement} import core.Types._ import core.classfile.ClassfileConstants import ast.Trees._ @@ -246,7 +246,7 @@ object GenericSignatures { typeParamSig(ref.paramName.lastPart) case defn.ArrayOf(elemtp) => - if (isUnboundedGeneric(elemtp)) + if (isGenericArrayElement(elemtp, isScala2 = false)) jsig(defn.ObjectType) else builder.append(ClassfileConstants.ARRAY_TAG) diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index 87342f3181a7..5209f71e5dd1 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -322,7 +322,7 @@ object TypeTestsCasts { transformTypeTest(e, tp1, flagUnrelated) .and(transformTypeTest(e, tp2, flagUnrelated)) } - case defn.MultiArrayOf(elem, ndims) if isUnboundedGeneric(elem) => + case defn.MultiArrayOf(elem, ndims) if isGenericArrayElement(elem, isScala2 = false) => def isArrayTest(arg: Tree) = ref(defn.runtimeMethodRef(nme.isArray)).appliedTo(arg, Literal(Constant(ndims))) if (ndims == 1) isArrayTest(expr) diff --git a/sbt-dotty/sbt-test/scala2-compat/erasure/dottyApp/Api.scala b/sbt-dotty/sbt-test/scala2-compat/erasure/dottyApp/Api.scala index ce4b8c4250ac..7ce908820390 100644 --- a/sbt-dotty/sbt-test/scala2-compat/erasure/dottyApp/Api.scala +++ b/sbt-dotty/sbt-test/scala2-compat/erasure/dottyApp/Api.scala @@ -8,6 +8,7 @@ trait B trait SubB extends B trait C trait Cov[+T] +trait Univ extends Any class D @@ -153,4 +154,45 @@ class Z { def int_61(x: Any with Int): Unit = {} def int_62(x: Int with AnyVal): Unit = {} def int_63(x: AnyVal with Int): Unit = {} + + def intARRAY_64(x: Array[Int with Singleton]): Unit = {} + def intARRAY_65(x: Array[_ <: Int]): Unit = {} + def intARRAY_66(x: Array[_ <: Int with Singleton]): Unit = {} + def intARRAY_67(x: Array[_ <: Singleton with Int]): Unit = {} + def intARRAY_68(x: Array[_ <: Int with Any]): Unit = {} + def intARRAY_69(x: Array[_ <: Any with Int]): Unit = {} + def intARRAY_70(x: Array[_ <: Int with AnyVal]): Unit = {} + def intARRAY_71(x: Array[_ <: AnyVal with Int]): Unit = {} + def intARRAY_71a(x: Array[_ <: Int | Int]): Unit = {} + def intARRAY_71b(x: Array[_ <: 1 | 2]): Unit = {} + + def stringARRAY_72(x: Array[String with Singleton]): Unit = {} + def stringARRAY_73(x: Array[_ <: String]): Unit = {} + def stringARRAY_74(x: Array[_ <: String with Singleton]): Unit = {} + def stringARRAY_75(x: Array[_ <: Singleton with String]): Unit = {} + def stringARRAY_76(x: Array[_ <: String with Any]): Unit = {} + def stringARRAY_77(x: Array[_ <: Any with String]): Unit = {} + def stringARRAY_78(x: Array[_ <: String with AnyRef]): Unit = {} + def stringARRAY_79(x: Array[_ <: AnyRef with String]): Unit = {} + def stringARRAY_79a(x: Array[_ <: String | String]): Unit = {} + def stringARRAY_79b(x: Array[_ <: "a" | "b"]): Unit = {} + + def object_80(x: Array[_ <: Singleton]): Unit = {} + def object_81(x: Array[_ <: AnyVal]): Unit = {} + def objectARRAY_82(x: Array[_ <: AnyRef]): Unit = {} + def object_83(x: Array[_ <: Any]): Unit = {} + def object_83a(x: Array[_ <: Matchable]): Unit = {} + def object_83b(x: Array[_ <: Int | Double]): Unit = {} + def object_83c(x: Array[_ <: String | Int]): Unit = {} + def object_83d(x: Array[_ <: Int | Matchable]): Unit = {} + def object_83e(x: Array[_ <: AnyRef | AnyVal]): Unit = {} + + def serializableARRAY_84(x: Array[_ <: Serializable]): Unit = {} + def univARRAY_85(x: Array[_ <: Univ]): Unit = {} + def aARRAY_86(x: Array[_ <: A]): Unit = {} + def aARRAY_87(x: Array[_ <: A with B]): Unit = {} + + def objectARRAY_88(x: Array[Any]): Unit = {} + def objectARRAY_89(x: Array[AnyRef]): Unit = {} + def objectARRAY_90(x: Array[AnyVal]): Unit = {} } diff --git a/sbt-dotty/sbt-test/scala2-compat/erasure/dottyApp/Main.scala b/sbt-dotty/sbt-test/scala2-compat/erasure/dottyApp/Main.scala index 7cc15cdd845b..be2c6c737316 100644 --- a/sbt-dotty/sbt-test/scala2-compat/erasure/dottyApp/Main.scala +++ b/sbt-dotty/sbt-test/scala2-compat/erasure/dottyApp/Main.scala @@ -74,13 +74,40 @@ object Main { z.int_61(1) z.int_62(1) z.int_63(1) + z.intARRAY_64(dummy) + z.object_65(dummy) + z.object_66(dummy) + z.object_67(dummy) + z.object_68(dummy) + z.object_69(dummy) + z.object_70(dummy) + z.object_71(dummy) + z.stringARRAY_72(dummy) + z.stringARRAY_73(dummy) + z.stringARRAY_74(dummy) + z.stringARRAY_75(dummy) + z.stringARRAY_76(dummy) + z.stringARRAY_77(dummy) + z.stringARRAY_78(dummy) + z.stringARRAY_79(dummy) + z.object_80(dummy) + z.object_81(dummy) + z.objectARRAY_82(dummy) + z.object_83(dummy) + z.object_84(dummy) + z.object_85(dummy) + z.aARRAY_86(dummy) + z.aARRAY_87(dummy) + z.objectARRAY_88(dummy) + z.objectARRAY_89(dummy) + z.objectARRAY_90(dummy) val methods = classOf[scala2Lib.Z].getDeclaredMethods.toList ++ classOf[dottyApp.Z].getDeclaredMethods.toList methods.foreach { m => m.getName match { case s"${prefix}_${suffix}" => - val paramClass = m.getParameterTypes()(0).getSimpleName - assert(prefix == paramClass.toLowerCase, s"Method `$m` erased to `$paramClass` which does not match its prefix `$prefix`") + val paramClass = m.getParameterTypes()(0).getSimpleName.toLowerCase.replaceAll("""\[\]""", "ARRAY") + assert(prefix == paramClass, s"Method `$m` erased to `$paramClass` which does not match its prefix `$prefix`") case _ => } } diff --git a/sbt-dotty/sbt-test/scala2-compat/erasure/scala2Lib/Api.scala b/sbt-dotty/sbt-test/scala2-compat/erasure/scala2Lib/Api.scala index c581879b32c2..2578f0556ecb 100644 --- a/sbt-dotty/sbt-test/scala2-compat/erasure/scala2Lib/Api.scala +++ b/sbt-dotty/sbt-test/scala2-compat/erasure/scala2Lib/Api.scala @@ -8,6 +8,7 @@ trait B trait SubB extends B trait C trait Cov[+T] +trait Univ extends Any class D @@ -153,4 +154,36 @@ class Z { def int_61(x: Any with Int): Unit = {} def int_62(x: Int with AnyVal): Unit = {} def int_63(x: AnyVal with Int): Unit = {} + + def intARRAY_64(x: Array[Int with Singleton]): Unit = {} + def object_65(x: Array[_ <: Int]): Unit = {} + def object_66(x: Array[_ <: Int with Singleton]): Unit = {} + def object_67(x: Array[_ <: Singleton with Int]): Unit = {} + def object_68(x: Array[_ <: Int with Any]): Unit = {} + def object_69(x: Array[_ <: Any with Int]): Unit = {} + def object_70(x: Array[_ <: Int with AnyVal]): Unit = {} + def object_71(x: Array[_ <: AnyVal with Int]): Unit = {} + + def stringARRAY_72(x: Array[String with Singleton]): Unit = {} + def stringARRAY_73(x: Array[_ <: String]): Unit = {} + def stringARRAY_74(x: Array[_ <: String with Singleton]): Unit = {} + def stringARRAY_75(x: Array[_ <: Singleton with String]): Unit = {} + def stringARRAY_76(x: Array[_ <: String with Any]): Unit = {} + def stringARRAY_77(x: Array[_ <: Any with String]): Unit = {} + def stringARRAY_78(x: Array[_ <: String with AnyRef]): Unit = {} + def stringARRAY_79(x: Array[_ <: AnyRef with String]): Unit = {} + + def object_80(x: Array[_ <: Singleton]): Unit = {} + def object_81(x: Array[_ <: AnyVal]): Unit = {} + def objectARRAY_82(x: Array[_ <: AnyRef]): Unit = {} + def object_83(x: Array[_ <: Any]): Unit = {} + + def object_84(x: Array[_ <: Serializable]): Unit = {} + def object_85(x: Array[_ <: Univ]): Unit = {} + def aARRAY_86(x: Array[_ <: A]): Unit = {} + def aARRAY_87(x: Array[_ <: A with B]): Unit = {} + + def objectARRAY_88(x: Array[Any]): Unit = {} + def objectARRAY_89(x: Array[AnyRef]): Unit = {} + def objectARRAY_90(x: Array[AnyVal]): Unit = {} } diff --git a/tests/run/array-erasure.scala b/tests/run/array-erasure.scala index ba5f9e12c39c..264fe46c36e5 100644 --- a/tests/run/array-erasure.scala +++ b/tests/run/array-erasure.scala @@ -35,11 +35,35 @@ object Test { } } + def arr3[T <: Int](x: Array[T]) = { + x(0) == 2 + x.sameElements(x) + } + + def arr4[T <: Int | Double](x: Array[T]) = { + x(0) == 2 + x.sameElements(x) + } + + def arr5[T <: Int | String](x: Array[T]) = { + x(0) == 2 + x.sameElements(x) + } + + def arr6[T <: Matchable](x: Array[T]) = { + x(0) == 2 + x.sameElements(x) + } + def main(args: Array[String]): Unit = { val x: Array[Int] = Array(0) arr0(x) arr1(x) arr2(x) + arr3(x) + arr4(x) + arr5(x) + arr6(x) } } diff --git a/tests/run/arrays-from-java/A_1.scala b/tests/run/arrays-from-java/A_1.scala new file mode 100644 index 000000000000..cc11122280a7 --- /dev/null +++ b/tests/run/arrays-from-java/A_1.scala @@ -0,0 +1,4 @@ +class A { + def foo1[T <: Serializable](x: Array[T]): Unit = {} + def foo2[T <: Object & Serializable](x: Array[T]): Unit = {} +} diff --git a/tests/run/arrays-from-java/C_2.java b/tests/run/arrays-from-java/C_2.java new file mode 100644 index 000000000000..a3573bdd3ac4 --- /dev/null +++ b/tests/run/arrays-from-java/C_2.java @@ -0,0 +1,21 @@ +import java.io.Serializable; + +class B extends A { + @Override + public void foo1(T[] x) {} + @Override + public void foo2(T[] x) {} +} + +public class C_2 { + public static void test() { + A a = new A(); + B b = new B(); + String[] arr = { "" }; + a.foo1(arr); + a.foo2(arr); + + b.foo1(arr); + b.foo2(arr); + } +} diff --git a/tests/run/arrays-from-java/Test_2.scala b/tests/run/arrays-from-java/Test_2.scala new file mode 100644 index 000000000000..c906f4fc0e8e --- /dev/null +++ b/tests/run/arrays-from-java/Test_2.scala @@ -0,0 +1,4 @@ +object Test { + def main(args: Array[String]): Unit = + C_2.test() +}