Nullability Analysis without NotNull #7556
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
Also, doc comments for Nullables.scala
Also recognize pure operations in Any and Object
Previously we simplified an `if c then a else b` to `a` or `b` only if `c` is literally `true` or `false`. We now do the same if `c` is a pure Boolean constant expression.
We keep the type (instead of retyping it) if - the function and argumens of an application are the same, - the application is a pure constant expression. It's a shame to lose the type in this case. This does constrain what can be done in transforms: It's forbidden to change a pure function with a constant type so that the application no longer has that type.
This is a temporary commit so that we can test the nullability analysis without having the full implementation of explicit nulls. Once we have explicit nulls, we could consider dropping this, as the benefits seem smallish.
Not computing it after typer (as was done before) fails Ycheck.
Constant folding null comparisons cannot be reproduced after pickling since notNullRefs are not maintained top-down. We should drop constant folding null comparisons once full explicit nulls are in. for full explicit nulles, we need to wrap references in "assert not null" calls to keep a record of what was inferred.
They don't link if assertFail() has Nothing as result type.
This is needed to support mutable variables. It required a fundamental change in data structures. Now, we keep two sets where before we kept one: The set of references known to be not null, and the set of references that are retracted, so that they can be null again.
Generalize not null computations so that any side effects in conditions if if and while expressions are accounted for.
NotNull is a common supertype of AnyRef and AnyVal. It will be interpreted specially in type comparisons.
When computing the type for local type inferennce, use a simplify transformation.
Makes use of the fact that `Null & NotNull = Nothing`. This fact is used in both type comparisons and glb operations.
For now, we treat _every_ type spelled "Null" as the Null type. That way we can introduce a user-level Null that sits next to AnyVal and AnyRef, like the real Null will once explicit nulls are in. This hack should be reverted once we have the changes for Null implemented.
There seems to be a missing simplification when compiling from Tasty. Here is the error message: ``` *** error while checking out/posTestFromTasty/pos/notNull/Test.class after phase readTasty *** java.lang.AssertionError: assertion failed: Types differ Original type : (x: Int): Int After checking: (x: (Int | Null) & NotNull): (Int | Null) & NotNull Original tree : identity[(Int | Null) & NotNull] After checking: identity[(Int | Null) & NotNull] ```
This reverts commit 27c92a9.
Better document Nullables.scala. Also, rename containsRef to impliesNotNull.
Add a method $nn to Any that has type
def $nn: this.type & NotNull
The method is marked as stable & realizable, which means it can appear in paths.
Special handling is required in asSeenFrom, to reflect the fact that $nn are not
real selections, i.e. the selected item has the same owner as the one in the prefix.
N.B. I tried to make $nn a field instead, but this caused AbstractMethod errors
for reasons I cannot quite track down.
dottybot
reviewed
Nov 14, 2019
There was a problem hiding this comment.
Hello, and thank you for opening this PR! 🎉
All contributors have signed the CLA, thank you! ❤️
Commit Messages
We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).
Please stick to these guidelines for commit messages:
- Separate subject from body with a blank line
- When fixing an issue, start your commit message with
Fix #<ISSUE-NBR>:- Limit the subject line to 72 characters
- Capitalize the subject line
- Do not end the subject line with a period
- Use the imperative mood in the subject line ("Add" instead of "Added")
- Wrap the body at 80 characters
- Use the body to explain what and why vs. how
adapted from https://chris.beams.io/posts/git-commit
Have an awesome day! ☀️
Closed
sjrd
force-pushed
the
add-flow-analysis-without-notnull
branch
from
b35fa49 to
ba6e0e5
Compare
odersky
reviewed
Nov 14, 2019
| class C { type T } | ||
| locally { | ||
| val x: C { type T = Int } = new C { type T = Int } | ||
| val y: x.$nn.T = 33 | ||
| val xnn: x.type & C { type T = Int } = notNull(x) |
There was a problem hiding this comment.
The problem is that the compiler will insert $nn on a path, and that the path can appear in a type or a term. I don't see how that could work with the revised version of notNull, since that wrapping would be no longer a path.
There was a problem hiding this comment.
As I explained at #6344 (comment), I don't think the use in paths is a real issue. We can get away without supporting it.
There was a problem hiding this comment.
I put my answer on that thread as well, since it covered several earlier remarks. #6344 (comment).
With 4 PRs in flight it is getting difficult to origanize comments!
odersky
approved these changes
Nov 15, 2019
| @@ -1855,7 +1838,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w | |||
| else if (!tp2.exists) tp1 | |||
| else tp.derivedAndType(tp1, tp2) | |||
|
|
|||
| /** If some (&-operand of) `tp` is a supertype of `sub` replace it with `NoType`. | |||
There was a problem hiding this comment.
This fix was unrelated to NotNull and should be kept.
There was a problem hiding this comment.
I reverted the reversion :)
|
After more discussion, I agree that it's better not to introduce |
sjrd
force-pushed
the
add-flow-analysis-without-notnull
branch
from
ba6e0e5 to
016f471
Compare
noti0na1
added a commit
to noti0na1/dotty
that referenced
this pull request
Nov 15, 2019
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.
This is a variant of #7520 without any definition of
NotNull, as I believe we can and should implement the whole explicit-nulls thing withoutNotNull. See the discussion starting at #6344 (comment)./cc @abeln @odersky