C++: Support access paths for flow sources#22113
Draft
MathiasVP wants to merge 5 commits into
Draft
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.
This PR adds the ability to specify sources with access paths in MaD. I've shamelessly stolen some of the code from #18298, but there are some problems that I can't seem to find a solution to. The code in 717a846 is (to the best of my knowledge) a clean translation from Rust to C++. However, the
.expectedfiles shows some issues that I've tried to solve in 15f09e8, but I really don't like this "solution". I'm not sure if this ever comes up in Rust.@hvitved would you mind taking a look at this draft and help me address these two (probably related) problems I'm facing:
The first problem happens when I change
sourceNodefrom:to:
this has the effect of changing all the MaD sources that specify output arguments as sources from the post-update node of the argument (i.e.,
ReadFile output argument) tocall to ReadFile(which is then followed by aFlowSummaryImpl::Private::Steps::sourceLocalStepstep to the output argument). However, this makes flow paths look really odd since the source isn't the return value of the function. You can see this happening in c2f7bf1.To work around this in 15f09e8 I hacked the one extension of
SourceElementto only be satisfied for models for which we have a non-trivialoutputcolumn. But, as I said, I really don't like this approach.The second problem (which may be related to the first one) can be seen from the missing result in c2f7bf1. This happens because I chose
SourceBaseto beCalls (or rather: the only extension I provided were calls). However, if there are no calls to the function in a database, but we still want to specify that a parameter of the uncalled function is a source then we don't have anyCallthat serves as the flow source entry. So this MaD flow source is no longer picked up as a flow source.@hvitved I'd love to know if these problems also exist in Rust and whether I've missed some subtle thing about how Rust does things to make the above work.