[clang-tidy] Fix misc-move-const-arg for move-only types.

[alexfh]

Summary

Fix misc-move-const-arg false positives on move-only types.

[alexfh]

Rebased on HEAD.

[alexfh]

Friendly ping.

[sbenza]

clang-tidy/misc/MoveConstantArgumentCheck.cpp

76 ↗

(On Diff #97971)

Can we get this from R->hasTrivialCopyConstructor or R->hasNonTrivialCopyConstructor instead of iterating all constructors?

[alexfh]

clang-tidy/misc/MoveConstantArgumentCheck.cpp

76 ↗

(On Diff #97971)

Interesting idea, trying R->hasTrivialCopyConstructor() || R->hasNonTrivialCopyConstructor().

[alexfh]

clang-tidy/misc/MoveConstantArgumentCheck.cpp

76 ↗

(On Diff #97971)

I meant the opposite: !R->hasTrivialCopyConstructor() && !R->hasNonTrivialCopyConstructor(). But anyway it doesn't work. We could store another bit for deleted copy constructor, but it wouldn't worth it, if it's needed in this single case.

clang-tools-extra/trunk/clang-tidy/misc/MoveConstantArgumentCheck.cpp

void MoveConstantArgumentCheck::check(const MatchFinder::MatchResult &Result) {
   if (!FileMoveRange.isValid())
     return;
 
   bool IsConstArg = Arg->getType().isConstQualified();
   bool IsTriviallyCopyable =
       Arg->getType().isTriviallyCopyableType(*Result.Context);
 
   if (IsConstArg || IsTriviallyCopyable) {
+    if (const CXXRecordDecl *R = Arg->getType()->getAsCXXRecordDecl()) {
+      for (const auto *Ctor : R->ctors()) {
+        if (Ctor->isCopyConstructor() && Ctor->isDeleted())
+          return;
+      }
+    }
     bool IsVariable = isa<DeclRefExpr>(Arg);
     const auto *Var =
         IsVariable ? dyn_cast<DeclRefExpr>(Arg)->getDecl() : nullptr;
     auto Diag = diag(FileMoveRange.getBegin(),
                      "std::move of the %select{|const }0"
                      "%select{expression|variable %4}1 "
                      "%select{|of the trivially-copyable type %5 }2"
                      "has no effect; remove std::move()"

clang-tools-extra/trunk/test/clang-tidy/misc-move-const-arg.cpp

void moveToConstReferenceNegatives() {
 
   // No warning inside of macro expansions.
   M3(NoMoveSemantics, no_move_semantics);
 
   // No warning inside of macro expansion, even if the macro expansion is inside
   // a lambda that is, in turn, an argument to a macro.
   CALL([no_move_semantics] { M3(NoMoveSemantics, no_move_semantics); });
 }
+
+class MoveOnly {
+public:
+  MoveOnly(const MoveOnly &other) = delete;
+  MoveOnly &operator=(const MoveOnly &other) = delete;
+  MoveOnly(MoveOnly &&other) = default;
+  MoveOnly &operator=(MoveOnly &&other) = default;
+};
+template <class T>
+void Q(T);
+void moveOnlyNegatives(MoveOnly val) {
+  Q(std::move(val));
+}