[PATCH] D141107: [clang-tidy] don't warn when returning the result for bugprone-standalone-empty

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry created this revision.
v1nh1shungry added reviewers: cjdb, hokein.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

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
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141107

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
@@ -576,3 +576,16 @@
     // CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 
'std::empty' [bugprone-standalone-empty]
   }
 }
+
+bool test_empty_in_return() {
+  {
+    std::vector<int> v;
+    return v.empty();
+    // no-warning
+  }
+  {
+    std::vector<int> v;
+    return std::empty(v);
+    // no-warning
+  }
+}
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
@@ -98,6 +98,7 @@
   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 @@
     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 @@
     if (PParentStmtExpr && ParentCompStmt &&
         ParentCompStmt->body_back() == NonMemberCall->getExprStmt())
       return;
+    if (ParentReturnStmt)
+      return;
 
     SourceLocation NonMemberLoc = NonMemberCall->getExprLoc();
     SourceLocation NonMemberEndLoc = NonMemberCall->getEndLoc();



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
@@ -576,3 +576,16 @@
     // CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
   }
 }
+
+bool test_empty_in_return() {
+  {
+    std::vector<int> v;
+    return v.empty();
+    // no-warning
+  }
+  {
+    std::vector<int> v;
+    return std::empty(v);
+    // no-warning
+  }
+}
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
@@ -98,6 +98,7 @@
   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 @@
     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 @@
     if (PParentStmtExpr && ParentCompStmt &&
         ParentCompStmt->body_back() == NonMemberCall->getExprStmt())
       return;
+    if (ParentReturnStmt)
+      return;
 
     SourceLocation NonMemberLoc = NonMemberCall->getExprLoc();
     SourceLocation NonMemberEndLoc = NonMemberCall->getEndLoc();

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits