C++: Make asExpr return AggregateLiterals
#228
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
… instanceof ClassAggregateLiteral' holds for this new node subclass.
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 solves the same problem as #227, but unlike that PR this one:
Instead of creating a new kind of SSA read, and a new
DataFlow::Nodebranch, this PR makesnode.asExpr() instanceof ClassAggregateLiteralhold whennodeis the post-update node generated from the last field write in the class aggregate initialization. This is semantically the perfect node to pick since it does represent the fully initialized aggregate. So I'm pretty happy with that!However, it relies on the presence of post-update nodes when initializing aggregates. This holds for class aggregate literals (since they are represented as a sequence of field writes), but it doesn't work for array aggregates (since we don't model array writes using post-update nodes).
I think the solution is to go with this approach and then I'll work on actually modeling array writes as using post-update nodes since this is something I've been wanting to do for a while anyway since it'll give us better precision in dataflow.
TLDR @bdrodes: This is the PR that I want to upstream. It works for class aggregate literals (which is where you brought up this issue to me again), but it doesn't work for array aggregate literals (which was the original issue you brought to my attention earl last year which I recorded in https://github.com/github/codeql-c-team/issues/2153).