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

Tue May 26 10:17:22 PDT 2020

baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:112

+Optional<SVal> ExprEngine::retrieveFromConstructionContext(
+    ProgramStateRef State, const LocationContext *LCtx,
----------------
NoQ wrote:
> baloghadamsoftware wrote:
> > NoQ wrote:
> > > Please instead re-use the code that computes the object under construction. That'll save you ~50 lines of code and will be more future-proof (eg., standalone temporaries without destructor technically have a construction context with 0 items so when we implement them correctly your procedure will stop working).
> > That was so my first thought. However, `handleConstructionContext()` is private and non-static. Now I tried to merge the two methods: if the value is already in the construction context, we return it, if not then we add it. Is this what you suggest? Or did I misunderstand you? At the very beginning I tried to simply use `handleConstructionContext()`, but it asserted because the value was already in the map.
> I'd like to preserve the assertion that objects-under-construction are never filled twice; it's a very useful sanity check. What you need in your checker is a function that computes object-under-construction but doesn't put it into the objects-under-construction map. So you have to separate the computation from filling in the state.
OK, so I (fortunately) misundertood you. Thus I should refactor this function to a calculation and a storing part?

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80366/new/

https://reviews.llvm.org/D80366