Merge pull request #9601 from dotty-staging/java-object-any-5

 Improve handling of references to `Object` coming from Java code

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(

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)
 

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) {}
+   *       <T> 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<Object> arg) {}
+   *       <T> void meth4(scala.List<T> 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) {}
+   *       <T> 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) {}
+   *       <T> 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
 

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      = "<FromJavaObject>"
     final val Product: N             = "Product"
     final val PartialFunction: N     = "PartialFunction"
     final val PrefixType: N          = "PrefixType"

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

@@ -413,24 +413,29 @@ 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.
+   *  `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
    *  <http://cr.openjdk.java.net/~briangoetz/valhalla/sov/02-object-model.html>,
-   *  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

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,35 +1773,15 @@ 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 {
       case formal1 :: rest1 =>
         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
         }

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
      */

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)

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))
       }
 

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)

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)