[PATCH] D126560: [analyzer][NFC] SimpleSValBuilder simplification: Remove superfluous workaround code and track Assume call stack rather

[Balázs Benics via Phabricator via cfe-commits]

steakhal accepted this revision.
steakhal added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:21
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/SaveAndRestore.h"
----------------
Unused.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:140-146
   /// A flag to indicate that clients should be notified of assumptions.
   /// By default this is the case, but sometimes this needs to be restricted
   /// to avoid infinite recursions within the ConstraintManager.
   ///
   /// Note that this flag allows the ConstraintManager to be re-entrant,
   /// but not thread-safe.
   bool NotifyAssumeClients = true;
----------------
martong wrote:
> This hunk is probably also superfluous. With tracking the assume stack below, it should be safe to call `assume` even from `evalAssume`. But, I'd rather remove that in a subsequent patch, because `checkNull`s behavior is dependent on that.
Oh, I haven't thought about that. Thanks. Feel free to do in a followup!


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:154
+    bool contains(const ProgramState *S) const {
+      return llvm::find(Aux, S) != Aux.end();
+    }
----------------
Use `llvm::is_contained()` instead.


================
Comment at: clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp:60
+  AssumeStack.push(RawSt);
+  auto AssumeStackBuilder =
+      llvm::make_scope_exit([this]() { AssumeStack.pop(); });
----------------
Don't we call these 'Guards'?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126560