[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

[Artem Dergachev via Phabricator via cfe-commits]

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