From 1c5a01b2b6bd006c2929f0aa9290492aeeb281d5 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 30 Jul 2020 15:54:40 +0200 Subject: [PATCH 1/4] Fix some typos in the documentation --- compiler/src/dotty/tools/dotc/core/TypeApplications.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala index 94c83bd97036..12488d305d4a 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala @@ -413,13 +413,13 @@ 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]` + /** Translate `T` to `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. + * whereas a fully generic Scala array erases to Object to allow primitive arrays + * as subtypes. * * Note: According to * , From 2a86ad0f0bf117d0dcc06ac9660d8d4340ceb119 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 12 Aug 2020 16:59:55 +0200 Subject: [PATCH 2/4] Fix #8615: Add testcase This appears to already be fixed. --- tests/pos/i8615.scala | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 tests/pos/i8615.scala diff --git a/tests/pos/i8615.scala b/tests/pos/i8615.scala new file mode 100644 index 000000000000..12c80ce01871 --- /dev/null +++ b/tests/pos/i8615.scala @@ -0,0 +1,6 @@ +class ArrayOrdering[N] extends scala.math.Ordering[Array[N]] { + def compare(x: Array[N], y: Array[N]) = ??? +} +class ArrayIntOrdering extends scala.math.Ordering[Array[Int]] { + def compare(x: Array[Int], y: Array[Int]) = ??? // works fine +} From 8f7df6729f6aedce3637bcdddd892a5efa3f8a1c Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 28 Jul 2020 18:21:30 +0200 Subject: [PATCH 3/4] Improve handling of references to `Object` coming from Java code In Java unlike Scala, `Object` is the top type, this leads to various usability problems when attempting to call or override a Java method from Scala. So far, we relied mainly on one mechanism to improve the situation: in the ClassfileParser, some references to `Object` in signatures were replaced by `Any` (cf `objToAny`). But this had several shortcomings: - To compensate for this substitution, `TypeComparer#matchingMethodParams` had to special case Any in Java methods and treat it like Object - There were various situation were this substitution was not applied, notably when using varargs (`Object... args`) or when jointly compiling .java sources since this is handled by JavaParser and not ClassfileParser. This commit replaces all of this by a more systematic solution: all references to `Object` in Java definitions (both in classfiles and .java source files) are replaced by a special type `FromJavaObject` which is a type alias of `Object` with one extra subtyping rule: tp <:< FromJavaObject is equivalent to: tp <:< Any See the documentation of `FromJavaObjectSymbol` for details on why this makes sense. This solution is very much inspired by https://github.com/scala/scala/pull/7966 (which was refined in https://github.com/scala/scala/pull/8049 and https://github.com/scala/scala/pull/8638) with two main differences: - We use a type with its own symbol and name distinct from `java.lang.Object`, because this type can show up in inferred types and therefore needs to be preserved when pickling so that unpickled trees pass `-Ycheck`. - Unlike in Scala 2, we cannot get rid of `JavaMethodType` because when calling `Type#signature` we need to know whether we're in a Java method or not, because signatures are based on erased types and erasure differs between Scala and Java (we cannot ignore this and always base signatures on the Scala erasure, because signatures are used to distinguish between overloads and overrides so they must agree with the actual JVM bytecode signature). Fixes #7467, #7963, #8588, #8977. --- .../dotty/tools/dotc/core/Definitions.scala | 99 ++++++++++++++++++- .../src/dotty/tools/dotc/core/StdNames.scala | 1 + .../tools/dotc/core/TypeApplications.scala | 15 ++- .../dotty/tools/dotc/core/TypeComparer.scala | 30 ++---- .../src/dotty/tools/dotc/core/Types.scala | 2 + .../dotc/core/classfile/ClassfileParser.scala | 27 +++-- .../tools/dotc/parsing/JavaParsers.scala | 4 +- .../tools/dotc/printing/PlainPrinter.scala | 16 ++- .../tools/dotc/printing/RefinedPrinter.scala | 4 +- .../tools/dotc/transform/ElimRepeated.scala | 72 +++++++------- .../dotty/tools/dotc/typer/TypeAssigner.scala | 9 +- project/Build.scala | 1 - tests/neg/i1747.scala | 4 +- .../i8588/ConfigFactory_1.java | 4 + .../i8588/Test_2.scala | 5 + .../java-object-any-unification-1/J_1.java | 16 +++ .../Test_2.scala | 5 + .../t11469_2/Test_1.java | 11 +++ .../t11469_2/Test_2.scala | 11 +++ tests/pos-java-interop/i8593/A.scala | 4 + tests/pos-java-interop/i8593/B.java | 4 + .../java-object-any-unification-2/J.java | 16 +++ .../java-object-any-unification-2/Test.scala | 5 + tests/pos-java-interop/t11469/Generic.java | 4 + tests/pos-java-interop/t11469/test.scala | 4 + tests/pos-java-interop/varargs/A.java | 6 ++ tests/pos-java-interop/varargs/B.scala | 4 + tests/pos/i1747.scala | 6 +- tests/pos/i7467.scala | 10 ++ tests/run/i7963.scala | 7 ++ tests/run/i8977.scala | 11 +++ 31 files changed, 326 insertions(+), 91 deletions(-) create mode 100644 tests/pos-java-interop-separate/i8588/ConfigFactory_1.java create mode 100644 tests/pos-java-interop-separate/i8588/Test_2.scala create mode 100644 tests/pos-java-interop-separate/java-object-any-unification-1/J_1.java create mode 100644 tests/pos-java-interop-separate/java-object-any-unification-1/Test_2.scala create mode 100644 tests/pos-java-interop-separate/t11469_2/Test_1.java create mode 100644 tests/pos-java-interop-separate/t11469_2/Test_2.scala create mode 100644 tests/pos-java-interop/i8593/A.scala create mode 100644 tests/pos-java-interop/i8593/B.java create mode 100644 tests/pos-java-interop/java-object-any-unification-2/J.java create mode 100644 tests/pos-java-interop/java-object-any-unification-2/Test.scala create mode 100644 tests/pos-java-interop/t11469/Generic.java create mode 100644 tests/pos-java-interop/t11469/test.scala create mode 100644 tests/pos-java-interop/varargs/A.java create mode 100644 tests/pos-java-interop/varargs/B.scala create mode 100644 tests/pos/i7467.scala create mode 100644 tests/run/i7963.scala create mode 100644 tests/run/i8977.scala diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 37c6c9e86eee..9c29da538c0a 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -195,7 +195,7 @@ class Definitions { RootClass, nme.EMPTY_PACKAGE, (emptypkg, emptycls) => ctx.base.rootLoader(emptypkg)).entered @tu lazy val EmptyPackageClass: ClassSymbol = EmptyPackageVal.moduleClass.asClass - /** A package in which we can place all methods that are interpreted specially by the compiler */ + /** A package in which we can place all methods and types that are interpreted specially by the compiler */ @tu lazy val OpsPackageVal: TermSymbol = newCompletePackageSymbol(RootClass, nme.OPS_PACKAGE).entered @tu lazy val OpsPackageClass: ClassSymbol = OpsPackageVal.moduleClass.asClass @@ -310,6 +310,103 @@ class Definitions { } def ObjectType: TypeRef = ObjectClass.typeRef + /** A type alias of Object used to represent any reference to Object in a Java + * signature, the secret sauce is that subtype checking treats it specially: + * + * tp <:< FromJavaObject + * + * is equivalent to: + * + * tp <:< Any + * + * This is useful to avoid usability problems when interacting with Java + * code where Object is the top type. This is safe because this type will + * only appear in signatures of Java definitions in positions where `Object` + * might appear, let's enumerate all possible cases this gives us: + * + * 1. At the top level: + * + * // A.java + * void meth1(Object arg) {} + * void meth2(T arg) {} // T implicitly extends Object + * + * // B.scala + * meth1(1) // OK + * meth2(1) // OK + * + * This is safe even though Int is not a subtype of Object, because Erasure + * will detect the mismatch and box the value type. + * + * 2. In a class type parameter: + * + * // A.java + * void meth3(scala.List arg) {} + * void meth4(scala.List arg) {} + * + * // B.scala + * meth3(List[Int](1)) // OK + * meth4(List[Int](1)) // OK + * + * At erasure, type parameters are removed and value types are boxed. + * + * 3. As the type parameter of an array: + * + * // A.java + * void meth5(Object[] arg) {} + * void meth6(T[] arg) {} + * + * // B.scala + * meth5(Array[Int](1)) // error: Array[Int] is not a subtype of Array[Object] + * meth6(Array[Int](1)) // error: Array[Int] is not a subtype of Array[T & Object] + * + * + * This is a bit more subtle: at erasure, Arrays keep their type parameter, + * and primitive Arrays are not subtypes of reference Arrays on the JVM, + * so we can't pass an Array of Int where a reference Array is expected. + * Array is invariant in Scala, so `meth5` is safe even if we use `FromJavaObject`, + * but generic Arrays are treated specially: we always add `& Object` (and here + * we mean the normal java.lang.Object type) to these types when they come from + * Java signatures (see `translateJavaArrayElementType`), this ensure that `meth6` + * is safe to use. + * + * 4. As the repeated argument of a varargs method: + * + * // A.java + * void meth7(Object... args) {} + * void meth8(T... args) {} + * + * // B.scala + * meth7(1) // OK + * meth8(1) // OK + * val ai = Array[Int](1) + * meth7(ai: _*) // OK (will copy the array) + * meth8(ai: _*) // OK (will copy the array) + * + * Java repeated arguments are erased to arrays, so it would be safe to treat + * them in the same way: add an `& Object` to the parameter type to disallow + * passing primitives, but that would be very inconvenient as it is common to + * want to pass a primitive to an Object repeated argument (e.g. + * `String.format("foo: %d", 1)`). So instead we type them _without_ adding the + * `& Object` and let `ElimRepeated` take care of doing any necessary adaptation + * (note that adapting a primitive array to a reference array requires + * copying the whole array, so this transformation only preserves semantics + * if the callee does not try to mutate the varargs array which is a reasonable + * assumption to make). + * + * + * This mechanism is similar to `ObjectTpeJavaRef` in Scala 2, except that we + * create a new symbol with its own name, this is needed because this type + * can show up in inferred types and therefore needs to be preserved when + * pickling so that unpickled trees pass `-Ycheck`. + * + * Note that by default we pretty-print `FromJavaObject` as `Object` or simply omit it + * if it's the sole upper-bound of a type parameter, use `-Yprint-debug` to explicitly + * display it. + */ + @tu lazy val FromJavaObjectSymbol: TypeSymbol = + newPermanentSymbol(OpsPackageClass, tpnme.FromJavaObject, JavaDefined, TypeAlias(ObjectType)).entered + def FromJavaObjectType: TypeRef = FromJavaObjectSymbol.typeRef + @tu lazy val AnyRefAlias: TypeSymbol = enterAliasType(tpnme.AnyRef, ObjectType) def AnyRefType: TypeRef = AnyRefAlias.typeRef diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index 5ec7fe75aca5..7e69d9488675 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/compiler/src/dotty/tools/dotc/core/StdNames.scala @@ -199,6 +199,7 @@ object StdNames { final val Null: N = "Null" final val UncheckedNull: N = "UncheckedNull" final val Object: N = "Object" + final val FromJavaObject: N = "" final val Product: N = "Product" final val PartialFunction: N = "PartialFunction" final val PrefixType: N = "PrefixType" diff --git a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala index 12488d305d4a..857b4e4aae42 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala @@ -417,20 +417,25 @@ class TypeApplications(val self: Type) extends AnyVal { * 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 Scala array erases to Object to allow primitive arrays - * as subtypes. + * `Object` is the top-level type in Java, but when it appears in a Java + * signature we replace it by a special `FromJavaObject` type for + * convenience, this in turns requires us to special-case generic arrays as + * described in case 3 in the documentation of `FromJavaObjectSymbol`. This + * is necessary because a fully generic Java array erases to an array of + * Object, whereas a fully generic Scala array erases to Object to allow + * primitive arrays as subtypes. * * Note: According to * , - * in the future the JVM will consider that: + * it's possible that future JVMs 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 + // A type parameter upper-bounded solely by `FromJavaObject` has `ObjectClass` as its classSymbol + if self.typeSymbol.isAbstractOrParamType && (self.classSymbol eq defn.ObjectClass) then AndType(self, defn.ObjectType) else self diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 2dc5218b8bc9..d29ae778a5ef 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -275,7 +275,11 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling case _ => secondTry end compareNamed - compareNamed(tp1, tp2) + // See the documentation of `FromJavaObjectSymbol` + if !ctx.erasedTypes && tp2.isFromJavaObject then + recur(tp1, defn.AnyType) + else + compareNamed(tp1, tp2) case tp2: ProtoType => isMatchedByProto(tp2, tp1) case tp2: BoundType => @@ -1769,8 +1773,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling } /** Do the parameter types of `tp1` and `tp2` match in a way that allows `tp1` - * to override `tp2` ? This is the case if they're pairwise =:=, as a special - * case, we allow `Any` in Java methods to match `Object`. + * to override `tp2` ? This is the case if they're pairwise `=:=`. */ def matchingMethodParams(tp1: MethodType, tp2: MethodType): Boolean = { def loop(formals1: List[Type], formals2: List[Type]): Boolean = formals1 match { @@ -1778,26 +1781,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling formals2 match { case formal2 :: rest2 => val formal2a = if (tp2.isParamDependent) formal2.subst(tp2, tp1) else formal2 - // The next two definitions handle the special case mentioned above, where - // the Java argument has type 'Any', and the Scala argument has type 'Object' or - // 'Object|Null', depending on whether explicit nulls are enabled. - def formal1IsObject = - if (ctx.explicitNulls) formal1 match { - case OrNull(formal1b) => formal1b.isAnyRef - case _ => false - } - else formal1.isAnyRef - def formal2IsObject = - if (ctx.explicitNulls) formal2 match { - case OrNull(formal2b) => formal2b.isAnyRef - case _ => false - } - else formal2.isAnyRef - (isSameTypeWhenFrozen(formal1, formal2a) - || tp1.isJavaMethod && formal2IsObject && formal1.isAny - || tp2.isJavaMethod && formal1IsObject && formal2.isAny - ) - && loop(rest1, rest2) + isSameTypeWhenFrozen(formal1, formal2a) && loop(rest1, rest2) case nil => false } diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index df248eb2987e..8f32ef3366f9 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -205,6 +205,8 @@ object Types { def isAnyRef(using Context): Boolean = isRef(defn.ObjectClass, skipRefined = false) def isAnyKind(using Context): Boolean = isRef(defn.AnyKindClass, skipRefined = false) + def isFromJavaObject(using Context): Boolean = typeSymbol eq defn.FromJavaObjectSymbol + /** Does this type refer exactly to class symbol `sym`, instead of to a subclass of `sym`? * Implemented like `isRef`, but follows more types: all type proxies as well as and- and or-types */ diff --git a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala index d4d0d702f45f..f558defee1d5 100644 --- a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala +++ b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala @@ -305,10 +305,6 @@ class ClassfileParser( } } - /** Map direct references to Object to references to Any */ - final def objToAny(tp: Type)(using Context): Type = - if (tp.isDirectRef(defn.ObjectClass) && !ctx.phase.erasedTypes) defn.AnyType else tp - def constantTagToType(tag: Int)(using Context): Type = (tag: @switch) match { case BYTE_TAG => defn.ByteType @@ -356,14 +352,14 @@ class ClassfileParser( case variance @ ('+' | '-' | '*') => index += 1 variance match { - case '+' => objToAny(TypeBounds.upper(sig2type(tparams, skiptvs))) + case '+' => TypeBounds.upper(sig2type(tparams, skiptvs)) case '-' => - val tp = sig2type(tparams, skiptvs) - // sig2type seems to return AnyClass regardless of the situation: - // we don't want Any as a LOWER bound. - if (tp.isDirectRef(defn.AnyClass)) TypeBounds.empty - else TypeBounds.lower(tp) - case '*' => TypeBounds.empty + val argTp = sig2type(tparams, skiptvs) + // Interpret `sig2type` returning `Any` as "no bounds"; + // morally equivalent to TypeBounds.empty, but we're representing Java code, so use FromJavaObjectType as the upper bound + if (argTp.typeSymbol == defn.AnyClass) TypeBounds.upper(defn.FromJavaObjectType) + else TypeBounds(argTp, defn.FromJavaObjectType) + case '*' => TypeBounds.upper(defn.FromJavaObjectType) } case _ => sig2type(tparams, skiptvs) } @@ -379,7 +375,8 @@ class ClassfileParser( } val classSym = classNameToSymbol(subName(c => c == ';' || c == '<')) - var tpe = processClassType(processInner(classSym.typeRef)) + val classTpe = if (classSym eq defn.ObjectClass) defn.FromJavaObjectType else classSym.typeRef + var tpe = processClassType(processInner(classTpe)) while (sig(index) == '.') { accept('.') val name = subName(c => c == ';' || c == '<' || c == '.').toTypeName @@ -426,7 +423,7 @@ class ClassfileParser( // `ElimRepeated` is responsible for correctly erasing this. defn.RepeatedParamType.appliedTo(elemType) else - objToAny(sig2type(tparams, skiptvs)) + sig2type(tparams, skiptvs) } index += 1 @@ -448,7 +445,7 @@ class ClassfileParser( while (sig(index) == ':') { index += 1 if (sig(index) != ':') // guard against empty class bound - ts += objToAny(sig2type(tparams, skiptvs)) + ts += sig2type(tparams, skiptvs) } val bound = if ts.isEmpty then defn.AnyType else ts.reduceLeft(AndType.apply) TypeBounds.upper(bound) @@ -497,7 +494,7 @@ class ClassfileParser( classTParams = tparams val parents = new ListBuffer[Type]() while (index < end) - parents += sig2type(tparams, skiptvs = false) // here the variance doesn't matter + parents += sig2type(tparams, skiptvs = false) // here the variance doesn't matter TempClassInfoType(parents.toList, instanceScope, owner) } if (ownTypeParams.isEmpty) tpe else TempPolyType(ownTypeParams, tpe) diff --git a/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala b/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala index 55b6eb917342..549ca054c136 100644 --- a/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala @@ -304,7 +304,7 @@ object JavaParsers { if (in.token == QMARK) { val offset = in.offset in.nextToken() - val hi = if (in.token == EXTENDS) { in.nextToken() ; typ() } else EmptyTree + val hi = if (in.token == EXTENDS) { in.nextToken() ; typ() } else javaLangObject() val lo = if (in.token == SUPER) { in.nextToken() ; typ() } else EmptyTree atSpan(offset) { /* @@ -434,7 +434,7 @@ object JavaParsers { def typeParam(flags: FlagSet): TypeDef = atSpan(in.offset) { val name = identForType() - val hi = if (in.token == EXTENDS) { in.nextToken() ; bound() } else EmptyTree + val hi = if (in.token == EXTENDS) { in.nextToken() ; bound() } else javaLangObject() TypeDef(name, TypeBoundsTree(EmptyTree, hi)).withMods(Modifiers(flags)) } diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index 84d422c99207..d7f5c8490f5c 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -3,6 +3,7 @@ package printing import core._ import Texts._, Types._, Flags._, Names._, Symbols._, NameOps._, Constants._, Denotations._ +import StdNames._ import Contexts._ import Scopes.Scope, Denotations.Denotation, Annotations.Annotation import StdNames.nme @@ -89,7 +90,10 @@ class PlainPrinter(_ctx: Context) extends Printer { || (sym.name == nme.PACKAGE) // package ) - def nameString(name: Name): String = name.toString + def nameString(name: Name): String = + if (name eq tpnme.FromJavaObject) && !printDebug + then nameString(tpnme.Object) + else name.toString def toText(name: Name): Text = Str(nameString(name)) @@ -123,11 +127,13 @@ class PlainPrinter(_ctx: Context) extends Printer { }) /** Direct references to these symbols are printed without their prefix for convenience. - * They are either aliased in scala.Predef or in the scala package object. + * They are either aliased in scala.Predef or in the scala package object, as well as `Object` */ private lazy val printWithoutPrefix: Set[Symbol] = (defn.ScalaPredefModule.termRef.typeAliasMembers ++ defn.ScalaPackageObject.termRef.typeAliasMembers).map(_.info.classSymbol).toSet + + defn.ObjectClass + + defn.FromJavaObjectSymbol def toText(tp: Type): Text = controlled { homogenize(tp) match { @@ -267,7 +273,9 @@ class PlainPrinter(_ctx: Context) extends Printer { simpleNameString(sym) + idString(sym) // + "<" + (if (sym.exists) sym.owner else "") + ">" def fullNameString(sym: Symbol): String = - if (sym.isRoot || sym == NoSymbol || sym.owner.isEffectiveRoot) + if (sym eq defn.FromJavaObjectSymbol) && !printDebug then + fullNameString(defn.ObjectClass) + else if sym.isRoot || sym == NoSymbol || sym.owner.isEffectiveRoot then nameString(sym) else fullNameString(fullNameOwner(sym)) + "." + nameString(sym) @@ -365,7 +373,7 @@ class PlainPrinter(_ctx: Context) extends Printer { " = " ~ toText(tp.alias) case TypeBounds(lo, hi) => (if (lo isRef defn.NothingClass) Text() else " >: " ~ toText(lo)) - ~ (if hi.isAny then Text() else " <: " ~ toText(hi)) + ~ (if hi.isAny || (!printDebug && hi.isFromJavaObject) then Text() else " <: " ~ toText(hi)) tparamStr ~ binder case tp @ ClassInfo(pre, cls, cparents, decls, selfInfo) => val preText = toTextLocal(pre) diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index f61956c9f275..f6ae7b252690 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -76,7 +76,9 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { } override def nameString(name: Name): String = - if (ctx.settings.YdebugNames.value) name.debugString else name.toString + if ctx.settings.YdebugNames.value + then name.debugString + else super.nameString(name) override protected def simpleNameString(sym: Symbol): String = nameString(if (ctx.property(XprintMode).isEmpty) sym.initial.name else sym.name) diff --git a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala index d94b42d6764e..f0711cf2d6ff 100644 --- a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala +++ b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala @@ -96,18 +96,18 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => val last = paramTypes(lastIdx) if last.isRepeatedParam then val isJava = tp.isJavaMethod - // A generic Java varargs `T...` where `T` is unbounded is erased to - // `Object[]` in bytecode, we directly translate such a type to - // `Array[_ <: Object]` instead of `Array[_ <: T]` here. This allows - // the tree transformer of this phase to emit the correct adaptation - // for repeated arguments if needed (for example, an `Array[Int]` will - // be copied into an `Array[Object]`, see `adaptToArray`). + // We need to be careful when handling Java repeated parameters + // of the form `Object...` or `T...` where `T` is unbounded: + // in both cases, `Object` will have been translated to `FromJavaObject` + // to allow passing primitives as repeated arguments, but we can't + // pass a primitive array as argument to such a method since the + // parameter will be erased to `Object[]`. To handle this correctly we + // drop usage of `FromJavaObject` as an element type here, the + // tree transformer of this phase is then responsible for handling + // mismatches by emitting the correct adaptation (cf `adaptToArray`). + // See also the documentation of `FromJavaObjectSymbol`. val last1 = - if isJava && { - val elemTp = last.elemType - elemTp.isInstanceOf[TypeParamRef] && elemTp.typeSymbol == defn.AnyClass - } - then + if isJava && last.elemType.isFromJavaObject then defn.ArrayOf(TypeBounds.upper(defn.ObjectType)) else last.translateFromRepeated(toArray = isJava) @@ -171,29 +171,35 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => */ private def adaptToArray(tree: Tree, elemPt: Type)(implicit ctx: Context): Tree = val elemTp = tree.tpe.elemType + val elemTpMatches = elemTp <:< elemPt val treeIsArray = tree.tpe.derivesFrom(defn.ArrayClass) - if elemTp <:< elemPt then - if treeIsArray then - tree // no adaptation needed - else - tree match - case SeqLiteral(elems, elemtpt) => - JavaSeqLiteral(elems, elemtpt).withSpan(tree.span) - case _ => - // Convert a Seq[T] to an Array[$elemPt] - ref(defn.DottyArraysModule) - .select(nme.seqToArray) - .appliedToType(elemPt) - .appliedTo(tree, clsOf(elemPt)) - else if treeIsArray then - // Convert an Array[T] to an Array[Object] - ref(defn.ScalaRuntime_toObjectArray) - .appliedTo(tree) - else - // Convert a Seq[T] to an Array[Object] - ref(defn.ScalaRuntime_toArray) - .appliedToType(elemTp) - .appliedTo(tree) + if elemTpMatches && treeIsArray then + tree // No adaptation necessary + else tree match + case SeqLiteral(elems, elemtpt) => + // By the precondition, we only have mismatches if elemPt is Object, in + // that case we use `FromJavaObject` as the element type to allow the + // sequence literal to typecheck no matter the types of the elements, + // Erasure will take care of any necessary boxing (see documentation + // of `FromJavaObjectSymbol` for more information). + val adaptedElemTpt = if elemTpMatches then elemtpt else TypeTree(defn.FromJavaObjectType) + JavaSeqLiteral(elems, adaptedElemTpt).withSpan(tree.span) + case _ => + if treeIsArray then + // Convert an Array[T] to an Array[Object] + ref(defn.ScalaRuntime_toObjectArray) + .appliedTo(tree) + else if elemTpMatches then + // Convert a Seq[T] to an Array[$elemPt] + ref(defn.DottyArraysModule) + .select(nme.seqToArray) + .appliedToType(elemPt) + .appliedTo(tree, clsOf(elemPt)) + else + // Convert a Seq[T] to an Array[Object] + ref(defn.ScalaRuntime_toArray) + .appliedToType(elemTp) + .appliedTo(tree) /** Convert an Array into a scala.Seq */ private def arrayToSeq(tree: Tree)(using Context): Tree = diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index 402d8b567d22..071378013d58 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -77,6 +77,7 @@ trait TypeAssigner { * (1) parameter accessors are always dereferenced. * (2) if the owner of the denotation is a package object, it is assured * that the package object shows up as the prefix. + * (3) in Java compilation units, `Object` is replaced by `defn.FromJavaObjectType` */ def ensureAccessible(tpe: Type, superAccess: Boolean, pos: SrcPos)(using Context): Type = { def test(tpe: Type, firstTry: Boolean): Type = tpe match { @@ -84,7 +85,7 @@ trait TypeAssigner { val pre = tpe.prefix val name = tpe.name val d = tpe.denot.accessibleFrom(pre, superAccess) - if (!d.exists) { + if !d.exists then // it could be that we found an inaccessible private member, but there is // an inherited non-private member with the same name and signature. val d2 = pre.nonPrivateMember(name) @@ -109,8 +110,10 @@ trait TypeAssigner { if (tpe.isError) tpe else errorType(ex"$whatCanNot be accessed as a member of $pre$where.$whyNot", pos) } - } - else TypeOps.makePackageObjPrefixExplicit(tpe withDenot d) + else if ctx.compilationUnit != null && ctx.compilationUnit.isJava && tpe.isAnyRef then + defn.FromJavaObjectType + else + TypeOps.makePackageObjPrefixExplicit(tpe withDenot d) case _ => tpe } diff --git a/project/Build.scala b/project/Build.scala index 6a40197cbb4f..9c9b7aefb6ab 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -1036,7 +1036,6 @@ object Build { -- "CollectionsOnCheckedListTest.scala" -- "CollectionsOnCheckedMapTest.scala" -- "CollectionsOnCheckedSetTest.scala" - -- "CollectionsOnCollectionsTest.scala" -- "CollectionsOnListsTest.scala" -- "CollectionsOnMapsTest.scala" -- "CollectionsOnSetFromMapTest.scala" diff --git a/tests/neg/i1747.scala b/tests/neg/i1747.scala index c34fe4f95b4b..5d5d80115d7e 100644 --- a/tests/neg/i1747.scala +++ b/tests/neg/i1747.scala @@ -1,3 +1,3 @@ -abstract class Coll[E] extends java.util.Collection[E] { - override def toArray[T](a: Array[T]): Array[T] = ??? // error: has different signature +abstract class Coll3[E] extends java.util.Collection[E] { + override def toArray[T](a: Array[T]): Array[T] = ??? // error: method toArray has a different signature than the overridden declaration } diff --git a/tests/pos-java-interop-separate/i8588/ConfigFactory_1.java b/tests/pos-java-interop-separate/i8588/ConfigFactory_1.java new file mode 100644 index 000000000000..f5a3d4b5b522 --- /dev/null +++ b/tests/pos-java-interop-separate/i8588/ConfigFactory_1.java @@ -0,0 +1,4 @@ +import java.util.Map; +public class ConfigFactory_1 { + public static void parseMap(Map values) { } +} diff --git a/tests/pos-java-interop-separate/i8588/Test_2.scala b/tests/pos-java-interop-separate/i8588/Test_2.scala new file mode 100644 index 000000000000..28efb9ef38b6 --- /dev/null +++ b/tests/pos-java-interop-separate/i8588/Test_2.scala @@ -0,0 +1,5 @@ +import scala.jdk.CollectionConverters._ +object Test { + ConfigFactory_1.parseMap(Map("a" -> 1).asJava) + ConfigFactory_1.parseMap(Map("a" -> "", "b" -> true).asJava) +} diff --git a/tests/pos-java-interop-separate/java-object-any-unification-1/J_1.java b/tests/pos-java-interop-separate/java-object-any-unification-1/J_1.java new file mode 100644 index 000000000000..b4837e5fa4a4 --- /dev/null +++ b/tests/pos-java-interop-separate/java-object-any-unification-1/J_1.java @@ -0,0 +1,16 @@ +package p1; + +public class J_1 { + public static class Map {} + + public static class A { + public synchronized void putAll(Map t) { + } + } + + public static class B extends A { + @Override + public synchronized void putAll(Map t) { + } + } +} diff --git a/tests/pos-java-interop-separate/java-object-any-unification-1/Test_2.scala b/tests/pos-java-interop-separate/java-object-any-unification-1/Test_2.scala new file mode 100644 index 000000000000..76907cfc6d18 --- /dev/null +++ b/tests/pos-java-interop-separate/java-object-any-unification-1/Test_2.scala @@ -0,0 +1,5 @@ +object Test { + def test(b: p1.J_1.B) = { + b.putAll(null) + } +} diff --git a/tests/pos-java-interop-separate/t11469_2/Test_1.java b/tests/pos-java-interop-separate/t11469_2/Test_1.java new file mode 100644 index 000000000000..bab00de09b95 --- /dev/null +++ b/tests/pos-java-interop-separate/t11469_2/Test_1.java @@ -0,0 +1,11 @@ +import java.util.Optional; + +public class Test_1 { + public abstract static class A { + public void m(Optional a) { } + } + public abstract static class B extends A { + @Override + public void m(Optional a) { } + } +} diff --git a/tests/pos-java-interop-separate/t11469_2/Test_2.scala b/tests/pos-java-interop-separate/t11469_2/Test_2.scala new file mode 100644 index 000000000000..296d2d02af39 --- /dev/null +++ b/tests/pos-java-interop-separate/t11469_2/Test_2.scala @@ -0,0 +1,11 @@ +class A { + new Test_1.B() { } + + new Test_1.B() { + override def m(a: java.util.Optional[_ <: Object]): Unit = { } + } + + new Test_1.B() { + override def m(a: java.util.Optional[_]): Unit = { } + } +} diff --git a/tests/pos-java-interop/i8593/A.scala b/tests/pos-java-interop/i8593/A.scala new file mode 100644 index 000000000000..220cd7831e2b --- /dev/null +++ b/tests/pos-java-interop/i8593/A.scala @@ -0,0 +1,4 @@ +abstract class A { + @throws(classOf[Throwable]) + def onReceive(message: Any): Unit +} diff --git a/tests/pos-java-interop/i8593/B.java b/tests/pos-java-interop/i8593/B.java new file mode 100644 index 000000000000..1f9c8c4f16fe --- /dev/null +++ b/tests/pos-java-interop/i8593/B.java @@ -0,0 +1,4 @@ +public class B extends A { + @Override + public void onReceive(Object message) throws Throwable {} +} diff --git a/tests/pos-java-interop/java-object-any-unification-2/J.java b/tests/pos-java-interop/java-object-any-unification-2/J.java new file mode 100644 index 000000000000..7583fd37161e --- /dev/null +++ b/tests/pos-java-interop/java-object-any-unification-2/J.java @@ -0,0 +1,16 @@ +package p1; + +public class J { + public static class Map {} + + public static class A { + public synchronized void putAll(Map t) { + } + } + + public static class B extends A { + @Override + public synchronized void putAll(Map t) { + } + } +} diff --git a/tests/pos-java-interop/java-object-any-unification-2/Test.scala b/tests/pos-java-interop/java-object-any-unification-2/Test.scala new file mode 100644 index 000000000000..d04a3b19e003 --- /dev/null +++ b/tests/pos-java-interop/java-object-any-unification-2/Test.scala @@ -0,0 +1,5 @@ +object Test { + def test(b: p1.J.B) = { + b.putAll(null) + } +} diff --git a/tests/pos-java-interop/t11469/Generic.java b/tests/pos-java-interop/t11469/Generic.java new file mode 100644 index 000000000000..479f49a65dc8 --- /dev/null +++ b/tests/pos-java-interop/t11469/Generic.java @@ -0,0 +1,4 @@ +package jkson; + +abstract class Erased { public abstract Object foo(); } +public abstract class Generic extends Erased { public T foo() { return null; } } diff --git a/tests/pos-java-interop/t11469/test.scala b/tests/pos-java-interop/t11469/test.scala new file mode 100644 index 000000000000..df8db87d63a1 --- /dev/null +++ b/tests/pos-java-interop/t11469/test.scala @@ -0,0 +1,4 @@ +import jkson.Generic + +abstract class ScalaGen[T] extends Generic[T] +abstract class ScalaMono extends Generic[Product] diff --git a/tests/pos-java-interop/varargs/A.java b/tests/pos-java-interop/varargs/A.java new file mode 100644 index 000000000000..7d03b5170a2e --- /dev/null +++ b/tests/pos-java-interop/varargs/A.java @@ -0,0 +1,6 @@ +import java.io.Serializable; + +public class A { + public void foo(Object... x) {} + public void foo(String x) {} +} diff --git a/tests/pos-java-interop/varargs/B.scala b/tests/pos-java-interop/varargs/B.scala new file mode 100644 index 000000000000..774d4ceba1d2 --- /dev/null +++ b/tests/pos-java-interop/varargs/B.scala @@ -0,0 +1,4 @@ +object Test { + val a = new A + a.foo(42) +} diff --git a/tests/pos/i1747.scala b/tests/pos/i1747.scala index 9be62a10a084..d09bca26c5ed 100644 --- a/tests/pos/i1747.scala +++ b/tests/pos/i1747.scala @@ -1,3 +1,7 @@ -abstract class Coll[E] extends java.util.Collection[E] { +abstract class Coll1[E] extends java.util.Collection[E] { override def toArray[T](a: Array[T with Object]): Array[T with Object] = ??? } + +abstract class Coll2[E] extends java.util.Collection[E] { + override def toArray[T <: Object](a: Array[T]): Array[T] = ??? +} diff --git a/tests/pos/i7467.scala b/tests/pos/i7467.scala new file mode 100644 index 000000000000..6ddee908eaee --- /dev/null +++ b/tests/pos/i7467.scala @@ -0,0 +1,10 @@ +import javax.swing._ +import java.awt._ + +class DuplicateSymbolError_DirectSuperclass extends DefaultListCellRenderer() { + override def getListCellRendererComponent(list: JList[_ <: Object], value: Object, index: Int, isSelected: Boolean, cellHasFocus: Boolean): Component = ??? +} + +class DuplicateSymbolError_IndirectInterface extends DefaultListCellRenderer() { + override def getListCellRendererComponent(list: JList[_], value: Object, index: Int, isSelected: Boolean, cellHasFocus: Boolean): Component = ??? +} diff --git a/tests/run/i7963.scala b/tests/run/i7963.scala new file mode 100644 index 000000000000..94b7e86adcbd --- /dev/null +++ b/tests/run/i7963.scala @@ -0,0 +1,7 @@ +object Test { + def main(args: Array[String]): Unit = { + val l = new java.util.ArrayList[String] + l.add("foo") + java.util.Collections.min(l) + } +} diff --git a/tests/run/i8977.scala b/tests/run/i8977.scala new file mode 100644 index 000000000000..68e9835bd314 --- /dev/null +++ b/tests/run/i8977.scala @@ -0,0 +1,11 @@ +object Test { + val pi = 3.1415926 + + def main(args: Array[String]): Unit = { + System.out.printf("pi = %6.4f\n", pi) + System.out.printf("pi = %6.4f\n", Seq[scala.Double](pi):_*) + System.out.printf("pi = %6.4f\n", Seq[java.lang.Double](pi):_*) + System.out.printf("pi = %6.4f\n", Array[scala.Double](pi):_*) + System.out.printf("pi = %6.4f\n", Array[java.lang.Double](pi):_*) + } +} From 173899b64d2000eb8ad47dc316384241df4cf81a Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 21 Aug 2020 15:06:29 +0200 Subject: [PATCH 4/4] Introduce Context#isJava Safer to use than ctx.compilationUnit.isJava since compilationUnit is currently nullable (and ends up being null at least in some of our tests). --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 2 +- compiler/src/dotty/tools/dotc/core/Contexts.scala | 7 +++++++ compiler/src/dotty/tools/dotc/transform/Erasure.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Checking.scala | 2 +- compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Typer.scala | 6 +++--- 6 files changed, 14 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 321639456124..fdf49447713f 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -1625,7 +1625,7 @@ object desugar { Apply(Select(Apply(scalaDot(nme.StringContext), strs), id).withSpan(tree.span), elems) case PostfixOp(t, op) => if ((ctx.mode is Mode.Type) && !isBackquoted(op) && op.name == tpnme.raw.STAR) { - if ctx.compilationUnit.isJava then + if ctx.isJava then AppliedTypeTree(ref(defn.RepeatedParamType), t) else Annotated( diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index ce2b2fa508e7..95875bfedcc1 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -363,6 +363,13 @@ object Contexts { /** Does current phase use an erased types interpretation? */ final def erasedTypes = phase.erasedTypes + /** Are we in a Java compilation unit? */ + final def isJava: Boolean = + // FIXME: It would be much nicer if compilationUnit was non-nullable, + // perhaps we need to introduce a `NoCompilationUnit` compilation unit + // to be used as a default value. + compilationUnit != null && compilationUnit.isJava + /** Is current phase after FrontEnd? */ final def isAfterTyper = base.isAfterTyper(phase) diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala index 5516346dc09f..fe215b369d5f 100644 --- a/compiler/src/dotty/tools/dotc/transform/Erasure.scala +++ b/compiler/src/dotty/tools/dotc/transform/Erasure.scala @@ -542,7 +542,7 @@ object Erasure { private def checkValue(tree: Tree)(using Context): Unit = val sym = tree.tpe.termSymbol if (sym is Flags.Package) - || (sym.isAllOf(Flags.JavaModule) && !ctx.compilationUnit.isJava) + || (sym.isAllOf(Flags.JavaModule) && !ctx.isJava) then report.error(JavaSymbolIsNotAValue(sym), tree.srcPos) diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 2b8987103556..a81386ec2209 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -943,7 +943,7 @@ trait Checking { /** Check that `tpt` does not define a higher-kinded type */ def checkSimpleKinded(tpt: Tree)(using Context): Tree = - if (!tpt.tpe.hasSimpleKind && !ctx.compilationUnit.isJava) + if (!tpt.tpe.hasSimpleKind && !ctx.isJava) // be more lenient with missing type params in Java, // needed to make pos/java-interop/t1196 work. errorTree(tpt, MissingTypeParameterFor(tpt.tpe)) diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index 071378013d58..144ca853c056 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -110,7 +110,7 @@ trait TypeAssigner { if (tpe.isError) tpe else errorType(ex"$whatCanNot be accessed as a member of $pre$where.$whyNot", pos) } - else if ctx.compilationUnit != null && ctx.compilationUnit.isJava && tpe.isAnyRef then + else if ctx.isJava && tpe.isAnyRef then defn.FromJavaObjectType else TypeOps.makePackageObjPrefixExplicit(tpe withDenot d) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 31e8fac005dc..adfd6713abe6 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -576,7 +576,7 @@ class Typer extends Namer val qual1 = typedType(tree.qualifier, selectionProto(tree.name, pt, this)) assignType(cpy.Select(tree)(qual1, tree.name), qual1) } - else if (ctx.compilationUnit.isJava && tree.name.isTypeName) + else if (ctx.isJava && tree.name.isTypeName) // SI-3120 Java uses the same syntax, A.B, to express selection from the // value A and from the type A. We have to try both. selectWithFallback(tryJavaSelectOnType) // !!! possibly exponential bcs of qualifier retyping @@ -1760,7 +1760,7 @@ class Typer extends Namer else if (tpt1.symbol == defn.orType) checkedArgs = checkedArgs.mapconserve(arg => checkSimpleKinded(checkNoWildcard(arg))) - else if (ctx.compilationUnit.isJava) + else if (ctx.isJava) if (tpt1.symbol eq defn.ArrayClass) then checkedArgs match { case List(arg) => @@ -3524,7 +3524,7 @@ class Typer extends Namer if ((pt eq AnyTypeConstructorProto) || tp.typeParamSymbols.isEmpty) tree else { val tp1 = - if (ctx.compilationUnit.isJava) + if (ctx.isJava) // Cook raw type AppliedType(tree.tpe, tp.typeParams.map(Function.const(TypeBounds.empty))) else