[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