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
ObjectTpeJava is truly unique; bounds; isAnyTpe #8049
Conversation
Strawman fix. Not sure this is ok for performance, but the test reliably shows that if we don't do something like this,
|
This fix exposed another bug in my attempt at this: |
This comment has been minimized.
This comment has been minimized.
Minimisation of the latest mole:
|
68b6b62
to
0122f31
Compare
if (tp.typeSymbol == AnyClass) TypeBounds.empty | ||
else TypeBounds.lower(tp) | ||
case '*' => TypeBounds.empty | ||
if (tp.typeSymbol == AnyClass) TypeBounds.upper(definitions.ObjectTpeJava) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add an example for the importance of distinguishing AnyTpe
and ObjectTpeJava
here?
Make sure the TypeRef is truly unique, even when embedded in other types, such as a TypeBounds. Since we're dealing with structural equality, `TypeBounds(lo, hi) == TypeBounds(lo', hi')` if `lo == lo'` and `hi == hi'`, if `hi = ObjectTpe` and `hi' = ObjectTpe'`, `unique`'ing (hashconsing) such a `TypeBounds` would indirectly replace `ObjectTpe` with `ObjectTpeJava` or vice versa. We can't use `eq` when trying to identify `ObjectTpeJava` in `unique` due to cycles. Further fixes by Lukas: - Don't show ObjectTpeJava upper bound in TypeBounds.toString - Upper bounds of wildcards should be ObjectTpeJava - Fix wildcardExtrapolation for BoundedWildcardType with hi ObjectTpeJava Reduce `_ >: lo <: ObjectTpeJava` to `lo`. This eliminates the `BoundedWildcardType` in the expected type when type-checking the lambda (c: java.util.Collection[String]).removeIf(x => true) where boolean removeIf(Predicate<? super E> filter)
/synch |
1 similar comment
/synch |
The merge of scala/scala#8049 to 2.13.x
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.
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.
Make sure the TypeRef is truly unique, even when embedded
in other types, such as a TypeBounds. Since we're dealing
with structural equality,
TypeBounds(lo, hi) == TypeBounds(lo', hi')
if
lo == lo'
andhi == hi'
, ifhi = ObjectTpe
andhi' = ObjectTpe'
,unique
'ing (hashconsing) such aTypeBounds
would indirectlyreplace
ObjectTpe
withObjectTpeJava
or vice versa.We can't use
eq
when trying to identifyObjectTpeJava
inunique
due to cycles.
isAnyTpe
returnstrue
forObjectTpeJava
.Further fixes by Lukas:
Reduce
_ >: lo <: ObjectTpeJava
tolo
. This eliminates theBoundedWildcardType
in the expected type when type-checking thelambda
where
Follow up for #7966
Fix scala/bug#11525