Skip to content

Commit

Permalink
Improve handling of references to Object coming from Java code
Browse files Browse the repository at this point in the history
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
scala/scala#7966 (which was refined in
scala/scala#8049 and
scala/scala#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 scala#7467, scala#7963, scala#8588, scala#8977.
  • Loading branch information
smarter committed Aug 21, 2020
1 parent 2a86ad0 commit 8f7df67
Show file tree
Hide file tree
Showing 31 changed files with 326 additions and 91 deletions.
99 changes: 98 additions & 1 deletion compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/core/StdNames.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
15 changes: 10 additions & 5 deletions compiler/src/dotty/tools/dotc/core/TypeApplications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
* <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
Expand Down
30 changes: 7 additions & 23 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
27 changes: 12 additions & 15 deletions compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
/*
Expand Down Expand Up @@ -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))
}

Expand Down
16 changes: 12 additions & 4 deletions compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 8f7df67

Please sign in to comment.