mboehme marked 4 inline comments as done. mboehme added a comment. In D145581#4290839 <https://reviews.llvm.org/D145581#4290839>, @PiotrZSL wrote:
> First, re-base code, looks like there were some changes in this check, and > now there are conflicts with this path. Done. (Not sure what the etiquette is with respect to rebasing -- that's why I kept it based on an old revision.) > Second I don't any more comments, for me this code looks fine, BUT I'm not > familiar too much with this check. > Check history for this check, and maybe consider adding to review some other > people who modify it recently. Thanks, will do. ================ Comment at: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp:68-73 + for (const Expr *Arg : Call->arguments()) { + if (isDescendantOrEqual(Descendant, Arg, Context)) + return true; + } + + return false; ---------------- PiotrZSL wrote: > NOTE: This looks like llvm::any_of Done. ================ Comment at: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp:77-82 + for (const Expr *Arg : Call->arguments()) { + if (Arg == TheStmt) + return true; + } + + return false; ---------------- PiotrZSL wrote: > NOTE: this looks like llvm::any_of, or even something like contains/find Done (with find). ================ Comment at: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp:161-165 + if (const auto *call = dyn_cast<CallExpr>(Parent); + call && call->getCallee() == Before) { + if (isDescendantOfArgs(After, call, Context)) + return true; + } ---------------- PiotrZSL wrote: > NOTE: probably you could move call outside if, and do rest with single if... Or as a single if statement like this? ================ Comment at: clang-tools-extra/docs/ReleaseNotes.rst:167 + <clang-tidy/checks/bugprone/use-after-move>`: + - Fixed handling for designated initializers. + - Fix: In C++17 and later, the callee of a function is guaranteed to be ---------------- PiotrZSL wrote: > NOTE: Probably better would be to keep similar template to rest of checks, > just list fixes in sentences (or separated with ,), not as an list. Done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145581/new/ https://reviews.llvm.org/D145581 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits