Skip to content

Commit 05519fa

Browse files
committed
[clang-tidy][performance-move-const-arg] Fix crash when argument type has no definition
Fix #111450. When the argument type is forward-declared and there is no definition, `hasMoveConstructor` triggers the assert here: https://github.com/llvm/llvm-project/blob/7f65377880ce6a0e5eaa4cb2591b86b8c8a24ee6/clang/include/clang/AST/DeclCXX.h#L465 https://github.com/llvm/llvm-project/blob/7f65377880ce6a0e5eaa4cb2591b86b8c8a24ee6/clang/include/clang/AST/DeclCXX.h#L864 Check `hasDefinition()` before `hasMoveConstructor()`.
1 parent 7f65377 commit 05519fa

File tree

3 files changed

+21
-2
lines changed

3 files changed

+21
-2
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,9 @@ void MoveConstArgCheck::check(const MatchFinder::MatchResult &Result) {
209209
}
210210

211211
if (const CXXRecordDecl *RecordDecl = ArgType->getAsCXXRecordDecl();
212-
RecordDecl && !(RecordDecl->hasMoveConstructor() &&
213-
RecordDecl->hasMoveAssignment())) {
212+
RecordDecl && RecordDecl->hasDefinition() &&
213+
!(RecordDecl->hasMoveConstructor() &&
214+
RecordDecl->hasMoveAssignment())) {
214215
const bool MissingMoveAssignment = !RecordDecl->hasMoveAssignment();
215216
const bool MissingMoveConstructor = !RecordDecl->hasMoveConstructor();
216217
const bool MissingBoth = MissingMoveAssignment && MissingMoveConstructor;

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,10 @@ Changes in existing checks
208208
<clang-tidy/checks/performance/avoid-endl>` check to use ``std::endl`` as
209209
placeholder when lexer cannot get source text.
210210

211+
- Improved :doc:`performance-move-const-arg
212+
<clang-tidy/checks/performance/move-const-arg>` check to fix a crash when
213+
an argument type is declared but not defined.
214+
211215
- Improved :doc:`readability-container-contains
212216
<clang-tidy/checks/readability/container-contains>` check to let it work on
213217
any class that has a ``contains`` method.

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,3 +546,17 @@ void testAlsoNonMoveable() {
546546
}
547547

548548
} // namespace issue_62550
549+
550+
namespace GH111450 {
551+
struct Status;
552+
553+
struct Error {
554+
Error(const Status& S);
555+
};
556+
557+
struct Result {
558+
Error E;
559+
Result(Status&& S) : E(std::move(S)) {}
560+
// 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]
561+
};
562+
} // namespace GH111450

0 commit comments

Comments
 (0)