Skip to content

Commit

Permalink
[clang-tidy] don't warn when returning the result for bugprone-standa…
Browse files Browse the repository at this point in the history 
…lone-empty

Relevant issue: #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
  • Loading branch information
@v1nh1shungry @cjdb
v1nh1shungry authored and cjdb committed Jan 13, 2023
1 parent d4cf89a commit 7910ee7
Show file tree
Hide file tree
Showing 2 changed files with 312 additions and 6 deletions.
6 changes: 6 additions & 0 deletions clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
View file
Expand Up @@ -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")) {
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
312 changes: 306 additions & 6 deletions clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
View file
Expand Up @@ -155,7 +155,7 @@ bool empty(T &&);
} // namespace qualifiers


void test_member_empty() {
bool test_member_empty() {
{
std::vector<int> v;
v.empty();
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
}
}

0 comments on commit 7910ee7

Please sign in to comment.