[PATCH] D70150: [analyzer] Don't clean up dead symbols from constraints twice.

[Artem Dergachev via Phabricator via cfe-commits]

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


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:763-764
     // generate a transition to that state.
     ProgramStateRef CleanedCheckerSt =
         StateMgr.getPersistentStateWithGDM(CleanedState, CheckerState);
     Bldr.generateNode(DiagnosticStmt, I, CleanedCheckerSt, &cleanupTag, K);
----------------
NoQ wrote:
> Note: The results of the first invocation are discarded here, as the updated state is getting frankensteined by attaching `ExprEngine`'s environment and store to checker's GDM, while range constraints also reside in the GDM.
This whole frankensteining process is kinda necessary because `SymbolReaper` needs to be populated with the data from the Environment and the Store. I.e., they're supposed to issue their `markLive()` calls, otherwise we wouldn't be able to properly judge whether something is live in `checkDeadSymbols`; but at the same time the intent is to provide the uncleaned state to the callback, so that the checkers had access to full information about the dying symbol.

I'm not sure whether any checkers actually take advantage of such information, but the intent to provide this information looks valid, so i don't plan to undo this decision.


Repository:
  rC Clang

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

https://reviews.llvm.org/D70150