Skip to content

Commit 556e4db

Browse files
authored
[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.
1 parent 741d7fa commit 556e4db

File tree

3 files changed

+37
-4
lines changed

3 files changed

+37
-4
lines changed

clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ void MoveConstArgCheck::registerMatchers(MatchFinder *Finder) {
4444
unless(isInTemplateInstantiation()))
4545
.bind("call-move");
4646

47+
// Match ternary expressions where either branch contains std::move
48+
auto TernaryWithMoveMatcher =
49+
conditionalOperator(hasDescendant(MoveCallMatcher));
50+
4751
Finder->addMatcher(
4852
expr(anyOf(
4953
castExpr(hasSourceExpression(MoveCallMatcher)),
@@ -58,13 +62,15 @@ void MoveConstArgCheck::registerMatchers(MatchFinder *Finder) {
5862
qualType(rValueReferenceType()).bind("invocation-parm-type");
5963
// Matches respective ParmVarDecl for a CallExpr or CXXConstructExpr.
6064
auto ArgumentWithParamMatcher = forEachArgumentWithParam(
61-
MoveCallMatcher, parmVarDecl(anyOf(hasType(ConstTypeParmMatcher),
62-
hasType(RValueTypeParmMatcher)))
63-
.bind("invocation-parm"));
65+
anyOf(MoveCallMatcher, TernaryWithMoveMatcher),
66+
parmVarDecl(
67+
anyOf(hasType(ConstTypeParmMatcher), hasType(RValueTypeParmMatcher)))
68+
.bind("invocation-parm"));
6469
// Matches respective types of arguments for a CallExpr or CXXConstructExpr
6570
// and it works on calls through function pointers as well.
6671
auto ArgumentWithParamTypeMatcher = forEachArgumentWithParamType(
67-
MoveCallMatcher, anyOf(ConstTypeParmMatcher, RValueTypeParmMatcher));
72+
anyOf(MoveCallMatcher, TernaryWithMoveMatcher),
73+
anyOf(ConstTypeParmMatcher, RValueTypeParmMatcher));
6874

6975
Finder->addMatcher(
7076
invocation(anyOf(ArgumentWithParamMatcher, ArgumentWithParamTypeMatcher))

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,10 @@ Changes in existing checks
120120
tolerating fix-it breaking compilation when functions is used as pointers
121121
to avoid matching usage of functions within the current compilation unit.
122122

123+
- Improved :doc:`performance-move-const-arg
124+
<clang-tidy/checks/performance/move-const-arg>` check by fixing false negatives
125+
on ternary operators calling ``std::move``.
126+
123127
Removed checks
124128
^^^^^^^^^^^^^^
125129

clang-tools-extra/test/clang-tidy/checkers/performance/move-const-arg.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,3 +560,26 @@ struct Result {
560560
// 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]
561561
};
562562
} // namespace GH111450
563+
564+
namespace GH126515 {
565+
566+
struct TernaryMoveCall {
567+
TernaryMoveCall();
568+
TernaryMoveCall(const TernaryMoveCall&);
569+
TernaryMoveCall operator=(const TernaryMoveCall&);
570+
571+
void TernaryCheckTriviallyCopyable(const char * c) {}
572+
573+
void testTernaryMove() {
574+
TernaryMoveCall t1;
575+
TernaryMoveCall other(false ? TernaryMoveCall() : TernaryMoveCall(std::move(t1)) );
576+
// 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]
577+
// CHECK-MESSAGES: :[[@LINE-11]]:8: note: 'TernaryMoveCall' is not move assignable/constructible
578+
579+
const char* a = "a";
580+
TernaryCheckTriviallyCopyable(true ? std::move(a) : "" );
581+
// 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]
582+
}
583+
584+
};
585+
} // namespace GH126515

0 commit comments

Comments
 (0)