gribozavr2 added inline comments.
================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:163
+ BoolValue &Val,
+ llvm::DenseMap<BoolValue *, BoolValue *> &SubstitutionsCache) {
+ auto It = SubstitutionsCache.find(&Val);
----------------
Could you refactor it into a free function? It does not use 'this', seems like.
================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:225-229
+ if (ConstraintsIt != FlowConditionConstraints.end()) {
+ auto &Constraints = *ConstraintsIt->second;
+ return substituteBoolVal(Constraints, SubstitutionsCache);
+ }
+ return getBoolLiteralValue(true);
----------------
It is better to put special cases (early returns) first.
================
Comment at:
clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp:191
+ // Empty flow condition holds regardless of value of boolean X
+ EXPECT_TRUE(Context.flowConditionImplies(FC, FCIndependentOfX));
+}
----------------
Could you do the following: build explicitly a BoolValue that represents the
expected result formula, and then ask the SAT solver to prove equivalence
between the two? That would be a stronger test than merely checking one
implication.
Same in the test below.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128363/new/
https://reviews.llvm.org/D128363
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
