[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D108695#2967540 <https://reviews.llvm.org/D108695#2967540>, @NoQ wrote:

> Would this work correctly when the property is changed but then reverted to its original state? This probably can't happen to MallocChecker (what has been freed cannot be unfreed) but it may happen to eg. PthreadLockChecker or to, well, Stores. In such cases it would be incorrect to say "returning without changing the state".

It should! But you have a point, I don't have the code to prove it right away. Maybe if I factored out the symbol part into NoStateChangeToSymbolVisitor, as teased in D105553#2864318 <https://reviews.llvm.org/D105553#2864318>, it'd have an easier time to see it. With that said, I don't currently see how the current implementation would be faulty, but even if it is, the patch adds an option, and doesn't the take old one (that NoStoreFuncVisitor still uses) away.



================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:370
+  while (N && !N->getLocationAs<CallExitEnd>())
+    N = N->getFirstSucc();
+  return N;
----------------
NoQ wrote:
> This is the right successor because we're in a heavily trimmed exploded graph, right?
Should be, yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108695