[PATCH] D153220: [clang-tidy] Improve `performance-move-const-arg` message when no move constructor is available

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.

+- LGTM



================
Comment at: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp:207
+
+    if (const CXXRecordDecl *RecordDecl = ArgType->getAsCXXRecordDecl();
+        !RecordDecl->hasMoveConstructor() || !RecordDecl->hasMoveAssignment())
----------------
You need t check RecordDecl  here also.
like `if (...; RecordDecl  && !(RecordDecl->hasMoveConstructor() && RecordDecl->hasMoveAssignment()))`


================
Comment at: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp:209
+        !RecordDecl->hasMoveConstructor() || !RecordDecl->hasMoveAssignment())
+      diag(RecordDecl->getLocation(), "%0 is not move constructible",
+           DiagnosticIDs::Note)
----------------
maybe this should be `constructible/assignable` depend on what we are missing (constructor, assign operator)...
Simply this may be strange to get warn about construct when we assign.
Still things like std::optional cold use move constructor instead of assign operator on assign.
So this wound need to be checked... It's fine if we won't be very detailed here


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:378
+- Improved :doc:`performance-move-const-arg
+  <clang-tidy/checks/performance/move-const-arg>`: check to warn when no move
+  constructor is available.
----------------
remove `:`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153220/new/

https://reviews.llvm.org/D153220