Skip to content

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 5 commits into from
Aug 25, 2024

Conversation

sjrd
Copy link
Member

@sjrd sjrd commented Aug 22, 2024

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.


ClassTypes and ArrayTypes 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 PreTransforms. We now replace that tracking by the actual tpe of the trees.

In order to be type-preserving, we must now insert most Casts, 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 Casts 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 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.

@sjrd sjrd force-pushed the nullable-types-in-ir branch 4 times, most recently from 972effe to 6c70f63 Compare August 22, 2024 20:44
@sjrd sjrd marked this pull request as ready for review August 22, 2024 20:46
@sjrd sjrd requested a review from gzm0 August 22, 2024 20:46
@sjrd sjrd changed the title CI ONLY Introduce non-nullable reference types in the IR. Introduce non-nullable reference types in the IR. Aug 22, 2024
sjrd added 3 commits August 25, 2024 11:27
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.
@sjrd sjrd force-pushed the nullable-types-in-ir branch from 869cda6 to eddb0a9 Compare August 25, 2024 09:27
@sjrd
Copy link
Member Author

sjrd commented Aug 25, 2024

Rebased.

Copy link
Contributor

@gzm0 gzm0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@sjrd sjrd force-pushed the nullable-types-in-ir branch from eddb0a9 to 22b69c8 Compare August 25, 2024 14:22
Copy link
Member Author

@sjrd sjrd left a 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.

`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.
@sjrd sjrd force-pushed the nullable-types-in-ir branch from 8e3da53 to 043cfc8 Compare August 25, 2024 17:31
For `This` nodes and `IsInstanceOf` nodes.

We only active the relevant deserialization hacks when reading IR
from before 1.17.
@sjrd
Copy link
Member Author

sjrd commented Aug 25, 2024

(adjusted MiMa exclusions)

@sjrd sjrd merged commit 20874df into scala-js:main Aug 25, 2024
3 checks passed
@sjrd sjrd deleted the nullable-types-in-ir branch August 25, 2024 21:26
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants