-
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
Conversation
It looks like this PR was initiated on top of the wrong branch. Please rebase on top of |
|| 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
.
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
Closed as subsumed by #23938 |
…llification rules (#23938) Given the nightly results, we think the nullification rules still need some polish and is not ready for all compiled Scala code. Hence, we only nullify tasty if `Ynullify-tasty` is set now. Based on #23911 Fixes #23908 Could fix #23933, #23935, #23936, and #23937 (I tested locally due the explicit nulls tests disabled temprarily)
More tests may be needed
Fixes #23908