[clang-tools-extra] [clang-tidy]bugprone-unused-return-value ignore `++` and `--` operator overloading (PR #84922)
Congcong Cai via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 14 08:30:12 PDT 2024
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/84922
>From d760b280a79be973642a0549db0e5a8838c397fd Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Tue, 12 Mar 2024 22:33:18 +0800
Subject: [PATCH 1/4] [clang-tidy]bugprone-unused-return-value ignore `++` and
`--` operator overloading
Fixes: #84705
---
.../bugprone/UnusedReturnValueCheck.cpp | 28 +++++++++----------
.../unused-return-value-avoid-assignment.cpp | 8 ++++++
2 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 243fe47c2036b6..83b332fba1e2da 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -165,20 +165,20 @@ void UnusedReturnValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
void UnusedReturnValueCheck::registerMatchers(MatchFinder *Finder) {
auto MatchedDirectCallExpr = expr(
- callExpr(
- callee(functionDecl(
- // Don't match void overloads of checked functions.
- unless(returns(voidType())),
- // Don't match copy or move assignment operator.
- unless(cxxMethodDecl(isOperatorOverloading(
- {OO_Equal, OO_PlusEqual, OO_MinusEqual, OO_StarEqual,
- OO_SlashEqual, OO_PercentEqual, OO_CaretEqual, OO_AmpEqual,
- OO_PipeEqual, OO_LessLessEqual, OO_GreaterGreaterEqual}))),
- anyOf(
- isInstantiatedFrom(
- matchers::matchesAnyListedName(CheckedFunctions)),
- returns(hasCanonicalType(hasDeclaration(namedDecl(
- matchers::matchesAnyListedName(CheckedReturnTypes)))))))))
+ callExpr(callee(functionDecl(
+ // Don't match void overloads of checked functions.
+ unless(returns(voidType())),
+ // Don't match copy or move assignment operator.
+ unless(cxxMethodDecl(isOperatorOverloading(
+ {OO_Equal, OO_PlusEqual, OO_MinusEqual, OO_StarEqual,
+ OO_SlashEqual, OO_PercentEqual, OO_CaretEqual,
+ OO_AmpEqual, OO_PipeEqual, OO_LessLessEqual,
+ OO_GreaterGreaterEqual, OO_PlusPlus, OO_MinusMinus}))),
+ anyOf(isInstantiatedFrom(
+ matchers::matchesAnyListedName(CheckedFunctions)),
+ returns(hasCanonicalType(hasDeclaration(
+ namedDecl(matchers::matchesAnyListedName(
+ CheckedReturnTypes)))))))))
.bind("match"));
auto CheckCastToVoid =
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-avoid-assignment.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-avoid-assignment.cpp
index b4a41004adf894..5809da9ec27b38 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-avoid-assignment.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-avoid-assignment.cpp
@@ -10,6 +10,10 @@ struct S {
S &operator=(S const &);
S &operator=(S &&);
S &operator+=(S);
+ S &operator++();
+ S &operator++(int);
+ S &operator--();
+ S &operator--(int);
};
S returnValue();
@@ -27,4 +31,8 @@ void bar() {
a.operator=(returnRef());
a += returnRef();
+ a++;
+ ++a;
+ a--;
+ --a;
}
>From 784df684a08f4b4da97d8db9ba3bf708dafbf626 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 13 Mar 2024 14:31:36 +0800
Subject: [PATCH 2/4] better version
---
.../bugprone/UnusedReturnValueCheck.cpp | 43 +++++++++++--------
.../unused-return-value-avoid-assignment.cpp | 4 ++
2 files changed, 30 insertions(+), 17 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 83b332fba1e2da..846505317c1c7a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -31,9 +31,20 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, Matcher<FunctionDecl>,
Finder, Builder);
}
-AST_MATCHER_P(CXXMethodDecl, isOperatorOverloading,
- llvm::SmallVector<OverloadedOperatorKind>, Kinds) {
- return llvm::is_contained(Kinds, Node.getOverloadedOperator());
+constexpr std::initializer_list<OverloadedOperatorKind>
+ AssignmentOverloadedOperatorKinds = {
+ OO_Equal, OO_PlusEqual, OO_MinusEqual, OO_StarEqual,
+ OO_SlashEqual, OO_PercentEqual, OO_CaretEqual, OO_AmpEqual,
+ OO_PipeEqual, OO_LessLessEqual, OO_GreaterGreaterEqual, OO_PlusPlus,
+ OO_MinusMinus};
+
+AST_MATCHER(CXXOperatorCallExpr, isAssignmentOverloadedOperatorCall) {
+ return llvm::is_contained(AssignmentOverloadedOperatorKinds,
+ Node.getOperator());
+}
+AST_MATCHER(CXXMethodDecl, isAssignmentOverloadedOperatorMethod) {
+ return llvm::is_contained(AssignmentOverloadedOperatorKinds,
+ Node.getOverloadedOperator());
}
} // namespace
@@ -165,20 +176,18 @@ void UnusedReturnValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
void UnusedReturnValueCheck::registerMatchers(MatchFinder *Finder) {
auto MatchedDirectCallExpr = expr(
- callExpr(callee(functionDecl(
- // Don't match void overloads of checked functions.
- unless(returns(voidType())),
- // Don't match copy or move assignment operator.
- unless(cxxMethodDecl(isOperatorOverloading(
- {OO_Equal, OO_PlusEqual, OO_MinusEqual, OO_StarEqual,
- OO_SlashEqual, OO_PercentEqual, OO_CaretEqual,
- OO_AmpEqual, OO_PipeEqual, OO_LessLessEqual,
- OO_GreaterGreaterEqual, OO_PlusPlus, OO_MinusMinus}))),
- anyOf(isInstantiatedFrom(
- matchers::matchesAnyListedName(CheckedFunctions)),
- returns(hasCanonicalType(hasDeclaration(
- namedDecl(matchers::matchesAnyListedName(
- CheckedReturnTypes)))))))))
+ callExpr(
+ callee(functionDecl(
+ // Don't match void overloads of checked functions.
+ unless(returns(voidType())),
+ anyOf(
+ isInstantiatedFrom(
+ matchers::matchesAnyListedName(CheckedFunctions)),
+ returns(hasCanonicalType(hasDeclaration(namedDecl(
+ matchers::matchesAnyListedName(CheckedReturnTypes)))))))),
+ // Don't match copy or move assignment operator.
+ unless(cxxOperatorCallExpr(isAssignmentOverloadedOperatorCall())),
+ unless(callee(cxxMethodDecl(isAssignmentOverloadedOperatorMethod()))))
.bind("match"));
auto CheckCastToVoid =
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-avoid-assignment.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-avoid-assignment.cpp
index 5809da9ec27b38..e7004f076228d2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-avoid-assignment.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-avoid-assignment.cpp
@@ -16,6 +16,8 @@ struct S {
S &operator--(int);
};
+S &operator-=(S&, S);
+
S returnValue();
S const &returnRef();
@@ -31,6 +33,8 @@ void bar() {
a.operator=(returnRef());
a += returnRef();
+ a -= returnRef();
+
a++;
++a;
a--;
>From e91d1da188d5afd26b65d21494d0b3c1add7e7b3 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Thu, 14 Mar 2024 21:29:33 +0800
Subject: [PATCH 3/4] fix
---
.../bugprone/UnusedReturnValueCheck.cpp | 9 +---
.../unused-return-value-avoid-assignment.cpp | 45 ++++++++++++-------
2 files changed, 31 insertions(+), 23 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 846505317c1c7a..26fed139eb149e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -38,11 +38,7 @@ constexpr std::initializer_list<OverloadedOperatorKind>
OO_PipeEqual, OO_LessLessEqual, OO_GreaterGreaterEqual, OO_PlusPlus,
OO_MinusMinus};
-AST_MATCHER(CXXOperatorCallExpr, isAssignmentOverloadedOperatorCall) {
- return llvm::is_contained(AssignmentOverloadedOperatorKinds,
- Node.getOperator());
-}
-AST_MATCHER(CXXMethodDecl, isAssignmentOverloadedOperatorMethod) {
+AST_MATCHER(FunctionDecl, isAssignmentOverloadedOperatorMethod) {
return llvm::is_contained(AssignmentOverloadedOperatorKinds,
Node.getOverloadedOperator());
}
@@ -186,8 +182,7 @@ void UnusedReturnValueCheck::registerMatchers(MatchFinder *Finder) {
returns(hasCanonicalType(hasDeclaration(namedDecl(
matchers::matchesAnyListedName(CheckedReturnTypes)))))))),
// Don't match copy or move assignment operator.
- unless(cxxOperatorCallExpr(isAssignmentOverloadedOperatorCall())),
- unless(callee(cxxMethodDecl(isAssignmentOverloadedOperatorMethod()))))
+ unless(callee(functionDecl(isAssignmentOverloadedOperatorMethod()))))
.bind("match"));
auto CheckCastToVoid =
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-avoid-assignment.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-avoid-assignment.cpp
index e7004f076228d2..564c07a724ccde 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-avoid-assignment.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-avoid-assignment.cpp
@@ -3,29 +3,37 @@
// RUN: {bugprone-unused-return-value.CheckedFunctions: "::*"}}' \
// RUN: --
-struct S {
- S(){};
- S(S const &);
- S(S &&);
- S &operator=(S const &);
- S &operator=(S &&);
- S &operator+=(S);
- S &operator++();
- S &operator++(int);
- S &operator--();
- S &operator--(int);
+struct S1 {
+ S1(){};
+ S1(S1 const &);
+ S1(S1 &&);
+ S1 &operator=(S1 const &);
+ S1 &operator=(S1 &&);
+ S1 &operator+=(S1);
+ S1 &operator++();
+ S1 &operator++(int);
+ S1 &operator--();
+ S1 &operator--(int);
};
-S &operator-=(S&, S);
+struct S2 {
+ S2(){};
+ S2(S2 const &);
+ S2(S2 &&);
+};
+
+S2 &operator-=(S2&, int);
+S2 &operator++(S2 &);
+S2 &operator++(S2 &, int);
-S returnValue();
-S const &returnRef();
+S1 returnValue();
+S1 const &returnRef();
void bar() {
returnValue();
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
- S a{};
+ S1 a{};
a = returnValue();
a.operator=(returnValue());
@@ -33,10 +41,15 @@ void bar() {
a.operator=(returnRef());
a += returnRef();
- a -= returnRef();
a++;
++a;
a--;
--a;
+
+ S2 b{};
+
+ b -= 1;
+ b++;
+ ++b;
}
>From 623720c63f63c165257f11ef89cf69b2b7072ef9 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Thu, 14 Mar 2024 23:29:06 +0800
Subject: [PATCH 4/4] fix
---
.../bugprone/UnusedReturnValueCheck.cpp | 27 +++++++++----------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 26fed139eb149e..73373147e96fc9 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -38,7 +38,7 @@ constexpr std::initializer_list<OverloadedOperatorKind>
OO_PipeEqual, OO_LessLessEqual, OO_GreaterGreaterEqual, OO_PlusPlus,
OO_MinusMinus};
-AST_MATCHER(FunctionDecl, isAssignmentOverloadedOperatorMethod) {
+AST_MATCHER(FunctionDecl, isAssignmentOverloadedOperator) {
return llvm::is_contained(AssignmentOverloadedOperatorKinds,
Node.getOverloadedOperator());
}
@@ -171,19 +171,18 @@ void UnusedReturnValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
}
void UnusedReturnValueCheck::registerMatchers(MatchFinder *Finder) {
- auto MatchedDirectCallExpr = expr(
- callExpr(
- callee(functionDecl(
- // Don't match void overloads of checked functions.
- unless(returns(voidType())),
- anyOf(
- isInstantiatedFrom(
- matchers::matchesAnyListedName(CheckedFunctions)),
- returns(hasCanonicalType(hasDeclaration(namedDecl(
- matchers::matchesAnyListedName(CheckedReturnTypes)))))))),
- // Don't match copy or move assignment operator.
- unless(callee(functionDecl(isAssignmentOverloadedOperatorMethod()))))
- .bind("match"));
+ auto MatchedDirectCallExpr =
+ expr(callExpr(callee(functionDecl(
+ // Don't match copy or move assignment operator.
+ unless(isAssignmentOverloadedOperator()),
+ // Don't match void overloads of checked functions.
+ unless(returns(voidType())),
+ anyOf(isInstantiatedFrom(matchers::matchesAnyListedName(
+ CheckedFunctions)),
+ returns(hasCanonicalType(hasDeclaration(
+ namedDecl(matchers::matchesAnyListedName(
+ CheckedReturnTypes)))))))))
+ .bind("match"));
auto CheckCastToVoid =
AllowCastToVoid ? castExpr(unless(hasCastKind(CK_ToVoid))) : castExpr();
More information about the cfe-commits
mailing list