NPECheck cleanup: removing weird hypothetical states, and removing instanceof states.#9450
Open
lahodaj wants to merge 1 commit into
Open
NPECheck cleanup: removing weird hypothetical states, and removing instanceof states.#9450lahodaj wants to merge 1 commit into
lahodaj wants to merge 1 commit into
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
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.
There's a
nullchecker injava/java.hints,NPECheck. I am considering some updates to it, and, as a preparation for that, I was looking at the currentStates there. There are some weird ones:NULL_HYPOTHETICALandNOT_NULL_HYPOTHETICAL- these are meant to cover cases like:the hypothetical states were basically used as "transient state" in expressions. I think that with the experience from pattern matching, this is overly complex: I think we can simply keep ordinary
NULL/NOT_NULLstates, but keep them "when true" and "when false", and then use and merge them accordingly. This also avoids the need to run conditions twice (once for ordinary run, and once for a negated one). Overall, this should make stuff simpler.INSTANCE_OF_FALSEandINSTANCE_OF_TRUE. I think the only real thing this was doing is code along these lines (or conversely the same with anif):Normally, the NPECheck recognizes several "possible nulls", in particular
POSSIBLE_NULL, which does not produce any warning, andPOSSIBLE_NULL_REPORT, which is used if there's a confirmation the variable might be null, like if there's a test for null for the variable, and which produces the "possibly dereferencing nullwarning. The instanceof states are used to make theinstanceofwork as a test for null, but, frankly, it is not: we have no firm idea whetherocan or cannot benullif theinstanceoffails. I.e. the behavior ofPOSSIBLE_NULLis more in order here, not thePOSSIBLE_NULL_REPORT`. So, this PR is stripping the instanceof states, and won't produce a warning in the above case.^Add meaningful description above
Click to collapse/expand PR instructions
By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -
Please make sure (eg.
git log) that all commits have a valid name and email address for you in the Author field.If you're a first time contributor, see the Contributing guidelines for more information.
If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.
PR approval and merge checklist:
If this PR targets the delivery branch: don't merge. (full wiki article)