Expose some cfg/builder APIs to allow inserting jumps and throws. #5156
Conversation
This file contains 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
|
Didn't want to make this part of the PR description, but for some context on how these APIs are being used, please see uber/NullAway#608. |
lazaroclapp
commented
Jun 8, 2022
| * <li><em>UNCONDITIONAL_JUMP</em>: {@link CFGBuilder.UnconditionalJump}. An unconditional jump to | ||
| * a label. | ||
| * <li><em>TWO_TARGET_CONDITIONAL_JUMP</em>: {@link CFGBuilder.ConditionalJump}. A conditional | ||
| * jump with two targets for both the 'then' and 'else' branch. |
There was a problem hiding this comment.
References (@link) in these docs were outdated, which tooling complained about as soon as the class was made public. Otherwise this is unchanged.
lazaroclapp
commented
Jun 8, 2022
| */ | ||
| protected ProcessingEnvironment getProcessingEnvironment() { | ||
| return env; | ||
| } |
There was a problem hiding this comment.
We technically don't need this one, but it's useful and avoids us keeping a duplicate ProcessingEnvironment field in the subclass.
smillst
previously approved these changes
Jun 9, 2022
There was a problem hiding this comment.
This looks good, thanks! The tests are failing because you haven't contributed before and aren't listed in contributors.tex . The error is:
diff --git a/docs/manual/contributors.tex b/docs/manual/contributors.tex
index ed108fe82..2ef619ceb 100644
--- a/docs/manual/contributors.tex
+++ b/docs/manual/contributors.tex
@@ -57,6 +57,7 @@ Junhao Hu,
Kartikeya Goswami,
Kivanc Muslu,
Konstantin Weitz,
+Lazaro Clapp,
Leo Liu,
Liam Miller-Cushon,
Lian Sun,
+ set +x
docs/manual/contributors.tex is not up to date.
If the above suggestion is appropriate, run: make -C docs/manual contributors.tex
If the suggestion contains a username rather than a human name, then do all the following:
* Update your git configuration by running: git config --global user.name "YOURFULLNAME"
* Add your name to your GitHub account profile at https://github.com/settings/profile
* Make a pull request to add your GitHub ID to
https://github.com/plume-lib/plume-scripts/blob/master/git-authors.sed
and remake contributors.tex after that pull request is merged.
smillst
approved these changes
Jun 10, 2022
|
Once plume-lib/plume-scripts#19 lands I think the CI job will be fixed here |
lazaroclapp
added a commit
to uber/NullAway
that referenced
this pull request
Jun 16, 2022
Needed for our changes in #608, which requires visibility into some previously `package` or `private` level internals of Checker Framework's CFG: - typetools/checker-framework#5152 - typetools/checker-framework#5156
lazaroclapp
added a commit
to uber/NullAway
that referenced
this pull request
Jun 17, 2022
…ation (#608) See #47 for discussion. This PR adds support for `Preconditions.checkArgument(...)` and `Preconditions.checkState(...)` throw a new `PreconditionsHandler` handler. In order to implement said handler, we need a custom CFG translation pipeline in NullAway (`NullAwayCFGBuilder`) which subclasses the Checker Framework's `CFGBuilder` (which we were using directly before this change). This pipeline allows us to add handler callbacks during the AST to CFG translation process. At the moment, we add a single such callback, right after visiting a `MethodInvocationTree`. We also add a more or less generic method to insert a conditional jump and a throw based on a given boolean expression node. Abstracting some details about our AST and CFG structures, we translate: ``` Preconditions.checkArgument($someBoolExpr[, $someString]); ``` Into something resembling: ``` Preconditions.checkArgument($someBoolExpr[, $someString]); if ($someBoolExpr) { throw new IllegalArgumentException(); } ``` Note that this causes `$someBoolExpr` to be evaluated twice. This is necessary based on how NullAway evaluates branch conditionals, since we currently do not track boolean values through our dataflow (e.g. `boolean b = (o == null); if (b) { throw [...] }; o.toString();` produces a dereference according to NullAway, independent on whether that code was added by rewriting or directly on the source. Obviously, this doesn't change the code being compiled or bytecode being produced, the rewrite is only ever used by/visible to NullAway itself. Finally, our implementation of `NullAwayCFGBuilder` and particularly `NullAwayCFGTranslationPhaseOne` in this PR, depend on a Checker Framework APIs that are currently private. We are simultaneously submitting a PR to change the visibility of these APIs (see [CheckerFramework#5156](typetools/checker-framework#5156))
wmdietl
pushed a commit
to eisop/checker-framework
that referenced
this pull request
Jul 13, 2022
msridhar
added a commit
that referenced
this pull request
Aug 14, 2023
In #5955, the `ExtendedNode` and `Label` classes from the dataflow framework were changed from public to package-private. However, NullAway relies on these classes being public (see #5156, #5152, and uber/NullAway#608). This PR makes the classes public again and adds an explanatory comment.
wmdietl
pushed a commit
to eisop/checker-framework
that referenced
this pull request
Oct 16, 2023
In typetools#5955, the `ExtendedNode` and `Label` classes from the dataflow framework were changed from public to package-private. However, NullAway relies on these classes being public (see typetools#5156, typetools#5152, and uber/NullAway#608). This PR makes the classes public again and adds an explanatory comment.
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 PR exposes a few previously package-private classes inside
org.checkerframework.dataflow.cfg.builder, as well as making a few private methodsof
CFGTranslationPhaseOne.javaprotected instead.We would like to have visibility into these classes and methods in order to implement a solution
to the
Preconditions.checkArgument(...)translation issue described in #5151. This is a follow upto #5152.