ymandel accepted this revision.
ymandel added inline comments.
This revision is now accepted and ready to land.
================
Comment at:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:41
+ ///
+ /// `Slvr` must not be null.
+ DataflowAnalysisContext(std::unique_ptr<Solver> Slvr)
----------------
I think `S` would be easier to read.
================
Comment at:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:244-245
+ /// order, will return the same result. If the given boolean values represent
+ /// the
+ // same value, the result will be the value itself.
+ BoolValue &makeAnd(BoolValue &LHS, BoolValue &RHS) {
----------------
================
Comment at:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:253-254
+ /// order, will return the same result. If the given boolean values represent
+ /// the
+ // same value, the result will be the value itself.
+ BoolValue &makeOr(BoolValue &LHS, BoolValue &RHS) {
----------------
================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:34
+
+ auto Res = ConjunctionVals.try_emplace(
+ {&LHS, &RHS},
----------------
Why not use `try_emplace` earlier, to save of one of the lookups?
================
Comment at:
clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp:28-31
+TEST_F(DataflowAnalysisContextTest,
+ GetOrCreateConjunctionValueReturnsSameExprOnSubsequentCalls) {
+ auto &X = Context.createAtomicBoolValue();
+ auto &Y = Context.createAtomicBoolValue();
----------------
maybe add another one which simply tests that two calls to
`createAtomicBoolValue` return distinct values (or even just distinct pointers?)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120711/new/
https://reviews.llvm.org/D120711
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
