Analyse escaping for local vars #4658
Draft
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
…ir defining scope
…defining scope or which variables will escape its defining scope
MBoegers
force-pushed
the
escape-analysis
branch
from
1c83352
to
a38d727
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
rewrite-java/src/test/java/org/openrewrite/java/search/FindEscapingVariablesTest.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/test/java/org/openrewrite/java/search/FindVariableEscapeLocationsTest.java
Outdated
Show resolved
Hide resolved
|
Not specifically related to this PR, but thought that I'd share a bit simpler data flow API example given I too found rewrite-security-frameworks a bit difficult to follow originally. I ended up using the data flow API over in rewrite-feature-flags to great effect. |
|
Thanks @shanman190 for the reference, I'll definitely have a look. I think the Dataflow API needs some introduction material. After I had a breakthrough I'll share some ;) |
…iableEscapeLocationsTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
rewrite-java/src/test/java/org/openrewrite/java/search/FindEscapingVariablesTest.java
Outdated
Show resolved
Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.
What's changed?
I added two visitors that help decide if variables escape and where.
What's your motivation?
When migrating existing code, the question "Is this instance ever leaking its defining scope?" raises from time to time.
Take the migration from
StringBuffertoStringBuilder.StringBuilderis generally in favor, if no synchronization is needed (see JavaDoc StringBuilder).Synchronization is not needed if the instance never escapes its defining scope.
These helping visitors enable such migration by answering the question of escaping vars.
Anything in particular you'd like reviewers to focus on?
Additional ways to leak an instance.
Anyone you would like to review specifically?
Have you considered any alternatives or workarounds?
I considered the Dataflow API, but I am not fluent enough to archive the same..
Any additional context
This visitor and its helper methods will help @mkarg to realize his migrations for the OpenJDK project.
Checklist