FlowSummaryImpl: Embed AST nodes in source/sink summary nodes for better locations#22145
Draft
hvitved wants to merge 12 commits into
Draft
FlowSummaryImpl: Embed AST nodes in source/sink summary nodes for better locations#22145hvitved wants to merge 12 commits into
hvitved wants to merge 12 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
hvitved
force-pushed
the
flow-summary-source-sink-locations
branch
from
b24377a to
d2107ce
Compare
github-actions
Bot
added
Rust
hvitved
force-pushed
the
flow-summary-source-sink-locations
branch
from
d2107ce to
d397063
Compare
hvitved
force-pushed
the
flow-summary-source-sink-locations
branch
from
d397063 to
6d8870b
Compare
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
2 participants
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 addresses the issues identified by @MathiasVP on #22113.
Note that the issue (and fix) only applies to Rust, which is the only language that currently supports source/sink definitions with non-empty access paths.
The underlying issue was that we were unable to map source/sink summary nodes to the locations represented by the output/input models-as-data specs, so for example a source with an
Argument[0]spec would be unable to be mapped to the corresponding argument, and we instead resorted to using the location of the call as the source location. The second issue identified by@MathiasVP, defining a source that is supposed to be a parameter, was not even supported in Rust.The fix to both issues is to embed language-specific AST nodes into the source/sink summary nodes, and then use the locations of those as the locations of the source/sink nodes. This PR also shows how to add support for parameter sources.
Commit-by-commit review is strongly encouraged, and the second commit should be reviewed ignoring whitespaces.