[PATCH] D128368: added clear check functionality

Christopher Di Bella via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 22 12:08:10 PDT 2022

cjdb added a comment.

A great start, thanks! I understand this CL to have been sent out a few minutes in advance, so as a note to other reviewers: maybe check back in in ~20 minutes please?

Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:52-55
+    auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) {
+      return F->getDeclName().getAsString() == "clear" &&
+             F->getMinRequiredArguments() == 0;
+    });
Note to other reviewers: I suggested this because we couldn't work out a better way to identify if `.clear()` exists (Abraham can best expand on the issues regarding how a matcher wasn't working).

Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:75-78
+    diag(NonMemberLoc, "ignoring the result of '%0'")
+        << llvm::dyn_cast<NamedDecl>(NonMemberCall->getCalleeDecl())
+               ->getQualifiedNameAsString()
+        << FixItHint::CreateReplacement(ReplacementRange, ReplacementText);
It looks as though this is unconditionally suggesting `clear` (even if it doesn't exist), and doesn't suggest it in the message.

Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp:5-9
 template <typename T>
 struct vector {
   bool empty();
   void clear();
Please remove `clear` from `vector`, and add `vector_with_clear`. We need to make sure that this works both when `clear` is absent and present.

Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp:16-21
+namespace absl {
+template <typename T>
+struct vector {
+  bool empty();
+  void clear();
As above. Please also rename `absl::vector` to something like `absl::string` (and drop the template altogether).

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list