[PATCH] D124943: [clang][dataflow] Add flowConditionIsTautology function

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:176-177
 
+  /// Returns true if the constraints of the flow condition identified by
+  /// `Token` is always true.
+  bool flowConditionIsTautology(AtomicBoolValue &Token);
----------------



================
Comment at: clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp:144
+TEST_F(DataflowAnalysisContextTest, FlowConditionTautologies) {
+  // Default flow condition with empty/no constraints is always true.
+  auto &FC1 = Context.makeFlowConditionToken();
----------------



================
Comment at: clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp:151
+  Context.addFlowConditionConstraint(FC2,
+                                     Context.getOrCreateNegationValue(FC1));
+  EXPECT_FALSE(Context.flowConditionIsTautology(FC2));
----------------
I don't think this is doing what the comment above it says. We probably shouldn't add flow conditions directly as constraints because this isn't tracking flow condition dependencies. I suggest removing this case and keeping the one with `FC5`.


================
Comment at: clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp:154
+
+  // FC1 || !FC1 is always true.
+  EXPECT_TRUE(
----------------
Again, I suggest removing this case. The one with `FC6` should suffice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124943