[clang-tidy] Fix check for trivially copyable types in modernize-pass-by-value

[malcolm.parsons]

Summary

rL270567 excluded trivially copyable types from being moved by
modernize-pass-by-value, but it didn't exclude references to them.
Change types used in the tests to not be trivially copyable.

[madsravn]

Looks good to me.

clang-tidy/modernize/PassByValueCheck.cpp

void PassByValueCheck::check(const MatchFinder::MatchResult &Result) {
 
   // If the parameter is used or anything other than the copy, do not apply
   // the changes.
   if (!paramReferredExactlyOnce(Ctor, ParamDecl))
     return;
 
   // If the parameter is trivial to copy, don't move it. Moving a trivivally
   // copyable type will cause a problem with misc-move-const-arg
-  if (ParamDecl->getType().isTriviallyCopyableType(*Result.Context))
+  if (ParamDecl->getType().getNonReferenceType().isTriviallyCopyableType(
+          *Result.Context))
     return;
 
   auto Diag = diag(ParamDecl->getLocStart(), "pass by value and use std::move");
 
   // Iterate over all declarations of the constructor.
   for (const ParmVarDecl *ParmDecl : collectParamDecls(Ctor, ParamDecl)) {
     auto ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc();
     auto RefTL = ParamTL.getAs<ReferenceTypeLoc>();

test/clang-tidy/Inputs/modernize-pass-by-value/header.h

 class ThreadId {
+public:
+  ThreadId(const ThreadId &) {}
+  ThreadId(ThreadId &&) {}
 };
 
 struct A {
   A(const ThreadId &tid) : threadid(tid) {}
   ThreadId threadid;
 };

test/clang-tidy/modernize-pass-by-value-header.cpp

 // RUN: cp %S/Inputs/modernize-pass-by-value/header.h %T/pass-by-value-header.h
 // RUN: clang-tidy %s -checks='-*,modernize-pass-by-value' -header-filter='.*' -fix -- -std=c++11 -I %T | FileCheck %s -check-prefix=CHECK-MESSAGES -implicit-check-not="{{warning|error}}:"
 // RUN: FileCheck -input-file=%T/pass-by-value-header.h %s -check-prefix=CHECK-FIXES
 
 #include "pass-by-value-header.h"
-// CHECK-MESSAGES: :5:5: warning: pass by value and use std::move [modernize-pass-by-value]
+// CHECK-MESSAGES: :8:5: warning: pass by value and use std::move [modernize-pass-by-value]
 // CHECK-FIXES: #include <utility>
 // CHECK-FIXES: A(ThreadId tid) : threadid(std::move(tid)) {}

test/clang-tidy/modernize-pass-by-value-macro-header.cpp

 // RUN: %check_clang_tidy %s modernize-pass-by-value %t -- -- -std=c++11 -isystem %S/Inputs/Headers
 
 // CHECK-FIXES: #include <utility>
 
 #define HEADER <./a.h>
 #include HEADER
 
 struct A {
+  A(const A &) {}
+  A(A &&) {}
 };
 
 struct B {
   B(const A &a) : a(a) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move [modernize-pass-by-value]
 // CHECK-FIXES: B(A a) : a(std::move(a)) {}
   A a;
 };

test/clang-tidy/modernize-pass-by-value.cpp

 // RUN: %check_clang_tidy %s modernize-pass-by-value %t -- -- -std=c++11 -fno-delayed-template-parsing
 
 namespace {
 // POD types are trivially move constructible.
+struct POD {
+  int a, b, c;
+};
+
 struct Movable {
   int a, b, c;
+  Movable() = default;
+  Movable(const Movable &) {}
+  Movable(Movable &&) {}
 };
 
 struct NotMovable {
   NotMovable() = default;
   NotMovable(const NotMovable &) = default;
   NotMovable(NotMovable &&) = delete;
   int a, b, c;
 };
template <typename T> struct N {
 
   Movable M;
   T A;
 };
 
 // Test with value parameter.
 struct O {
   O(Movable M) : M(M) {}
-  // CHECK-FIXES: O(Movable M) : M(M) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move
+  // CHECK-FIXES: O(Movable M) : M(std::move(M)) {}
   Movable M;
 };
 
 // Test with a const-value parameter.
 struct P {
   P(const Movable M) : M(M) {}
   // CHECK-FIXES: P(const Movable M) : M(M) {}
   Movable M;

 };
 
 // Test that value-parameters with a nested name specifier are left as-is.
 namespace ns_R {
 typedef ::Movable RMovable;
 }
 struct R {
   R(ns_R::RMovable M) : M(M) {}
-  // CHECK-FIXES: R(ns_R::RMovable M) : M(M) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move
+  // CHECK-FIXES: R(ns_R::RMovable M) : M(std::move(M)) {}
   ns_R::RMovable M;
 };
 
 // Test with rvalue parameter.
 struct S {
   S(Movable &&M) : M(M) {}
   // CHECK-FIXES: S(Movable &&M) : M(M) {}
   Movable M;
 };
 
 template <typename T, int N> struct array { T A[N]; };
 
 // Test that types that are trivially copyable will not use std::move. This will
 // cause problems with misc-move-const-arg, as it will revert it.
 struct T {
   T(array<int, 10> a) : a_(a) {}
   // CHECK-FIXES: T(array<int, 10> a) : a_(a) {}
   array<int, 10> a_;
 };
+
+struct U {
+  U(const POD &M) : M(M) {}
+  POD M;
+};