[PATCH] D145581: [clang-tidy] In C++17, callee is guaranteed to be sequenced before arguments.
Martin Böhme via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 24 05:06:46 PDT 2023
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
More information about the cfe-commits
mailing list