[PATCH] D124104: [clang][dataflow] Fix `Environment::join`'s handling of flow condition merging

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.


================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:339-341
   // Move `LocToVal` so that `Environment::ValueModel::merge` can safely assign
   // values to storage locations while this code iterates over the current
   // assignments.
----------------
This comment is no longer relevant, right?


================
Comment at: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:299
 
+// Models an analysis that uses the flow conditions.
+class SpecialBoolAnalysis
----------------



================
Comment at: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:340-344
+    if (!Type->isRecordType())
+      return false;
+    const auto *Decl = Type->getAsCXXRecordDecl();
+    if (Decl->getIdentifier() == nullptr || Decl->getName() != "SpecialBool")
+      return false;
----------------



================
Comment at: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:356
+
+    return V1 == V2 ||
+           Env1.flowConditionImplies(*V1) == Env2.flowConditionImplies(*V2);
----------------
That's guaranteed to be false in `compareEquivalent`.


================
Comment at: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:364-368
+    if (!Type->isRecordType())
+      return true;
+    const auto *Decl = Type->getAsCXXRecordDecl();
+    if (Decl->getIdentifier() == nullptr || Decl->getName() != "SpecialBool")
+      return true;
----------------



================
Comment at: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:414
+      void set();
+    };
+    void target(bool Cond) {
----------------



================
Comment at: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:422
+      } else {
+        Foo.set();
+      }
----------------
Add `/*[[p3]]*/` after this statement and ensure that the value is implied in the "else" branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124104