ymandel added inline comments.
================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:38-41
+ if (auto *LHSValue =
+ dyn_cast_or_null<BoolValue>(Env.getValue(LHS, SkipPast::Reference)))
+ if (auto *RHSValue =
+ dyn_cast_or_null<BoolValue>(Env.getValue(RHS,
SkipPast::Reference)))
----------------
li.zhe.hua wrote:
> ymandel wrote:
> > Is the idea that the skipping is now built into `getValue` via
> > `getStorageLocation`?
> I don't believe I fixed this. d002495 addressed this by having integral casts
> route to the sub-expression.
Great. Just to be sure I understand: you mean that the `IgnoreCasts()` calls
were already redundant because of d002495?
================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:545-561
+ void VisitParenExpr(const ParenExpr *S) {
+ // The CFG does not contain `ParenExpr` as top-level statements in basic
+ // blocks, however manual traversal to sub-expressions may encounter them.
+ // Redirect to the sub-expression.
+ auto *SubExpr = S->getSubExpr();
+ assert(SubExpr != nullptr);
+ Visit(SubExpr);
----------------
li.zhe.hua wrote:
> ymandel wrote:
> > I thought this is the default behavior?
> The default behavior of `StmtVisitor` is that `VisitFoo` will call
> `VisitFooBase` where `Foo` derives from `FooBase` (e.g. the default
> implementation of `VisitExprWithCleanups` calls `VisitFullExpr`). The base
> case is a `VisitStmt` implementation that does [approximately
> nothing](https://github.com/llvm/llvm-project/blob/c2a5a87500d92c7c2e76c595f1b0f439b98b0aff/clang/include/clang/AST/StmtVisitor.h#L170-L171),
> especially if `RetTy = void`.
>
> So in this case, I'm changing this to automatically ignore `ParenExpr` and
> `ExprWithCleanups` and visit the sub-expression, which is not the default
> behavior.
>
> This isn't used when we call the transfer function from CFG-provided
> statements, because the CFG doesn't emit these nodes. However, this is
> relevant when we manually call the transfer function, e.g. from
> [here](https://github.com/llvm/llvm-project/blob/c2a5a87500d92c7c2e76c595f1b0f439b98b0aff/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp#L108).
Interesting. Thanks for the explanation!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124965/new/
https://reviews.llvm.org/D124965
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
