ymandel accepted this revision. ymandel added a comment. This revision is now accepted and ready to land.
Thanks! ================ Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:368 void Environment::setStorageLocation(const Expr &E, StorageLocation &Loc) { - assert(!isa<ExprWithCleanups>(&E)); - assert(ExprToLoc.find(&E) == ExprToLoc.end()); - ExprToLoc[&E] = &Loc; + assert(ExprToLoc.find(&ignoreCFGOmittedNodes(E)) == ExprToLoc.end()); + ExprToLoc[&ignoreCFGOmittedNodes(E)] = &Loc; ---------------- maybe bind to a temporary to avoid doing this twice (in debug code, that is)? arguably not that important to optimize debug mode but may also improve readability. ================ 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))) ---------------- Is the idea that the skipping is now built into `getValue` via `getStorageLocation`? ================ 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); ---------------- I thought this is the default behavior? 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