Author: David Rivera Date: 2025-02-27T19:35:10+08:00 New Revision: 556e4dbdcdfc88bc52b43324c4b3af0100c75cc4
URL: https://github.com/llvm/llvm-project/commit/556e4dbdcdfc88bc52b43324c4b3af0100c75cc4 DIFF: https://github.com/llvm/llvm-project/commit/556e4dbdcdfc88bc52b43324c4b3af0100c75cc4.diff LOG: [clang-tidy] Fix performance-move-const-arg false negative in ternary… (#128402) This PR aims to fix `performance-move-const-arg` #126515 ## Changes Enhanced the `performance-move-arg` check in Clang-Tidy to detect cases where `std::move` is used in **ternary operator expressions which was not being matched therefore being tagged as a false negative** ## Testing - A new mock class has been where the changes have been tested & all tests pass I'd appreciate any feedback since this is my first time contributing to LLVM. Added: Modified: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/performance/move-const-arg.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp b/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp index 421ce003975bc..703ad162f99cf 100644 --- a/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp @@ -44,6 +44,10 @@ void MoveConstArgCheck::registerMatchers(MatchFinder *Finder) { unless(isInTemplateInstantiation())) .bind("call-move"); + // Match ternary expressions where either branch contains std::move + auto TernaryWithMoveMatcher = + conditionalOperator(hasDescendant(MoveCallMatcher)); + Finder->addMatcher( expr(anyOf( castExpr(hasSourceExpression(MoveCallMatcher)), @@ -58,13 +62,15 @@ void MoveConstArgCheck::registerMatchers(MatchFinder *Finder) { qualType(rValueReferenceType()).bind("invocation-parm-type"); // Matches respective ParmVarDecl for a CallExpr or CXXConstructExpr. auto ArgumentWithParamMatcher = forEachArgumentWithParam( - MoveCallMatcher, parmVarDecl(anyOf(hasType(ConstTypeParmMatcher), - hasType(RValueTypeParmMatcher))) - .bind("invocation-parm")); + anyOf(MoveCallMatcher, TernaryWithMoveMatcher), + parmVarDecl( + anyOf(hasType(ConstTypeParmMatcher), hasType(RValueTypeParmMatcher))) + .bind("invocation-parm")); // Matches respective types of arguments for a CallExpr or CXXConstructExpr // and it works on calls through function pointers as well. auto ArgumentWithParamTypeMatcher = forEachArgumentWithParamType( - MoveCallMatcher, anyOf(ConstTypeParmMatcher, RValueTypeParmMatcher)); + anyOf(MoveCallMatcher, TernaryWithMoveMatcher), + anyOf(ConstTypeParmMatcher, RValueTypeParmMatcher)); Finder->addMatcher( invocation(anyOf(ArgumentWithParamMatcher, ArgumentWithParamTypeMatcher)) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2dcefa2ddec83..b87ea491b3ad1 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -120,6 +120,10 @@ Changes in existing checks tolerating fix-it breaking compilation when functions is used as pointers to avoid matching usage of functions within the current compilation unit. +- Improved :doc:`performance-move-const-arg + <clang-tidy/checks/performance/move-const-arg>` check by fixing false negatives + on ternary operators calling ``std::move``. + Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/move-const-arg.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/move-const-arg.cpp index 8e325b0ae6ca3..e616cbe78bc3a 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance/move-const-arg.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/move-const-arg.cpp @@ -560,3 +560,26 @@ struct Result { // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: passing result of std::move() as a const reference argument; no move will actually happen [performance-move-const-arg] }; } // namespace GH111450 + +namespace GH126515 { + +struct TernaryMoveCall { +TernaryMoveCall(); +TernaryMoveCall(const TernaryMoveCall&); +TernaryMoveCall operator=(const TernaryMoveCall&); + +void TernaryCheckTriviallyCopyable(const char * c) {} + +void testTernaryMove() { + TernaryMoveCall t1; + TernaryMoveCall other(false ? TernaryMoveCall() : TernaryMoveCall(std::move(t1)) ); + // CHECK-MESSAGES: :[[@LINE-1]]:69: warning: passing result of std::move() as a const reference argument; no move will actually happen [performance-move-const-arg] + // CHECK-MESSAGES: :[[@LINE-11]]:8: note: 'TernaryMoveCall' is not move assignable/constructible + + const char* a = "a"; + TernaryCheckTriviallyCopyable(true ? std::move(a) : "" ); + // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: std::move of the variable 'a' of the trivially-copyable type 'const char *' has no effect; remove std::move() [performance-move-const-arg] +} + +}; +} // namespace GH126515 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
