From c7fb0fc68214ec5c436135af2ec9ffa09a1f590d Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Mon, 13 Apr 2020 23:59:03 +0200 Subject: [PATCH 1/4] Fix TypeComparer.explained usage in TreeChecker --- compiler/src/dotty/tools/dotc/transform/TreeChecker.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 3dc3b3eba1cc..49d82102d2da 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -315,7 +315,7 @@ class TreeChecker extends Phase with SymTransformer { |Original tree : ${tree.show} |After checking: ${tree1.show} |Why different : - """.stripMargin + core.TypeComparer.explained((tp1 <:< tp2)(_)) + """.stripMargin + core.TypeComparer.explained(tp1 <:< tp2) if (tree.hasType) // it might not be typed because Typer sometimes constructs new untyped trees and resubmits them to typedUnadapted assert(isSubType(tree1.tpe, tree.typeOpt), divergenceMsg(tree1.tpe, tree.typeOpt)) tree1 From c80d6e428d30353600db066a4ee9a1ca831ca3f2 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 7 Apr 2020 22:41:51 +0200 Subject: [PATCH 2/4] Fix ClassCastException with Java primitive varargs The class constant corresponding to the primitive type `int` is represented in bytecode by `Integer.TYPE`, this is correctly handled when calling `clsOf`, but so far `ElimRepeated` was manually creating a constant instead which does not work (I've now changed the backend to emit a failure instead of doing the wrong thing when someone does that, I also plan to rework the way we handle classOf to make this less error-prone). Note: after this commit, tests/run/t1360.scala started failing, this is fixed in the next commit. --- .../dotty/tools/backend/jvm/BCodeBodyBuilder.scala | 11 ++++------- .../src/dotty/tools/dotc/transform/ElimRepeated.scala | 2 +- tests/run/java-varargs/A_1.java | 4 ++++ tests/run/java-varargs/Test_2.scala | 6 ++++++ 4 files changed, 15 insertions(+), 8 deletions(-) create mode 100644 tests/run/java-varargs/A_1.java create mode 100644 tests/run/java-varargs/Test_2.scala diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala index a269d9f7fcce..54c14db8cc54 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala @@ -483,13 +483,10 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { case NullTag => emit(asm.Opcodes.ACONST_NULL) case ClazzTag => - val toPush: BType = { - toTypeKind(const.typeValue) match { - case kind: PrimitiveBType => boxedClassOfPrimitive(kind) - case kind => kind - } - } - mnode.visitLdcInsn(toPush.toASMType) + val tp = toTypeKind(const.typeValue) + // classOf[Int] is transformed to Integer.TYPE by ClassOf + assert(!tp.isPrimitive, s"expected class type in classOf[T], found primitive type $tp") + mnode.visitLdcInsn(tp.toASMType) case EnumTag => val sym = const.symbolValue diff --git a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala index e4c75b04cc7e..e8b374f432dd 100644 --- a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala +++ b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala @@ -99,7 +99,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => ref(defn.DottyArraysModule) .select(nme.seqToArray) .appliedToType(elemType) - .appliedTo(tree, Literal(Constant(elemClass.typeRef))) + .appliedTo(tree, clsOf(elemClass.typeRef)) } /** Convert Java array argument to Scala Seq */ diff --git a/tests/run/java-varargs/A_1.java b/tests/run/java-varargs/A_1.java new file mode 100644 index 000000000000..46f82d07893e --- /dev/null +++ b/tests/run/java-varargs/A_1.java @@ -0,0 +1,4 @@ +class A_1 { + public static void foo(int... args) { + } +} diff --git a/tests/run/java-varargs/Test_2.scala b/tests/run/java-varargs/Test_2.scala new file mode 100644 index 000000000000..4ef2d5b001f7 --- /dev/null +++ b/tests/run/java-varargs/Test_2.scala @@ -0,0 +1,6 @@ +object Test { + def main(args: Array[String]): Unit = { + A_1.foo(Array(1): _*) + A_1.foo(Seq(1): _*) + } +} From dbb98b37d3eae024f7d4bb692945b1b5c61b5351 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 24 Mar 2020 18:52:44 +0100 Subject: [PATCH 3/4] Don't drop Object intersection for varargs A Java generic array `T[]` is unpickled in ClassfileParser to `Array[T & Object]`, because it will not accept primitive arrays. However, Java varargs were special-cased to drop this intersection. Erasure contained some code to compensate for this, but this is still unsound and lead to tests/run/t1360.scala failing with a ClassCastException. Thanks to #8669 we can drop this special-casing of varargs without affecting typing too much: it's still possible to pass `Array(1, 2): _*` where `(T & Object)*` is expected because adaptation will take care of boxing. One test case had to be adapted, I replaced: def apply[X](xs : X*): java.util.List[X] java.util.Arrays.asList(xs: _*) with: def apply[X <: AnyRef](xs : X*): java.util.List[X] = java.util.Arrays.asList(xs: _*) I don't think that's too constraining. Note: after this commit, tests/run/i533 started failing, this is fixed in the next commit. --- .../core/unpickleScala2/Scala2Unpickler.scala | 15 +-------------- .../src/dotty/tools/dotc/transform/Erasure.scala | 5 ----- tests/pos/arrays2.scala | 2 +- tests/run/java-varargs/A_1.java | 3 +++ tests/run/java-varargs/Test_2.scala | 2 ++ 5 files changed, 7 insertions(+), 20 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala index 67d626270a7e..e4678022e877 100644 --- a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala @@ -67,22 +67,9 @@ object Scala2Unpickler { case tp: MethodType => val lastArg = tp.paramInfos.last assert(lastArg isRef defn.ArrayClass) - val elemtp0 :: Nil = lastArg.baseType(defn.ArrayClass).argInfos - val elemtp = elemtp0 match { - case AndType(t1, t2) => // drop intersection with Object for abstract types and parameters in varargs. Erasure can handle them. - if t2.isAnyRef then - t1 match { - case t1: TypeParamRef => t1 - case t1: TypeRef if t1.symbol.isAbstractOrParamType => t1 - case _ => elemtp0 - } - else elemtp0 - case _ => - elemtp0 - } tp.derivedLambdaType( tp.paramNames, - tp.paramInfos.init :+ defn.RepeatedParamType.appliedTo(elemtp), + tp.paramInfos.init :+ lastArg.translateParameterized(defn.ArrayClass, defn.RepeatedParamClass), tp.resultType) case tp: PolyType => tp.derivedLambdaType(tp.paramNames, tp.paramInfos, arrayToRepeated(tp.resultType)) diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala index cb7f61b47f4b..119f977940e0 100644 --- a/compiler/src/dotty/tools/dotc/transform/Erasure.scala +++ b/compiler/src/dotty/tools/dotc/transform/Erasure.scala @@ -309,11 +309,6 @@ object Erasure { assert(!pt.isInstanceOf[SingletonType], pt) if (pt isRef defn.UnitClass) unbox(tree, pt) else (tree.tpe.widen, pt) match { - case (JavaArrayType(treeElem), JavaArrayType(ptElem)) - if treeElem.widen.isPrimitiveValueType && !ptElem.isPrimitiveValueType => - // See SI-2386 for one example of when this might be necessary. - cast(ref(defn.runtimeMethodRef(nme.toObjectArray)).appliedTo(tree), pt) - // When casting between two EVTs, we need to check which one underlies the other to determine // whether u2evt or evt2u should be used. case (tp1 @ ErasedValueType(tycon1, underlying1), tp2 @ ErasedValueType(tycon2, underlying2)) => diff --git a/tests/pos/arrays2.scala b/tests/pos/arrays2.scala index 8984fd615ad1..b2fe6ff1d6f0 100644 --- a/tests/pos/arrays2.scala +++ b/tests/pos/arrays2.scala @@ -23,5 +23,5 @@ one warning found // #2461 object arrays3 { - def apply[X](xs : X*) : java.util.List[X] = java.util.Arrays.asList(xs: _*) + def apply[X <: AnyRef](xs : X*) : java.util.List[X] = java.util.Arrays.asList(xs: _*) } diff --git a/tests/run/java-varargs/A_1.java b/tests/run/java-varargs/A_1.java index 46f82d07893e..9e98992e4097 100644 --- a/tests/run/java-varargs/A_1.java +++ b/tests/run/java-varargs/A_1.java @@ -1,4 +1,7 @@ class A_1 { public static void foo(int... args) { } + + public static void gen(T... args) { + } } diff --git a/tests/run/java-varargs/Test_2.scala b/tests/run/java-varargs/Test_2.scala index 4ef2d5b001f7..eb77d6e248f7 100644 --- a/tests/run/java-varargs/Test_2.scala +++ b/tests/run/java-varargs/Test_2.scala @@ -2,5 +2,7 @@ object Test { def main(args: Array[String]): Unit = { A_1.foo(Array(1): _*) A_1.foo(Seq(1): _*) + A_1.gen(Array(1): _*) + A_1.gen(Seq(1): _*) } } From c3cb92d306b81e1247d1df92eac27d2f772ea84e Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 8 Apr 2020 21:23:19 +0200 Subject: [PATCH 4/4] Properly handle generic arrays coming from Java sources Treat them like generic arrays coming from Java classfiles: `T[]` needs to be typed as `Array[T & Object]` since it will not accept primitive arrays. --- .../tools/dotc/core/TypeApplications.scala | 22 +++++++++++++++++++ .../dotc/core/classfile/ClassfileParser.scala | 12 ++-------- .../src/dotty/tools/dotc/typer/Typer.scala | 9 ++++++++ .../run/generic-java-array-src/Test.scala | 2 +- tests/neg/i533/JA.java | 9 ++++++++ tests/neg/i533/Test.scala | 8 +++++++ tests/run/i533/JA.java | 6 ++++- tests/run/i533/Test.scala | 3 ++- 8 files changed, 58 insertions(+), 13 deletions(-) create mode 100644 tests/neg/i533/JA.java create mode 100644 tests/neg/i533/Test.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala index 9ea66b9e4f07..39e8390e099e 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala @@ -408,6 +408,28 @@ class TypeApplications(val self: Type) extends AnyVal { def translateToRepeated(from: ClassSymbol)(using Context): Type = translateParameterized(from, defn.RepeatedParamClass) + /** Translate `T` by `T & Object` in the situations where an `Array[T]` + * coming from Java would need to be interpreted as an `Array[T & Object]` + * to be erased correctly. + * + * This is necessary because a fully generic Java array erases to an array of Object, + * whereas a fully generic Java array erases to Object to allow primitive arrays + * as subtypeS. + * + * Note: According to + * , + * in the future the JVM will consider that: + * + * int[] <: Integer[] <: Object[] + * + * So hopefully our grand-children will not have to deal with this non-sense! + */ + def translateJavaArrayElementType(using Context): Type = + if self.typeSymbol.isAbstractOrParamType && !self.derivesFrom(defn.ObjectClass) then + AndType(self, defn.ObjectType) + else + self + /** If this is an encoding of a (partially) applied type, return its arguments, * otherwise return Nil. * Existential types in arguments are returned as TypeBounds instances. diff --git a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala index 6dcda56dac10..e14389cbaa71 100644 --- a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala +++ b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala @@ -395,16 +395,8 @@ class ClassfileParser( tpe case ARRAY_TAG => while ('0' <= sig(index) && sig(index) <= '9') index += 1 - var elemtp = sig2type(tparams, skiptvs) - // make unbounded Array[T] where T is a type variable into Array[T with Object] - // (this is necessary because such arrays have a representation which is incompatible - // with arrays of primitive types. - // NOTE that the comparison to Object only works for abstract types bounded by classes that are strict subclasses of Object - // if the bound is exactly Object, it will have been converted to Any, and the comparison will fail - // see also RestrictJavaArraysMap (when compiling java sources directly) - if (elemtp.typeSymbol.isAbstractOrParamType && !(elemtp.derivesFrom(defn.ObjectClass))) - elemtp = AndType(elemtp, defn.ObjectType) - defn.ArrayOf(elemtp) + val elemtp = sig2type(tparams, skiptvs) + defn.ArrayOf(elemtp.translateJavaArrayElementType) case '(' => // we need a method symbol. given in line 486 by calling getType(methodSym, ..) val paramtypes = new ListBuffer[Type]() diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 63f7682a3003..833e4315c43c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1641,6 +1641,15 @@ class Typer extends Namer else if (tpt1.symbol == defn.orType) checkedArgs = checkedArgs.mapconserve(arg => checkSimpleKinded(checkNoWildcard(arg))) + else if (ctx.compilationUnit.isJava) + if (tpt1.symbol eq defn.ArrayClass) || (tpt1.symbol eq defn.RepeatedParamClass) then + checkedArgs match { + case List(arg) => + val elemtp = arg.tpe.translateJavaArrayElementType + if (elemtp ne arg.tpe) + checkedArgs = List(TypeTree(elemtp).withSpan(arg.span)) + case _ => + } assignType(cpy.AppliedTypeTree(tree)(tpt1, checkedArgs), tpt1, checkedArgs) } } diff --git a/tests/explicit-nulls/run/generic-java-array-src/Test.scala b/tests/explicit-nulls/run/generic-java-array-src/Test.scala index 22cc5ea1eb91..26e4886d499b 100644 --- a/tests/explicit-nulls/run/generic-java-array-src/Test.scala +++ b/tests/explicit-nulls/run/generic-java-array-src/Test.scala @@ -4,7 +4,7 @@ object Test { // then on the Scala side we'll need to pass a nullable array. // i.e. with explicit nulls the previously-implicit cast becomes an explicit // type annotation. - val x = new Array[Int|Null](1) + val x = new Array[Integer|Null](1) x(0) = 10 println(JA.get(x)) diff --git a/tests/neg/i533/JA.java b/tests/neg/i533/JA.java new file mode 100644 index 000000000000..49ce2e01f76c --- /dev/null +++ b/tests/neg/i533/JA.java @@ -0,0 +1,9 @@ +class JA { + public static T get(T[] arr) { + return arr[0]; + } + + public static T getVarargs(T ...arr) { + return arr[0]; + } +} diff --git a/tests/neg/i533/Test.scala b/tests/neg/i533/Test.scala new file mode 100644 index 000000000000..784715736574 --- /dev/null +++ b/tests/neg/i533/Test.scala @@ -0,0 +1,8 @@ +object Test { + def main(args: Array[String]): Unit = { + val x = new Array[Int](1) + x(0) = 10 + println(JA.get(x)) // error + println(JA.getVarargs(x: _*)) // error + } +} diff --git a/tests/run/i533/JA.java b/tests/run/i533/JA.java index 92421e5b1fee..49ce2e01f76c 100644 --- a/tests/run/i533/JA.java +++ b/tests/run/i533/JA.java @@ -2,4 +2,8 @@ class JA { public static T get(T[] arr) { return arr[0]; } -} \ No newline at end of file + + public static T getVarargs(T ...arr) { + return arr[0]; + } +} diff --git a/tests/run/i533/Test.scala b/tests/run/i533/Test.scala index 3f1091bf8e02..a8420450dc4e 100644 --- a/tests/run/i533/Test.scala +++ b/tests/run/i533/Test.scala @@ -1,7 +1,8 @@ object Test { def main(args: Array[String]): Unit = { - val x = new Array[Int](1) + val x = new Array[Integer](1) x(0) = 10 println(JA.get(x)) + println(JA.getVarargs(x: _*)) } }