[PATCH] D47416: [analyzer] Clean up the program state map of DanglingInternalBufferChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:92-94
+  if (!SymReaper.hasDeadSymbols())
+    return;
+
----------------
This optimization is in fact incorrect due to an old bug that i didn't yet get to fixing: D18860. The proposed patch would most likely remove the check anyway, because the set of dead symbols is not well-defined. So i think we shouldn't add it.


================
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:98
+  for (const auto Entry : RPM) {
+    if (SymReaper.isDead(Entry.second))
+      State = State->remove<RawPtrMap>(Entry.first);
----------------
For the same reason (D18860), it should be more correct to use `!isLive()`. Otherwise you may miss some symbols.


================
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:100-103
+    if (!SymReaper.isLiveRegion(Entry.first))
+      // Due to incomplete destructor support, some dead regions might still
+      // remain in the program state map. Clean them up.
+      State = State->remove<RawPtrMap>(Entry.first);
----------------
I mildly advocate for braces here because that's as many as three lines to wrap.


https://reviews.llvm.org/D47416