[clang-tools-extra] 556e4db - [clang-tidy] Fix performance-move-const-arg false negative in ternary… (#128402)

[via cfe-commits]

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