[PATCH] D137334: [clang][dataflow] Generalize custom comparison to return tri-value result.

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:52
+/// Indicates the result of a tentative comparison.
+enum class ComparisonStatus {
+  Same,
----------------
Alternative that I slightly prefer - `ComparisonResult`.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:75
+    ///   `Different`:`Val1` is distinct from `Val2`, according to the model.
+    ///   `Unknown`: The model does not determine a relationship between `Val1`
+    ///    and `Val2`.
----------------
Perhaps replace "does not" with "can't"?


================
Comment at: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:365
     if (IsSet1 == nullptr)
-      return true;
+      return IsSet2 ? ComparisonStatus::Same : ComparisonStatus::Different;
 
----------------
Why is it same if `IsSet1` is null and `IsSet2` isn't null?


================
Comment at: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:1185
-
-  bool compareEquivalent(QualType Type, const Value &Val1,
-                         const Environment &Env1, const Value &Val2,
----------------
Why remove this? It's not the same as the default implementation, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137334