Author: v1nh1shungry Date: 2023-01-13T01:14:51Z New Revision: 7910ee7d8c6dcb679200ba171fba5d8d5f237007
URL: https://github.com/llvm/llvm-project/commit/7910ee7d8c6dcb679200ba171fba5d8d5f237007 DIFF: https://github.com/llvm/llvm-project/commit/7910ee7d8c6dcb679200ba171fba5d8d5f237007.diff LOG: [clang-tidy] don't warn when returning the result for bugprone-standalone-empty Relevant issue: https://github.com/llvm/llvm-project/issues/59517 Currently this check will warn when the result is used in a `return` statement, e.g. ``` bool foobar() { std::vector<int> v; return v.empty(); // will get a warning here, which makes no sense IMO } ``` Reviewed By: cjdb, denik Differential Revision: https://reviews.llvm.org/D141107 Added: Modified: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp index 34ccd2c61c784..9236bb9009145 100644 --- a/clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp @@ -98,6 +98,7 @@ void StandaloneEmptyCheck::check(const MatchFinder::MatchResult &Result) { const auto PParentStmtExpr = Result.Nodes.getNodeAs<Expr>("stexpr"); const auto ParentCompStmt = Result.Nodes.getNodeAs<CompoundStmt>("parent"); const auto *ParentCond = getCondition(Result.Nodes, "parent"); + const auto *ParentReturnStmt = Result.Nodes.getNodeAs<ReturnStmt>("parent"); if (const auto *MemberCall = Result.Nodes.getNodeAs<CXXMemberCallExpr>("empty")) { @@ -109,6 +110,9 @@ void StandaloneEmptyCheck::check(const MatchFinder::MatchResult &Result) { if (PParentStmtExpr && ParentCompStmt && ParentCompStmt->body_back() == MemberCall->getExprStmt()) return; + // Skip if it's a return statement + if (ParentReturnStmt) + return; SourceLocation MemberLoc = MemberCall->getBeginLoc(); SourceLocation ReplacementLoc = MemberCall->getExprLoc(); @@ -150,6 +154,8 @@ void StandaloneEmptyCheck::check(const MatchFinder::MatchResult &Result) { if (PParentStmtExpr && ParentCompStmt && ParentCompStmt->body_back() == NonMemberCall->getExprStmt()) return; + if (ParentReturnStmt) + return; SourceLocation NonMemberLoc = NonMemberCall->getExprLoc(); SourceLocation NonMemberEndLoc = NonMemberCall->getEndLoc(); diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp index 4f7248a78546f..d7615fdbbe922 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp @@ -155,7 +155,7 @@ bool empty(T &&); } // namespace qualifiers -void test_member_empty() { +bool test_member_empty() { { std::vector<int> v; v.empty(); @@ -231,9 +231,69 @@ void test_member_empty() { s.empty(); // CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty] } + + { + std::vector<int> v; + return v.empty(); + // no-warning + } + + { + std::vector_with_clear<int> v; + return v.empty(); + // no-warning + } + + { + std::vector_with_int_empty<int> v; + return v.empty(); + // no-warning + } + + { + std::vector_with_clear_args<int> v; + return v.empty(); + // no-warning + } + + { + std::vector_with_clear_variable<int> v; + return v.empty(); + // no-warning + } + + { + absl::string s; + return s.empty(); + // no-warning + } + + { + absl::string_with_clear s; + return s.empty(); + // no-warning + } + + { + absl::string_with_int_empty s; + return s.empty(); + // no-warning + } + + { + absl::string_with_clear_args s; + return s.empty(); + // no-warning + } + + { + absl::string_with_clear_variable s; + return s.empty(); + // no-warning + } } -void test_qualified_empty() { +bool test_qualified_empty() { { absl::string_with_clear v; std::empty(v); @@ -260,9 +320,30 @@ void test_qualified_empty() { absl::empty(nullptr); // no-warning } + + { + absl::string_with_clear s; + return std::empty(s); + // no-warning + return absl::empty(s); + // no-warning + } + + { + absl::string s; + return std::empty(s); + // no-warning + } + + { + return std::empty(0); + // no-warning + return absl::empty(nullptr); + // no-warning + } } -void test_unqualified_empty() { +bool test_unqualified_empty() { { std::vector<int> v; empty(v); @@ -370,6 +451,106 @@ void test_unqualified_empty() { // CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'absl::empty'; did you mean 'clear()'? [bugprone-standalone-empty] // CHECK-FIXES: {{^ }} s.clear();{{$}} } + + { + std::vector<int> v; + return empty(v); + // no-warning + } + + { + std::vector_with_void_empty<int> v; + return empty(v); + // no-warning + } + + { + std::vector_with_clear<int> v; + return empty(v); + // no-warning + } + + { + std::vector_with_int_empty<int> v; + return empty(v); + // no-warning + } + + { + std::vector_with_clear_args<int> v; + return empty(v); + // no-warning + } + + { + std::vector_with_clear_variable<int> v; + return empty(v); + // no-warning + } + + { + absl::string s; + return empty(s); + // no-warning + } + + { + absl::string_with_void_empty s; + return empty(s); + // no-warning + } + + { + absl::string_with_clear s; + return empty(s); + // no-warning + } + + { + absl::string_with_int_empty s; + return empty(s); + // no-warning + } + + { + absl::string_with_clear_args s; + return empty(s); + // no-warning + } + + { + absl::string_with_clear_variable s; + return empty(s); + // no-warning + } + + { + std::vector<int> v; + using std::empty; + return empty(v); + // no-warning + } + + { + std::vector_with_clear<int> v; + using std::empty; + return empty(v); + // no-warning + } + + { + absl::string s; + using absl::empty; + return empty(s); + // no-warning + } + + { + absl::string_with_clear s; + using absl::empty; + return empty(s); + // no-warning + } } void test_empty_method_expressions() { @@ -444,8 +625,7 @@ void test_empty_expressions() { // CHECK-MESSAGES: :[[#@LINE-1]]:27: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty] } -void test_clear_in_base_class() { - +bool test_clear_in_base_class() { { base::vector<int> v; v.empty(); @@ -497,9 +677,57 @@ void test_clear_in_base_class() { empty(v); // CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'base::empty' [bugprone-standalone-empty] } + + { + base::vector<int> v; + return v.empty(); + // no-warning + } + + { + base::vector_non_dependent<int> v; + return v.empty(); + // no-warning + } + + { + base::vector_clear_with_args<int> v; + return v.empty(); + // no-warning + } + + { + base::vector_clear_variable<int> v; + return v.empty(); + // no-warning + } + + { + base::vector<int> v; + return empty(v); + // no-warning + } + + { + base::vector_non_dependent<int> v; + return empty(v); + // no-warning + } + + { + base::vector_clear_with_args<int> v; + return empty(v); + // no-warning + } + + { + base::vector_clear_variable<int> v; + return empty(v); + // no-warning + } } -void test_clear_with_qualifiers() { +bool test_clear_with_qualifiers() { { qualifiers::vector_with_const_clear<int> v; v.empty(); @@ -575,4 +803,76 @@ void test_clear_with_qualifiers() { empty(v); // CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty] } + + { + qualifiers::vector_with_const_clear<int> v; + return v.empty(); + // no-warning + } + + { + const qualifiers::vector_with_const_clear<int> v; + return v.empty(); + // no-warning + } + + { + const qualifiers::vector_with_const_empty<int> v; + return v.empty(); + // no-warning + } + + { + qualifiers::vector_with_const_clear<int> v; + return empty(v); + // no-warning + } + + { + const qualifiers::vector_with_const_clear<int> v; + return empty(v); + // no-warning + } + + { + const std::vector_with_clear<int> v; + return empty(v); + // no-warning + } + + { + qualifiers::vector_with_volatile_clear<int> v; + return v.empty(); + // no-warning + } + + { + volatile qualifiers::vector_with_volatile_clear<int> v; + return v.empty(); + // no-warning + } + + { + volatile qualifiers::vector_with_volatile_empty<int> v; + return v.empty(); + // no-warning + } + + { + qualifiers::vector_with_volatile_clear<int> v; + return empty(v); + // no-warning + } + + { + volatile qualifiers::vector_with_volatile_clear<int> v; + return empty(v); + // no-warning + } + + { + volatile std::vector_with_clear<int> v; + return empty(v); + // no-warning + } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
