[clang-tools-extra] [clang-tidy]avoid bugprone-unused-return-value false positive for assignment operator overloading (PR #84489)
Congcong Cai via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 8 17:36:43 PST 2024
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/84489
>From 265db5ee772772bef4099cc97b69995cfa67b3f2 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Fri, 8 Mar 2024 22:15:20 +0800
Subject: [PATCH 1/3] [clang-tidy]avoid bugprone-unused-return-value false
positive for assignment operator overloading
Fixes: #84480
We assuem assignemnt at most of time, operator overloading means the value is assigned to the other variable, then clang-tidy should suppress warning even if this operator overloading match the regex.
---
.../bugprone/UnusedReturnValueCheck.cpp | 24 ++++++++------
clang-tools-extra/docs/ReleaseNotes.rst | 4 +--
.../unused-return-value-avoid-assignment.cpp | 31 +++++++++++++++++++
3 files changed, 47 insertions(+), 12 deletions(-)
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-avoid-assignment.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 1252b2f23805a1..2167d381c42b03 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -11,6 +11,7 @@
#include "../utils/OptionsUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
using namespace clang::ast_matchers;
using namespace clang::ast_matchers::internal;
@@ -157,16 +158,19 @@ 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)))))))))
- .bind("match"));
+ 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(anyOf(isCopyAssignmentOperator(),
+ isMoveAssignmentOperator()))),
+ anyOf(isInstantiatedFrom(
+ matchers::matchesAnyListedName(CheckedFunctions)),
+ returns(hasCanonicalType(hasDeclaration(
+ namedDecl(matchers::matchesAnyListedName(
+ CheckedReturnTypes)))))))))
+ .bind("match"));
auto CheckCastToVoid =
AllowCastToVoid ? castExpr(unless(hasCastKind(CK_ToVoid))) : castExpr();
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index b5f025ce467a15..c7121fe07e0ad3 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -152,9 +152,9 @@ Changes in existing checks
- Improved :doc:`bugprone-unused-return-value
<clang-tidy/checks/bugprone/unused-return-value>` check by updating the
- parameter `CheckedFunctions` to support regexp and avoiding false postive for
+ parameter `CheckedFunctions` to support regexp, avoiding false positive for
function with the same prefix as the default argument, e.g. ``std::unique_ptr``
- and ``std::unique``.
+ and ``std::unique``, avoiding false positive for assignment operator overloading.
- Improved :doc:`bugprone-use-after-move
<clang-tidy/checks/bugprone/use-after-move>` check to also handle
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
new file mode 100644
index 00000000000000..8bd3c30e71b51a
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-avoid-assignment.cpp
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s bugprone-unused-return-value %t \
+// RUN: -config='{CheckOptions: \
+// RUN: {bugprone-unused-return-value.CheckedFunctions: "::*"}}' \
+// RUN: --
+
+struct S {
+ S(){};
+ S(S const &);
+ S(S &&);
+ S &operator=(S const &);
+ S &operator=(S &&);
+};
+
+S returnValue();
+S 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{};
+ a = returnValue();
+ // CHECK-NOT: [[@LINE-1]]:3: warning
+ a.operator=(returnValue());
+ // CHECK-NOT: [[@LINE-1]]:3: warning
+
+ a = returnRef();
+ // CHECK-NOT: [[@LINE-1]]:3: warning
+ a.operator=(returnRef());
+ // CHECK-NOT: [[@LINE-1]]:3: warning
+}
>From 301340bc2557df4d5709cd645ca1d6dc95495789 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Sat, 9 Mar 2024 05:13:13 +0800
Subject: [PATCH 2/3] add all assignment sematic operator overloading
---
.../bugprone/UnusedReturnValueCheck.cpp | 31 ++++++++++++-------
.../checks/bugprone/unused-return-value.rst | 2 ++
.../unused-return-value-avoid-assignment.cpp | 7 ++---
3 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 2167d381c42b03..14b78e777bd70e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -12,6 +12,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/OperatorKinds.h"
using namespace clang::ast_matchers;
using namespace clang::ast_matchers::internal;
@@ -29,6 +30,11 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, Matcher<FunctionDecl>,
return InnerMatcher.matches(InstantiatedFrom ? *InstantiatedFrom : Node,
Finder, Builder);
}
+
+AST_MATCHER_P(CXXMethodDecl, isOperatorOverloading,
+ llvm::SmallVector<OverloadedOperatorKind>, kinds) {
+ return llvm::is_contained(kinds, Node.getOverloadedOperator());
+}
} // namespace
UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
@@ -159,17 +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(anyOf(isCopyAssignmentOperator(),
- isMoveAssignmentOperator()))),
- 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}))),
+ anyOf(
+ isInstantiatedFrom(
+ matchers::matchesAnyListedName(CheckedFunctions)),
+ returns(hasCanonicalType(hasDeclaration(namedDecl(
+ matchers::matchesAnyListedName(CheckedReturnTypes)))))))))
.bind("match"));
auto CheckCastToVoid =
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst
index 9c01ef50b53814..823dd47f8e3ecb 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst
@@ -5,6 +5,8 @@ bugprone-unused-return-value
Warns on unused function return values. The checked functions can be configured.
+Operator overloading with assignment semantics are ignored。
+
Options
-------
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 8bd3c30e71b51a..b4a41004adf894 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
@@ -9,6 +9,7 @@ struct S {
S(S &&);
S &operator=(S const &);
S &operator=(S &&);
+ S &operator+=(S);
};
S returnValue();
@@ -20,12 +21,10 @@ void bar() {
S a{};
a = returnValue();
- // CHECK-NOT: [[@LINE-1]]:3: warning
a.operator=(returnValue());
- // CHECK-NOT: [[@LINE-1]]:3: warning
a = returnRef();
- // CHECK-NOT: [[@LINE-1]]:3: warning
a.operator=(returnRef());
- // CHECK-NOT: [[@LINE-1]]:3: warning
+
+ a += returnRef();
}
>From 7b878aa92b53b4249635d4c802e2987c77721eed Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Sat, 9 Mar 2024 09:36:30 +0800
Subject: [PATCH 3/3] fix
---
.../clang-tidy/bugprone/UnusedReturnValueCheck.cpp | 4 ++--
.../docs/clang-tidy/checks/bugprone/unused-return-value.rst | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 14b78e777bd70e..243fe47c2036b6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -32,8 +32,8 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, Matcher<FunctionDecl>,
}
AST_MATCHER_P(CXXMethodDecl, isOperatorOverloading,
- llvm::SmallVector<OverloadedOperatorKind>, kinds) {
- return llvm::is_contained(kinds, Node.getOverloadedOperator());
+ llvm::SmallVector<OverloadedOperatorKind>, Kinds) {
+ return llvm::is_contained(Kinds, Node.getOverloadedOperator());
}
} // namespace
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst
index 823dd47f8e3ecb..9205ba98729c4b 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst
@@ -5,7 +5,7 @@ bugprone-unused-return-value
Warns on unused function return values. The checked functions can be configured.
-Operator overloading with assignment semantics are ignored。
+Operator overloading with assignment semantics are ignored.
Options
-------
More information about the cfe-commits
mailing list