abrahamcd created this revision. Herald added a subscriber: carlosgalvezp. Herald added a project: All. abrahamcd requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits.
Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D128368 Files: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp @@ -6,76 +6,74 @@ struct vector { bool empty(); void clear(); - // CHECK-MESSAGES: HERE }; -// template <typename T> -// bool empty(T &&); +template <typename T> +bool empty(T &&); } // namespace std -// namespace absl { -// template <typename T> -// struct vector { -// bool empty(); -// void clear(); -// // CHEscdcCK-MESSAGES: HERE -// }; +namespace absl { +template <typename T> +struct vector { + bool empty(); + void clear(); +}; -// template <class T> -// bool empty(T &&); -// } // namespace absl +template <class T> +bool empty(T &&); +} // namespace absl int test_member_empty() { std::vector<int> v; v.empty(); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty] - // CvfdafHECK-FIXES: {{^ }}v.clear();{{$}} + // CHECK-FIXES: {{^ }}v.clear();{{$}} - // absl::vector<int> w; + absl::vector<int> w; - // w.empty(); + w.empty(); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty] - // ChafdafHECK-FIXES: {{^ }}w.clear();{{$}} + // CHECK-FIXES: {{^ }}w.clear();{{$}} } -// int test_qualified_empty() { -// absl::vector<int> v; - -// std::empty(v); -// // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty] -// // CHECK-FIXES: {{^ }}v.clear();{{$}} - -// absl::empty(v); -// // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty] -// // CHECK-FIXES: {{^ }}v.clear();{{$}} -// } - -// int test_unqualified_empty() { -// std::vector<int> v; - -// empty(v); -// // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty] -// // CHECK-FIXES: {{^ }}v.clear();{{$}} - -// absl::vector<int> w; - -// empty(w); -// // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty] -// // CHECK-FIXES: {{^ }}w.clear();{{$}} - -// { -// using std::empty; -// empty(v); -// // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty] -// // CHECK-FIXES: {{^ }} v.clear();{{$}} -// } - -// { -// using absl::empty; -// empty(w); -// // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty] -// // CHECK-FIXES: {{^ }} w.clear();{{$}} -// } -// } +int test_qualified_empty() { + absl::vector<int> v; + + std::empty(v); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty] + // CHECK-FIXES: {{^ }}v.clear();{{$}} + + absl::empty(v); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty] + // CHECK-FIXES: {{^ }}v.clear();{{$}} +} + +int test_unqualified_empty() { + std::vector<int> v; + + empty(v); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty] + // CHECK-FIXES: {{^ }}v.clear();{{$}} + + absl::vector<int> w; + + empty(w); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty] + // CHECK-FIXES: {{^ }}w.clear();{{$}} + + { + using std::empty; + empty(v); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty] + // CHECK-FIXES: {{^ }} v.clear();{{$}} + } + + { + using absl::empty; + empty(w); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty] + // CHECK-FIXES: {{^ }} w.clear();{{$}} + } +} Index: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp +++ clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp @@ -18,6 +18,7 @@ #include "clang/Basic/Diagnostic.h" #include "clang/Basic/SourceLocation.h" #include "clang/Lex/Lexer.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" namespace clang { @@ -25,67 +26,57 @@ namespace bugprone { void StandaloneEmptyCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { - const auto CallMatcher = ast_matchers::callExpr( - ast_matchers::callee(ast_matchers::functionDecl( - ast_matchers::hasName("empty")))) - .bind("empty"); - const auto MemberMatcher = ast_matchers::cxxMemberCallExpr( - ast_matchers::callee(ast_matchers::cxxMethodDecl( - ast_matchers::hasName("empty")))) - .bind("empty"); - - const auto ClearMatcher = ast_matchers::cxxMethodDecl(ast_matchers::hasName("clear")).bind("clear"); + const auto CallMatcher = + ast_matchers::callExpr(ast_matchers::callee(ast_matchers::functionDecl( + ast_matchers::hasName("empty")))) + .bind("empty"); + const auto MemberMatcher = + ast_matchers::cxxMemberCallExpr( + ast_matchers::callee( + ast_matchers::cxxMethodDecl(ast_matchers::hasName("empty")))) + .bind("empty"); Finder->addMatcher(MemberMatcher, this); Finder->addMatcher(CallMatcher, this); - Finder->addMatcher(ClearMatcher, this); - - // Finder->addMatcher(ast_matchers::expr( - // ast_matchers::anyOf(CallMatcher, MemberMatcher)), - // this); } void StandaloneEmptyCheck::check( const ast_matchers::MatchFinder::MatchResult &Result) { - - bool foundClear = false; - if (const auto *ClearDecl = Result.Nodes.getNodeAs<CXXMethodDecl>("clear")){ - SourceLocation ClearLoc = ClearDecl->getBeginLoc(); - diag(ClearLoc, "HERE"); - foundClear = true; - } - - if (const auto *MemberCall = Result.Nodes.getNodeAs<CXXMemberCallExpr>("empty")) { + if (const auto *MemberCall = + Result.Nodes.getNodeAs<CXXMemberCallExpr>("empty")) { SourceLocation MemberLoc = MemberCall->getBeginLoc(); SourceLocation ReplacementLoc = MemberCall->getExprLoc(); SourceRange ReplacementRange = SourceRange(ReplacementLoc, ReplacementLoc); - DiagnosticBuilder builder = diag(MemberLoc, "ignoring the result of 'empty()', did you mean 'clear()'? "); - if (foundClear) { - builder << FixItHint::CreateReplacement(ReplacementRange, "clear"); + + auto Methods = MemberCall->getRecordDecl()->methods(); + auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) { + return F->getDeclName().getAsString() == "clear" && + F->getMinRequiredArguments() == 0; + }); + + if (Clear != Methods.end()) { + DiagnosticBuilder Builder = + diag(MemberLoc, + "ignoring the result of 'empty()', did you mean 'clear()'? "); + Builder << FixItHint::CreateReplacement(ReplacementRange, "clear"); } - - - } else if (const auto *NonMemberCall = Result.Nodes.getNodeAs<CallExpr>("empty")) { + } else if (const auto *NonMemberCall = + Result.Nodes.getNodeAs<CallExpr>("empty")) { SourceLocation NonMemberLoc = NonMemberCall->getExprLoc(); SourceLocation NonMemberEndLoc = NonMemberCall->getEndLoc(); - const Expr* arg = NonMemberCall->getArg(0); - std::string ReplacementText = std::string(Lexer::getSourceText( - CharSourceRange::getTokenRange(arg->getSourceRange()), - *Result.SourceManager, getLangOpts())) + ".clear()"; + const Expr *Arg = NonMemberCall->getArg(0); + std::string ReplacementText = + std::string(Lexer::getSourceText( + CharSourceRange::getTokenRange(Arg->getSourceRange()), + *Result.SourceManager, getLangOpts())) + + ".clear()"; SourceRange ReplacementRange = SourceRange(NonMemberLoc, NonMemberEndLoc); - diag(NonMemberLoc, "ignoring the result of '%0'") << llvm::dyn_cast<NamedDecl>(NonMemberCall->getCalleeDecl())->getQualifiedNameAsString() - << FixItHint::CreateReplacement(ReplacementRange, ReplacementText); + diag(NonMemberLoc, "ignoring the result of '%0'") + << llvm::dyn_cast<NamedDecl>(NonMemberCall->getCalleeDecl()) + ->getQualifiedNameAsString() + << FixItHint::CreateReplacement(ReplacementRange, ReplacementText); } - - - // diag(MatchedDecl->getLocation(), "insert 'awesome'", DiagnosticIDs::Note) - // << FixItHint::CreateInsertion(MatchedDecl->getLocation(), "awesome_"); - - - - // diag(NonMemberLoc, "alternate message"); - } } // namespace bugprone
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits