[PATCH] D130270: [clang][dataflow] Use a dedicated bool to encode which branch was taken

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.


================
Comment at: clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp:439
   Context.addFlowConditionConstraint(FC2, Y);
-  // JoinedFC = (FC1 || FC2) && Z = (X || Y) && Z
-  auto &JoinedFC = Context.joinFlowConditions(FC1, FC2);
+  // JoinedFC = ((!B && FC1) || (B && FC2)) && Z = ((!B && X) || (B && Y)) && Z
+  auto &B = Context.createAtomicBoolValue();
----------------
I'm a bit concerned by the need to update this (essentially) unrelated test. It implies the test is depending on implementation details that it shouldn't be concerned with.

Not for this patch, but if you have thoughts on how to simplify this test to do what it needs w/o being tied to the details of how joins work, that would be appreciated. At least, it seems worth a FIXME.

thanks


================
Comment at: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:1202
+
+        EXPECT_FALSE(Env.flowConditionImplies(FooVal));
+      });
----------------
should we also test the inverse?
```
EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
```
That is, I would think we want to show that neither is provable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130270