baloghadamsoftware marked 2 inline comments as done. baloghadamsoftware added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:575 + std::tie(State, RetVal) = + static_cast<ExprEngine*>(&Engine)->handleConstructionContext(getOriginExpr(), + getState(), ---------------- This is extremely ugly and was one of the reasons I originally did not reuse `handleConstructionContext()`. What should I do here? Create a pure virtual `handleConstructionContext()` into `SubEngine`? Or somehow make `handleConstructionContext` static? Is there maybe a more proper way to access `ExprEngine` from `CallEvent`? ================ Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:112 +Optional<SVal> ExprEngine::retrieveFromConstructionContext( + ProgramStateRef State, const LocationContext *LCtx, ---------------- 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. 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
