ymandel added a comment.
Just FYI (since you mentioned it in your description): LatticeEffect is
vestigial and should be removed. It's now only used (properly) for widening.
Regarding the design. This looks like an optimal solution in terms of copying
but introduces lifetime risks and complexity that I'm uncomfortable with.
There's a lot of flexibility here and I wonder if we can reduce that without
(substantially?) impacting the amount of copying. Specifically, I wonder if we
can have the builder *always* own an environment, which simplifies the handling
of `CurrentState` and, generally, the number of cases to consider. It costs
more in principle, but maybe negligible in practice?
================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:613
mergeDistinctValues(Loc->getType(), *Val, *this, *It->second,
Other,
JoinedEnv, Model)) {
JoinedEnv.LocToVal.insert({Loc, MergedVal});
----------------
nit: i think you can now drop the braces.
================
Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:208
+ if (!CurrentState) {
+ CurrentState = &State;
+ } else if (!OwnedState) {
----------------
Is there some invariant that guarantees the safety here? Because `const &` can
bind to a temporary, so this feels like a risky API.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153493/new/
https://reviews.llvm.org/D153493
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
