-
Notifications
You must be signed in to change notification settings - Fork 396
Introduce non-nullable reference types in the IR. #5018
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
972effe
to
6c70f63
Compare
If we do, we generate code that statically refers to the array type `void[]`, which is not a valid type. With the following commit, the optimizer will have enough knowledge to enter in this case when optimizing `ClassTag`s, resulting in ClassDef checking errors after the optimizer.
869cda6
to
eddb0a9
Compare
Rebased. |
gzm0
requested changes
Aug 25, 2024
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.
Nice!
linker/shared/src/main/scala/org/scalajs/linker/backend/wasmemitter/TypeTransformer.scala
Outdated
Show resolved
Hide resolved
linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala
Show resolved
Hide resolved
linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala
Show resolved
Hide resolved
linker/shared/src/main/scala/org/scalajs/linker/checker/ClassDefChecker.scala
Outdated
Show resolved
Hide resolved
eddb0a9
to
22b69c8
Compare
sjrd
commented
Aug 25, 2024
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.
I believe I have addressed all the comments.
linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala
Show resolved
Hide resolved
gzm0
requested changes
Aug 25, 2024
linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala
Show resolved
Hide resolved
gzm0
approved these changes
Aug 25, 2024
`ClassType`s and `ArrayType`s now carry a `nullable: Boolean` flag. When `nullable` is false, the type does not admit `null` values. We also introduce `AnyNotNullType` to be the non-nullable variant of `AnyType`. This way, every non-void type `tpe` has a non-nullable variant, which we can get with `tpe.toNonNullable`. There are a few sources of non-nullable values, such as `New` nodes, and, most importantly, `This` nodes. Since non-nullable reference types have no corresponding `TypeRef`, they cannot appear in the signature of methods. They only live locally within method bodies. --- The optimizer already had dedicated tracking of non-nullable `PreTransform`s. We now replace that tracking by the actual `tpe` of the trees. In order to be type-preserving, we must now insert most `Cast`s, to non-nullable types. Before, at the IR level everything became nullable again, so even if the optimizer had determined that a tree could not be null, that did not influence the produced IR. On the plus side, since IR nodes track their nullability, we do not need `AssumeNotNull` anymore. The `Cast`s to non-nullable types achieve the same result in a more consistent way. In fact, the new non-nullable types allow the optimizer to better keep track of nullability, resulting in fewer `$n` calls to check for nulls. This is the main source of code side reduction. --- Since we now have non-nullable reference types, we change `IsInstanceOf` to require non-nullable test types. This makes more sense, since `IsInstanceOf` always answers `false` when the value is `null`. --- We introduce deserialization hacks to: * adapt the type of `This` nodes, and * adapt the test type of `IsInstanceOf` nodes. In this commit, we do not change the compiler nor the hard-coded IR of `jl.Object` yet, in order to test the deserialization hacks.
8e3da53
to
043cfc8
Compare
For `This` nodes and `IsInstanceOf` nodes. We only active the relevant deserialization hacks when reading IR from before 1.17.
(adjusted MiMa exclusions) |
sjrd
added a commit
to dotty-staging/dotty
that referenced
this pull request
Jan 17, 2025
And port the changes that were made to the compiler backend. Most nobably, the changes from the following upstream PRs: * Introduce non-nullable reference types in the IR. scala-js/scala-js#5018 * Opt: Remove useless *Ident indirections in the IR model. scala-js/scala-js#5092 * Merge the IR node This into VarRef, with a magic LocalName. scala-js/scala-js#5090 * Merge several IR nodes into UnaryOp. scala-js/scala-js#5088 * Restrict usages of StoreModule even further. scala-js/scala-js#5059 * Allow Return arg to be a void if the target Labeled is a void. scala-js/scala-js#5074 * Rename NoType to VoidType and print it as "void". scala-js/scala-js#5061
sjrd
added a commit
to dotty-staging/dotty
that referenced
this pull request
Jan 17, 2025
And port the changes that were made to the compiler backend. Most notably, the changes from the following upstream PRs: * Introduce non-nullable reference types in the IR. scala-js/scala-js#5018 * Opt: Remove useless *Ident indirections in the IR model. scala-js/scala-js#5092 * Merge the IR node This into VarRef, with a magic LocalName. scala-js/scala-js#5090 * Merge several IR nodes into UnaryOp. scala-js/scala-js#5088 * Restrict usages of StoreModule even further. scala-js/scala-js#5059 * Allow Return arg to be a void if the target Labeled is a void. scala-js/scala-js#5074 * Rename NoType to VoidType and print it as "void". scala-js/scala-js#5061
sjrd
added a commit
to scala/scala3
that referenced
this pull request
Jan 22, 2025
And port the changes that were made to the compiler backend. Most notably, the changes from the following upstream PRs: * scala-js/scala-js#5018 * scala-js/scala-js#5092 * scala-js/scala-js#5090 * scala-js/scala-js#5088 * scala-js/scala-js#5059 * scala-js/scala-js#5074 * scala-js/scala-js#5061
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #4993 to abuse the optimizer+Wasm backend as validation that the optimizer is type-preserving, including wrt. nullability. Otherwise ready to review. The last 5 commits belong to this PR.ClassType
s andArrayType
s now carry anullable: Boolean
flag. Whennullable
is false, the type does not admitnull
values. We also introduceAnyNotNullType
to be the non-nullable variant ofAnyType
.This way, every non-void type
tpe
has a non-nullable variant, which we can get withtpe.toNonNullable
.There are a few sources of non-nullable values, such as
New
nodes, and, most importantly,This
nodes.Since non-nullable reference types have no corresponding
TypeRef
, they cannot appear in the signature of methods. They only live locally within method bodies.The optimizer already had dedicated tracking of non-nullable
PreTransform
s. We now replace that tracking by the actualtpe
of the trees.In order to be type-preserving, we must now insert most
Cast
s, to non-nullable types. Before, at the IR level everything became nullable again, so even if the optimizer had determined that a tree could not be null, that did not influence the produced IR.On the plus side, since IR nodes track their nullability, we do not need
AssumeNotNull
anymore. TheCast
s to non-nullable types achieve the same result in a more consistent way.In fact, the new non-nullable types allow the optimizer to better keep track of nullability, resulting in fewer
$n
calls to check for nulls. This is the main source of code side reduction.Since we now have non-nullable reference types, we change
IsInstanceOf
to require non-nullable test types. This makes more sense, sinceIsInstanceOf
always answersfalse
when the value isnull
.We introduce deserialization hacks to:
This
nodes, andIsInstanceOf
nodes.In the first major commit, we do not change the compiler nor the hard-coded IR of
jl.Object
yet, in order to test the deserialization hacks.In the last commit, we adapt the compiler and
jl.Object
, and only enable the deserialization hacks when reading IR < 1.17.