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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
