-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Only nullfiy simple kinded types #23911
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
base: main
Are you sure you want to change the base?
Conversation
It looks like this PR was initiated on top of the wrong branch. Please rebase on top of |
// We don't modify unit types. | ||
|| tp.isRef(defn.UnitClass) | ||
// We don't modify `Any` because it's already nullable. | ||
|| tp.isRef(defn.AnyClass) => false | ||
case tp: TypeParamRef if !tp.hasSimpleKind => false |
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.
!tp.hasSimpleKind
can be moved to the first if
together with outermostLevelAlreadyNullable
.
@@ -107,10 +107,13 @@ object ImplicitNullInterop { | |||
case tp: TypeRef if !tp.hasSimpleKind | |||
// We don't modify value types because they're non-nullable even in Java. | |||
|| tp.symbol.isValueClass | |||
|| tp.isRef(defn.NullClass) |
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.
We may also need to consider the following special classes: SingletonClass
, AnyKindClass
, FromJavaObjectSymbol
, etc...
val n1: List[Map[String, Int]] = ??? | ||
val n2 = new N[List]() | ||
val n3 = n2.accept[Any](n1) | ||
|
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.
Let's also test some type lambdas like [X] =>> R
More tests may be needed
Fixes #23908