Skip to content

Improve handling of references to Object coming from Java code #9601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
7 changes: 7 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
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
17 changes: 11 additions & 6 deletions compiler/src/dotty/tools/dotc/core/TypeApplications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
Loading