-
Notifications
You must be signed in to change notification settings - Fork 3.1k
ObjectTpeJava is truly unique; bounds; isAnyTpe #8049
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
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 = ObjectTpeandhi' = ObjectTpe',unique'ing (hashconsing) such aTypeBoundswould indirectlyreplace
ObjectTpewithObjectTpeJavaor vice versa.We can't use
eqwhen trying to identifyObjectTpeJavainuniquedue to cycles.
isAnyTpereturnstrueforObjectTpeJava.Further fixes by Lukas:
Reduce
_ >: lo <: ObjectTpeJavatolo. This eliminates theBoundedWildcardTypein the expected type when type-checking thelambda
where
Follow up for #7966
Fix scala/bug#11525