[PATCH] D135397: [clang][dataflow] Add support for a Top value in boolean formulas.

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:156
 
+  TopBoolValue &createTopBoolValue() {
+    return takeOwnership(std::make_unique<TopBoolValue>());
----------------
Please add a comment.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:291
 
+  BoolValue &makeTopBoolValue() const {
+    return DACtx->createTopBoolValue();
----------------
Please add a comment.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/Value.h:100
+public:
+  explicit TopBoolValue() : BoolValue(Kind::TopBool) {}
+
----------------
`explicit` seems unnecessary.


================
Comment at: clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp:237-238
+      case Value::Kind::TopBool:
+        // Nothing more to do. Each `TopBool` instance is mapped to a fresh
+        // variable.
+        break;
----------------
Where? Does that mean that `TopBool` never reaches the solver? I think it'd be good to clarify.


================
Comment at: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:1220
+      (void)0;
+      /*[[p2]]*/
+    }
----------------
Why do we need to check two code points here and in the test below? It's not obvious what the difference between `p1` and `p2` is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135397