https://github.com/RiverDave created
https://github.com/llvm/llvm-project/pull/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 expressions which was not being detected before**
## Testing
- A new mock class has been where the changes have been tested & all tests pass
>From d9dbf1b67ec6f249656c0b4af35ea48ef29cc095 Mon Sep 17 00:00:00 2001
From: Riverdave <davidriv...@gmail.com>
Date: Sat, 22 Feb 2025 03:57:35 -0500
Subject: [PATCH] [clang-tidy] Fix performance-move-const-arg false negative in
ternary operators
---
.../performance/MoveConstArgCheck.cpp | 13 +++++++++--
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++
.../checkers/performance/move-const-arg.cpp | 23 +++++++++++++++++++
3 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
b/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
index 421ce003975bc..3de41e707cfd7 100644
--- a/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
@@ -44,6 +44,12 @@ void MoveConstArgCheck::registerMatchers(MatchFinder
*Finder) {
unless(isInTemplateInstantiation()))
.bind("call-move");
+ // Match ternary expressions where either branch contains std::move
+ auto TernaryWithMoveMatcher =
+ conditionalOperator(
+ hasDescendant(MoveCallMatcher)
+ ).bind("ternary-move");
+
Finder->addMatcher(
expr(anyOf(
castExpr(hasSourceExpression(MoveCallMatcher)),
@@ -58,13 +64,16 @@ 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),
+ 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 41ff1c1016f25..2eb65d61f5e78 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,10 @@ Changes in existing checks
<clang-tidy/checks/misc/redundant-expression>` check by providing additional
examples and fixing some macro related false positives.
+- 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
