[clang-tools-extra] 7910ee7 - [clang-tidy] don't warn when returning the result for bugprone-standalone-empty

Christopher Di Bella via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 12 17:15:34 PST 2023


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
+  }
 }


        


More information about the cfe-commits mailing list