Skip to content

Commit d9dbf1b

Browse files
committed
[clang-tidy] Fix performance-move-const-arg false negative in ternary operators
1 parent bac4171 commit d9dbf1b

File tree

3 files changed

+38
-2
lines changed

3 files changed

+38
-2
lines changed

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ 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(
50+
hasDescendant(MoveCallMatcher)
51+
).bind("ternary-move");
52+
4753
Finder->addMatcher(
4854
expr(anyOf(
4955
castExpr(hasSourceExpression(MoveCallMatcher)),
@@ -58,13 +64,16 @@ void MoveConstArgCheck::registerMatchers(MatchFinder *Finder) {
5864
qualType(rValueReferenceType()).bind("invocation-parm-type");
5965
// Matches respective ParmVarDecl for a CallExpr or CXXConstructExpr.
6066
auto ArgumentWithParamMatcher = forEachArgumentWithParam(
61-
MoveCallMatcher, parmVarDecl(anyOf(hasType(ConstTypeParmMatcher),
67+
anyOf(MoveCallMatcher, TernaryWithMoveMatcher),
68+
parmVarDecl(anyOf(hasType(ConstTypeParmMatcher),
6269
hasType(RValueTypeParmMatcher)))
6370
.bind("invocation-parm"));
6471
// Matches respective types of arguments for a CallExpr or CXXConstructExpr
6572
// and it works on calls through function pointers as well.
6673
auto ArgumentWithParamTypeMatcher = forEachArgumentWithParamType(
67-
MoveCallMatcher, anyOf(ConstTypeParmMatcher, RValueTypeParmMatcher));
74+
anyOf(MoveCallMatcher, TernaryWithMoveMatcher),
75+
anyOf(ConstTypeParmMatcher, RValueTypeParmMatcher));
76+
6877

6978
Finder->addMatcher(
7079
invocation(anyOf(ArgumentWithParamMatcher, ArgumentWithParamTypeMatcher))

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,10 @@ Changes in existing checks
115115
<clang-tidy/checks/misc/redundant-expression>` check by providing additional
116116
examples and fixing some macro related false positives.
117117

118+
- Improved :doc:`performance-move-const-arg
119+
<clang-tidy/checks/performance/move-const-arg>` check by fixing false negatives
120+
on ternary operators calling ``std::move``.
121+
118122
Removed checks
119123
^^^^^^^^^^^^^^
120124

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)