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 <congcongcai0...@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 <congcongcai0...@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 <congcongcai0...@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 ------- _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits