denik created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
denik requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
The check incorrectly identified empty() method call in the template
class definition as a stand-alone function call.
This led to a crash because the checker did not expect empty() function
calls without arguments.
Fixes: https://github.com/llvm/llvm-project/issues/59487
Repository:
rG LLVM Github Monorepo
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
