NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Great, thank you!
================
Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:543
+
+ const StackFrameContext *StackFrame = Call.getCalleeStackFrame(BlockCount);
+ if (!StackFrame)
----------------
baloghadamsoftware wrote:
> NoQ wrote:
> > It is clear that the final return value of `getConstructionContext()` does
> > not depend on the current block count; in fact the information you're
> > trying to obtain is purely syntactic. You probably might as well pass 0
> > here. As i mentioned before, the ideal solution would be to make
> > `CallEvent` carry a `CFGElementRef` to begin with (as a replacement for the
> > origin expr), so that it didn't need to be recovered like this.
> >
> > We should absolutely eliminate the `BlockCount` parameter of
> > `getReturnValueUnderConstruction()` as well. Checker developers shouldn't
> > understand what it is, especially given that it doesn't matter at all which
> > value do you pass.
> OK, I eliminated it. Do we need `BlockCount` for `getParameterLocation()`
> (the other patch)?
We absolutely need `BlockCount` in the other patch because it's part of the
identity of the parameter region.
That said, the API that requires the user to provide `BlockCount` manually is
still terrible and extremely error-prone. Namely, the contract of
`getParameterLocation()` would be "The behavior is undefined unless you provide
the same block count that is there when call happens". Which is a good
indication that we should have stored `BlockCount` in `CallEvent` from the
start.
================
Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:539
+static const ConstructionContext
+*getConstructionContext(const CallEvent &Call) {
+ const StackFrameContext *StackFrame = Call.getCalleeStackFrame(0);
----------------
This looks useful. Let's turn it into a `CallEvent` method as well.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80366/new/
https://reviews.llvm.org/D80366
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
