sgatev added inline comments.
================ Comment at: clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp:9 +// +// This file defines a simplistic version of Constant Propagation as an example +// of a forward, monotonic dataflow analysis. The analysis tracks all ---------------- Indent two spaces. ================ Comment at: clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp:160 + : ValueLattice::top(); + return Vars; + } ---------------- Consider removing this `return` and moving the assignment that follows in an `else` block. I think this way the two cases (with and without initializer) will be more apparent. ================ Comment at: clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp:162 + } + Vars[Var] = ValueLattice::top(); + return Vars; ---------------- I believe this should be `bottom` as `ValueLattice::bottom` models an undefined value. ================ Comment at: clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp:171-173 + Vars[Var] = (E->EvaluateAsInt(R, Context) && R.Val.isInt()) + ? ValueLattice(R.Val.getInt().getExtValue()) + : ValueLattice::top(); ---------------- I think this makes the transfer function non-monotonic. Concretely, `Vars[var]` can be `top` and the right-hand side can be something else. The transfer function must be monotonic to guarantee termination. Perhaps this should be `Vars[Var].join(...)`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116370/new/ https://reviews.llvm.org/D116370 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
