[PATCH] D142423: [clang-tidy] Fix segfault in bugprone-standalone-empty

[Christopher Di Bella via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG716469b6139a: [clang-tidy] Fix segfault in 
bugprone-standalone-empty (authored by denik, committed by cjdb).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142423/new/

https://reviews.llvm.org/D142423

Files:
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  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
@@ -80,6 +80,10 @@
 void empty(T &&);
 } // namespace test
 
+namespace test_no_args {
+bool empty();
+} // namespace test_no_args
+
 namespace base {
 template <typename T>
 struct base_vector {
@@ -306,6 +310,9 @@
 
     test::empty(v);
     // no-warning
+
+    test_no_args::empty();
+    // no-warning
   }
 
   {
@@ -876,3 +883,24 @@
     // no-warning
   }
 }
+
+namespace user_lib {
+template <typename T>
+struct vector {
+  bool empty();
+  bool test_empty_inside_impl() {
+    empty();
+    // no-warning
+    return empty();
+    // no-warning
+  }
+};
+} // namespace user_lib
+
+bool test_template_empty_outside_impl() {
+  user_lib::vector<int> v;
+  v.empty();
+  // CHECK-MESSAGES: :[[#@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  return v.empty();
+  // no-warning
+}
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
@@ -3,8 +3,8 @@
 bugprone-standalone-empty
 =========================
 
-Warns when `empty()` is used on a range and the result is ignored. Suggests 
-`clear()` if it is an existing member function.
+Warns when ``empty()`` is used on a range and the result is ignored. Suggests
+``clear()`` if it is an existing member function.
 
 The ``empty()`` method on several common ranges returns a Boolean indicating
 whether or not the range is empty, but is often mistakenly interpreted as
@@ -29,3 +29,8 @@
   std::vector<int> v;
   ...
   v.clear();
+
+Limitations:
+- Doesn't warn if ``empty()`` is defined and used with the ignore result in the
+  class template definition (for example in the library implementation). These
+  error cases can be caught with ``[[nodiscard]]`` attribute.
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
@@ -29,10 +29,12 @@
 using ast_matchers::BoundNodes;
 using ast_matchers::callee;
 using ast_matchers::callExpr;
+using ast_matchers::classTemplateDecl;
 using ast_matchers::cxxMemberCallExpr;
 using ast_matchers::cxxMethodDecl;
 using ast_matchers::expr;
 using ast_matchers::functionDecl;
+using ast_matchers::hasAncestor;
 using ast_matchers::hasName;
 using ast_matchers::hasParent;
 using ast_matchers::ignoringImplicit;
@@ -70,10 +72,13 @@
 }
 
 void StandaloneEmptyCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
+  // Ignore empty calls in a template definition which fall under callExpr
+  // non-member matcher even if they are methods.
   const auto NonMemberMatcher = expr(ignoringImplicit(ignoringParenImpCasts(
       callExpr(
           hasParent(stmt(optionally(hasParent(stmtExpr().bind("stexpr"))))
                         .bind("parent")),
+          unless(hasAncestor(classTemplateDecl())),
           callee(functionDecl(hasName("empty"), unless(returns(voidType())))))
           .bind("empty"))));
   const auto MemberMatcher =
@@ -154,6 +159,8 @@
       return;
     if (ParentReturnStmt)
       return;
+    if (NonMemberCall->getNumArgs() != 1)
+      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