[PATCH] D128368: added clear check functionality

[Christopher Di Bella via Phabricator via cfe-commits]

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).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128368/new/

https://reviews.llvm.org/D128368