xazax.hun added inline comments.
================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:68
+/// `LatticeT` can optionally provide the following members:
+/// * `bool widen(const LatticeT &Previous)` - chooses a lattice element that
+/// approximates the join of this element with the argument. Widen is
called
----------------
We should document what is the return value for. Also I see `LatticeJoinEffect`
instead of bool in the code, but I might be confused.
================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:108
+ LatticeJoinEffect widenTypeErased(TypeErasedLattice &Current,
+ const TypeErasedLattice &Previous) final {
----------------
I wonder if `LatticeJoinEffect` is the right name if we also use this for
widening. (Also, are we in the process of removing these?)
================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:140
+ ///
+ /// `Prev` must precede `Current` in the value ordering.
+ ///
----------------
First, I was confused, because I thought they could be equivalent as well. But
I guess this code is only called for values that are not considered equivalent.
Documenting this is really useful.
================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:374-378
+ Environment WidenedEnv(*DACtx);
+
+ WidenedEnv.CallStack = CallStack;
+ WidenedEnv.ReturnLoc = ReturnLoc;
+ WidenedEnv.ThisPointeeLoc = ThisPointeeLoc;
----------------
Shouldn't we have a simple copy ctor from PrefEnv that could populate all these
fields?
Also, this makes me wonder if we actually want to move some of these out from
environment, since these presumably would not change between basic blocks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137948/new/
https://reviews.llvm.org/D137948
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
