sgatev added inline comments.
================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:99
+ const Environment &Env2, Value &MergedVal,
+ Environment &Env) {
return false;
----------------
xazax.hun wrote:
> `MergedEnv`? Also, the documentation above gives a short description of the
> relationship between `Val1`, `Val2`, and `MergedVal`. But it gives little
> pointers what are we supposed to do with the `Environment`.
Updated the name and added some pointers in the documentation.
================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:154
+
+ // `X v (X ^ Y ^ ...)` is logically equivalent to `X`. The common conditions
+ // have already been added to the result so we don't have to do anything here
----------------
ymandel wrote:
> The current comment gets at the big picture, but focusing on the actual
> disjunction that is being guarded (that is, just the remaining, unshared
> clauses) may be better. So, maybe instead point out that "True v (X ^ ...)"
> is equivalent to "True", since if either val is nullptr it represents "true"?
I added a general comment at the top and modified the comment here to target
the specific case.
================
Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:112
+
+ // The condition must be inversed in one of the successors.
+ if (BlockSuccIdx == 1)
----------------
ymandel wrote:
> Can we be more specific? I'd think we need to invert for specifically the
> successor corresponding to "else" or what not.
Yup. Updated the comment to be more specific.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120984/new/
https://reviews.llvm.org/D120984
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
