[PATCH] New diagnostic for non-idiomatic copy or move operations (v2)

[aaron.ballman]

Summary

All copy operations were created equal (as far as the core language is concerned), but some copy operations are more equal than others (as far as the library is concerned). The same is true of move operations. This patch diagnoses copy and move operations (constructors and assignment operators) that are non-idiomatic so that users are not surprised when their copyable type cannot be placed in a standard library container or used with a standard library algorithm.

Idiomatic copy and move operations are (where T is the class type for the object):
Copy Constructor: takes a const T&
Move constructor: takes a T&&
Copy Assignment: takes a const T&, returns a T&, and has no ref-qualifiers
Move Assignment: takes a T&&, returns a T&, and has no ref-qualifiers

This patch does not diagnose *all* non-idiomatic uses because of false positives in practice. Specifically, it does not diagnose when a non-idiomatic operation is overloaded with an idiomatic one (T(const T&); T(T&);) or when there are multiple non-idiomatic operation overloads (T(volatile T&); T(T&);). In both cases, it is reasonable to assume that the user knows they have non-idiomatic signatures. This means that the diagnostic can always be suppressed by either converting the operation into an idiomatic one, or by adding an overload that is deleted (or is private, as in pre-C++11 code bases), depending on the behavior they desire.

One thing this patch does could have are fix-it hints to correct the types involved. However, I cannot find a way to get the full source range for the type, including qualifiers. For a parameter of type 'const volatile T&', the TypeLoc appears to only track the location of T&, but not the qualifiers. Assistance in understanding how to do this would be appreciated.

[aaron.ballman]

I should note that the majority of the changes in this patch are to deal with the diagnostic triggering on test cases. I tried to correct the test cases to be idiomatic where it appears the signature of the operation was not the purpose of the test. The other cases either got an expected-warning, or had the flag disabled.

I ran the diagnostic over several code bases, and the outcome was:

LLVM: 0 warnings
Qt: 7206 warnings (most of these are duplicate diagnostics because the signatures are in a header file)
rethinkdb: 34 warnings (same here)
cocos2d: 0 warnings
opencv: 0 warnings

[aaron.ballman]

Ping.

[aaron.ballman]

Ping now that the holidays are somewhat over-ish.

[rsmith]

I'm unconvinced this meets the bar for an on-by-default warning. Your suggested workaround for correct-but-diagnosed code doesn't seem to work: if I have

struct X {
  X(const volatile X&);
};

... adding

X(const X&) = delete;

or a private "normal" copy constructor will break clients of my code that happen to have a non-volatile X object. This will also diagnose the pre-C++11 idiom of:

struct noncopyable {
  noncopyable(noncopyable&);
  void operator=(noncopyable&);
};

... where the references are deliberately references to non-const in order to allow more bugs to be caught at compile time.

I'd also expect that if the user wrote more tokens than would be present in the idiomatic declaration, they probably know what they're doing (so don't reject extra cv-qualifiers and ref-qualifiers).

Can you provide a list of the things this found in Qt and rethinkdb? Is there some pattern in them? Are they bugs?

lib/Sema/SemaDeclCXX.cpp

4924–4929

I don't think it makes sense to warn on this when the qualifier is &. In that case, the user's code is *better* than the normal idiom. And if they used the ref-qualifier &&, perhaps we should just assume they know what they're doing?

[aaron.ballman]

In D15456#320946, @rsmith wrote:

I'm unconvinced this meets the bar for an on-by-default warning. Your suggested workaround for correct-but-diagnosed code doesn't seem to work: if I have

struct X {
  X(const volatile X&);
};

... adding

X(const X&) = delete;

or a private "normal" copy constructor will break clients of my code that happen to have a non-volatile X object.

That's a good point. You could use delegating constructors to silence the warning, but that is a bit intrusive (and non-functional for coding targeting older versions of the standard).

This will also diagnose the pre-C++11 idiom of:

struct noncopyable {
  noncopyable(noncopyable&);
  void operator=(noncopyable&);
};

... where the references are deliberately references to non-const in order to allow more bugs to be caught at compile time.

This was intentional, based on discussion. The thought was that (1) these are still copy constructible/assignable (within the context of the class type itself), and (2) they're still not idiomatic. However, I do think that's a bit of a weak argument and would be fine allowing those cases.

I'd also expect that if the user wrote more tokens than would be present in the idiomatic declaration, they probably know what they're doing (so don't reject extra cv-qualifiers and ref-qualifiers).

It would be oddly inconsistent to warn on some non-idiomatic operations, but then fail to warn on other non-idiomatic operations though. However, it may make sense

Can you provide a list of the things this found in Qt and rethinkdb? Is there some pattern in them? Are they bugs?

The Qt issues are things like: http://code.qt.io/cgit/qt/qtbase.git/tree/src/tools/uic/ui4.h?h=dev#n379 However, there are also plenty like:

dom/QualifiedName.h:70:11: warning: non-idiomatic copy assignment operator declaration; consider returning 'T&' instead [-Wnon-idiomatic-copy-move]

const QualifiedName& operator=(const QualifiedName& other) { other.ref(); deref(); m_impl = other.m_impl; return *this; }

Rethink's look more like:

[aaron.ballman]

(Sorry, fat fingered the original response.)

In D15456#320946, @rsmith wrote:

I'm unconvinced this meets the bar for an on-by-default warning. Your suggested workaround for correct-but-diagnosed code doesn't seem to work: if I have

struct X {
  X(const volatile X&);
};

... adding

X(const X&) = delete;

or a private "normal" copy constructor will break clients of my code that happen to have a non-volatile X object.

That's a good point. You could use delegating constructors to silence the warning, but that is a bit intrusive (and non-functional for coding targeting older versions of the standard).

This will also diagnose the pre-C++11 idiom of:

struct noncopyable {
  noncopyable(noncopyable&);
  void operator=(noncopyable&);
};

... where the references are deliberately references to non-const in order to allow more bugs to be caught at compile time.

This was intentional, based on discussion. The thought was that (1) these are still copy constructible/assignable (within the context of the class type itself), and (2) they're still not idiomatic. However, I do think that's a bit of a weak argument and would be fine allowing those cases.

I'd also expect that if the user wrote more tokens than would be present in the idiomatic declaration, they probably know what they're doing (so don't reject extra cv-qualifiers and ref-qualifiers).

It would be oddly inconsistent to warn on some non-idiomatic operations, but then fail to warn on other non-idiomatic operations though. However, it may make sense

Can you provide a list of the things this found in Qt and rethinkdb? Is there some pattern in them? Are they bugs?

The Qt issues are things like: http://code.qt.io/cgit/qt/qtbase.git/tree/src/tools/uic/ui4.h?h=dev#n379 However, there are also plenty like:

dom/QualifiedName.h:70:11: warning: non-idiomatic copy assignment operator declaration; consider returning 'T&' instead [-Wnon-idiomatic-copy-move]

const QualifiedName& operator=(const QualifiedName& other) { other.ref(); deref(); m_impl = other.m_impl; return *this; }

Rethink's look more like:
./src/rapidjson/document.h:329:5: error: non-idiomatic copy assignment operator declaration; consider returning 'T&' instead [-Werror,-Wnon-idiomatic-copy-move]

GenericStringRef operator=(const GenericStringRef&);
^

./src/rapidjson/document.h:595:43: error: non-idiomatic copy assignment operator declaration; consider 'const T&' instead [-Werror,-Wnon-idiomatic-copy-move]

GenericValue& operator=(GenericValue& rhs) RAPIDJSON_NOEXCEPT {
                                      ^

Whether the issues pointed out are bugs or not is a bit more subjective as it depends on the usage. Some seems like they aren't (such as Qt's stuff in ui4.h which are old-style "deleted" functions). Others seem like they definitely are, such as what I pointed out from rethinkdb (though rethink also had some old-style deleted functions as well). And others seem questionable, such as Qt's QualifiedName, where it could go either way.

include/clang/Basic/DiagnosticGroups.td

 def GNUZeroLineDirective : DiagGroup<"gnu-zero-line-directive">;
 def GNUZeroVariadicMacroArguments : DiagGroup<"gnu-zero-variadic-macro-arguments">;
 def Fallback : DiagGroup<"fallback">;
+def NonIdiomaticCopyMove : DiagGroup<"non-idiomatic-copy-move">;
 
 // This covers both the deprecated case (in C++98)
 // and the extension case (in C++11 onwards).

include/clang/Basic/DiagnosticSemaKinds.td

   "%select{copy|move}0 assignment operator of %1 is implicitly deleted "
   "because field %2 is of %select{reference|const-qualified}4 type %3">;
 
+def warn_non_idiomatic_copy_move_ctor : Warning<
+  "non-idiomatic %select{copy|move}0 constructor declaration; consider "
+  "%select{'const T&'|'T&&'}0 instead">,
+  InGroup<NonIdiomaticCopyMove>;
+def warn_non_idiomatic_copy_move_assign : Warning<
+  "non-idiomatic %select{copy|move}0 assignment operator declaration; consider "
+  "%select{returning 'T&'|'%select{const T&|T&&}0'|removing ref-qualifiers}1 "
+  "instead">, InGroup<NonIdiomaticCopyMove>;
+
 // These should be errors.
 def warn_undefined_internal : Warning<
   "%select{function|variable}0 %q1 has internal linkage but is not defined">,

lib/Sema/SemaDeclCXX.cpp

   }
 }
 
+static void DiagnoseNonIdiomaticConstructor(Sema &S,
+                                            const CXXConstructorDecl *Ctor,
+                                            bool Move) {
+  // This is already known to be a copy or move constructor, so there is at
+  // least one parameter of a sensible class type. Check the qualifiers on that
+  // type to determine if they are idiomatic or not.
+  QualType ParamType = Ctor->getParamDecl(0)->getType();
+  assert(ParamType->isReferenceType() &&
+         "Copy or move ctor with non-reference type");
+  QualType RefType =
+      S.Context.getCanonicalType(ParamType.getNonReferenceType());
+
+  if (RefType.isVolatileQualified() ||
+      ((!Move && !RefType.isConstQualified()) ||
+       (Move && RefType.isConstQualified())))
+    S.Diag(Ctor->getParamDecl(0)->getLocation(),
+           diag::warn_non_idiomatic_copy_move_ctor) << Move;
+}
+
+static void DiagnoseNonIdiomaticAssignment(Sema &S,
+                                           const CXXMethodDecl *Assign,
+                                           bool Move) {
+  // Check that there are no ref-qualifiers.
+  if (Assign->getRefQualifier() != RQ_None) {
+    S.Diag(Assign->getLocation(), diag::warn_non_idiomatic_copy_move_assign)
+        << Move << /*ref-qualifier*/ 2;
+    return;
+  }
+
+  // Check that the return type is an lvalue reference type that is an
+  // unqualified reference to the class type.
+  QualType ClassType = S.Context.getTagDeclType(Assign->getParent());
+  QualType ReturnType = Assign->getReturnType();
+  if (!ReturnType->isLValueReferenceType() ||
+      ReturnType.getNonReferenceType().hasQualifiers() ||
+      S.Context.getCanonicalType(ReturnType).getNonReferenceType() !=
+          ClassType) {
+    S.Diag(Assign->getReturnTypeSourceRange().getBegin(),
+           diag::warn_non_idiomatic_copy_move_assign) << Move << /*return*/ 0;
+    return;
+  }
+
+  // This is already known to be a copy or move assignment operator, so there
+  // is at least one parameter of a sensible class type. Check that it is an
+  // idiomatic parameter type. For a move assignment, if the non-reference type
+  // has any qualifiers, it is not idiomatic. For a copy assignment, if the type
+  // is not a reference type, or if it is a reference type that is not solely
+  // const-qualified, it is not idiomatic.
+  QualType ParamType =
+      S.Context.getCanonicalType(Assign->getParamDecl(0)->getType());
+  QualType ParamNonRefType = ParamType.getNonReferenceType();
+  if ((Move && ParamNonRefType.hasQualifiers()) ||
+      (!Move && (!ParamType->isReferenceType() ||
+                 ParamNonRefType.isVolatileQualified() ||
+                 !ParamNonRefType.isConstQualified()))) {
+    S.Diag(Assign->getParamDecl(0)->getLocation(),
+           diag::warn_non_idiomatic_copy_move_assign) << Move << /*param*/ 1;
+    return;
+  }
+}
+
 /// \brief Perform semantic checks on a class definition that has been
 /// completing, introducing implicitly-declared members, checking for
 /// abstract types, etc.

   DeclareInheritingConstructors(Record);
 
   checkClassLevelDLLAttribute(Record);
+
+  // Diagnose non-idiomatic copy/move constructors. Do not diagnose an operation
+  // in the presence of an idiomatic variant or multiple non-idiomatic variants.
+  if (Record->hasUserDeclaredCopyConstructor() ||
+      Record->hasUserDeclaredMoveConstructor()) {
+    SmallVector<const CXXConstructorDecl *, 1> CopyCtors, MoveCtors;
+    for (const auto *Ctor : Record->ctors()) {
+      // If the constructor has default arguments or has been flagged as being
+      // invalid, skip it.
+      if (Ctor->isInvalidDecl() || Ctor->getNumParams() != 1)
+        continue;
+      if (Ctor->isCopyConstructor())
+        CopyCtors.push_back(Ctor);
+      else if (Ctor->isMoveConstructor())
+        MoveCtors.push_back(Ctor);
+    }
+
+    // If there are multiple copy/move constructors (considered separately),
+    // then no diagnostics should be triggered even if some of the constructors
+    // are non-idiomatic. If there is only one such constructor, and it is non-
+    // idiomatic, then diagnose.
+    if (CopyCtors.size() == 1)
+      DiagnoseNonIdiomaticConstructor(*this, CopyCtors.front(), false);
+    if (MoveCtors.size() == 1)
+      DiagnoseNonIdiomaticConstructor(*this, MoveCtors.front(), true);
+  }
+
+  // Diagnose non-idiomatic copy/move assignment operators. Do not diagnose an
+  // operation in the presence of an idiomatic variant or multiple non-idiomatic
+  // variants.
+  if (Record->hasUserDeclaredCopyAssignment() ||
+      Record->hasUserDeclaredMoveAssignment()) {
+    SmallVector<const CXXMethodDecl *, 1> CopyAssigns, MoveAssigns;
+    // If the assignment operator has been flagged as being invalid, skip it.
+    for (const auto *M : Record->methods()) {
+      if (M->isInvalidDecl())
+        continue;
+      if (M->isCopyAssignmentOperator())
+        CopyAssigns.push_back(M);
+      else if (M->isMoveAssignmentOperator())
+        MoveAssigns.push_back(M);
+    }
+
+    // If there are multiple copy/move assignment operators (considered
+    // separately), then no diagnostics should be triggered even if some of the
+    // operators are non-idiomatic. If there is only one such operator, and it
+    // is non-idiomatic, then diagnose.
+    if (CopyAssigns.size() == 1)
+      DiagnoseNonIdiomaticAssignment(*this, CopyAssigns.front(), false);
+    if (MoveAssigns.size() == 1)
+      DiagnoseNonIdiomaticAssignment(*this, MoveAssigns.front(), true);
+  }
+
+  if (Record->hasUserDeclaredCopyAssignment() ||
+      Record->hasUserDeclaredMoveAssignment()) {
+  }
 }
 
 /// Look up the special member function that would be called by a special

test/Analysis/NewDelete-checker-test.cpp

 //----- Other cases
 void testNewMemoryIsInHeap() {
   int *p = new int;
-  if (global != p) // condition is always true as 'p' wraps a heap region that 
+  if (global != p) // condition is always true as 'p' wraps a heap region that
                    // is different from a region wrapped by 'global'
     global = p; // pointer escapes
 }

     explicit shared_ptr(T *p) : p(p), control(new control_block) {
       control->retain();
     }
-    shared_ptr(shared_ptr &other) : p(other.p), control(other.control) {
+    shared_ptr(const shared_ptr &other) : p(other.p), control(other.control) {
       if (control)
           control->retain();
     }

test/Analysis/inlining/path-notes.cpp

-// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=text -analyzer-config c++-inlining=destructors -std=c++11 -verify -Wno-tautological-undefined-compare %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=plist-multi-file -analyzer-config c++-inlining=destructors -std=c++11 -analyzer-config path-diagnostics-alternate=false %s -o %t.plist -Wno-tautological-undefined-compare
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=text -analyzer-config c++-inlining=destructors -std=c++11 -verify -Wno-non-idiomatic-copy-move -Wno-tautological-undefined-compare %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=plist-multi-file -analyzer-config c++-inlining=destructors -std=c++11 -analyzer-config path-diagnostics-alternate=false %s -o %t.plist -Wno-non-idiomatic-copy-move -Wno-tautological-undefined-compare
 // RUN: FileCheck --input-file=%t.plist %s
 
 class Foo {

test/Analysis/operator-calls.cpp

-// RUN: %clang_cc1 -std=c++11 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -verify %s
+// RUN: %clang_cc1 -std=c++11 -Wno-non-idiomatic-copy-move -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -verify %s
 void clang_analyzer_eval(bool);
 
 struct X0 { };

test/CXX/basic/basic.types/p10.cpp

 
 struct BeingDefined;
 extern BeingDefined beingdefined;
-struct BeingDefined { 
+struct BeingDefined {
   static constexpr BeingDefined& t = beingdefined;
 };
 

   template<typename T> constexpr CtorTemplate(T);
 };
 struct CopyCtorOnly { // expected-note {{'CopyCtorOnly' is not literal because it is not an aggregate and has no constexpr constructors other than copy or move constructors}}
-  constexpr CopyCtorOnly(CopyCtorOnly&);
+  constexpr CopyCtorOnly(const CopyCtorOnly&);
 };
 constexpr int f(CopyCtorOnly) { return 0; } // expected-error {{'CopyCtorOnly' is not a literal type}}
 struct MoveCtorOnly { // expected-note {{no constexpr constructors other than copy or move constructors}}

test/CXX/class.access/p4.cpp

 
   A a; // expected-error {{calling a private constructor}}
   A A::foo; // okay
-  
   class B : A { }; // expected-error {{base class 'test2::A' has private default constructor}}
   B b; // expected-note{{implicit default constructor}}
-  
-  class C : virtual A { 
+  class C : virtual A {
   public:
     C();
   };

     virtual Base<1>, // expected-error 2 {{base class 'Base<1>' has private destructor}}
     Base2, // expected-error 2 {{base class 'test3::Base2' has private destructor}}
     virtual Base3
-  {}; 
+  {};
   Derived3 d3; // expected-note {{implicit default constructor}}\
                // expected-note{{implicit destructor}}}
 }

 // Implicit copy assignment operator uses.
 namespace test5 {
   class A {
-    void operator=(const A &); // expected-note 2 {{implicitly declared private here}}
+    A& operator=(const A &); // expected-note 2 {{implicitly declared private here}}
   };
 
   class Test1 { A a; }; // expected-error {{private member}}
   void test1() {
-    Test1 a; 
+    Test1 a;
     a = Test1(); // expected-note{{implicit copy}}
   }
 

   };
 }
 
-// Ignored operator new and delete overloads are not 
+// Ignored operator new and delete overloads are not
 namespace test8 {
   typedef __typeof__(sizeof(int)) size_t;
 

   class X {
     ~X(); // expected-note {{declared private here}}
   };
-  
   struct Y1 {
     operator X();
   };
-  
   void g() {
     const X &xr = Y1(); // expected-error{{temporary of type 'test14::X' has private destructor}}
   }

   };
 
   template class A<int>;
-  template class A<long>; // expected-note 4 {{in instantiation}} 
+  template class A<long>; // expected-note 4 {{in instantiation}}
 
   template <class T> class B : public A<T> {
     // TODO: These first two accesses can be detected as ill-formed at

 // PR8325
 namespace test19 {
   class A { ~A(); };
-  // The destructor is not implicitly referenced here.  Contrast to test16, 
+  // The destructor is not implicitly referenced here.  Contrast to test16,
   // testing PR7281, earlier in this file.
   void b(A* x) { throw x; }
 }

test/CXX/class/class.union/p1.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify %s 
+// RUN: %clang_cc1 -fsyntax-only -verify %s
 
 void abort() __attribute__((noreturn));
 

 };
 
 class CopyCtor { // expected-note 2{{because no constructor can be used to copy an object of type 'const CopyCtor'}}
-  CopyCtor(CopyCtor &cc) { abort(); }
+  CopyCtor(CopyCtor &cc) { abort(); } // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
 };
 
 class CopyAssign { // expected-note 2 {{because no assignment operator can be used to copy an object of type 'const CopyAssign'}}
-  CopyAssign& operator=(CopyAssign& CA) { abort(); }
+  CopyAssign& operator=(CopyAssign& CA) { abort(); } // expected-warning {{non-idiomatic copy assignment operator declaration; consider 'const T&' instead}}
 };
 
 class Dtor {

test/CXX/class/p6-0x.cpp

 class Trivial { int n; void f(); };
 class NonTrivial1 { NonTrivial1(const NonTrivial1 &); };
 class NonTrivial2 { NonTrivial2(NonTrivial2 &&); };
-class NonTrivial3 { NonTrivial3 operator=(const NonTrivial3 &); };
-class NonTrivial4 { NonTrivial4 operator=(NonTrivial4 &&); };
+class NonTrivial3 { NonTrivial3& operator=(const NonTrivial3 &); };
+class NonTrivial4 { NonTrivial4& operator=(NonTrivial4 &&); };
 class NonTrivial5 { ~NonTrivial5(); };
 
 static_assert(__is_trivial(Trivial), "Trivial is not trivial");

test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p1.cpp

-// RUN: %clang_cc1 -verify %s -std=c++11
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move %s -std=c++11
 
 // A function that is explicitly defaulted shall
 struct A {

test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp

   X3();
 
 private:
-  X3(X3&); // expected-note{{candidate constructor not viable: expects an l-value for 1st argument}}
+  X3(X3&); // expected-note{{candidate constructor not viable: expects an l-value for 1st argument}} \
+           // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
 };
 
 // Check for instantiation of default arguments

 struct X4 {
   X4();
   X4(const X4&, T = get_value_badly<T>()); // expected-note{{in instantiation of}}
-}; 
+};
 
 // Check for "dangerous" default arguments that could cause recursion.
 struct X5 {

test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx0x-no-extra-copy.cpp

   X3();
 
 private:
-  X3(X3&);
+  X3(X3&); // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
 };
 
 template<typename T>

test/CXX/dcl.decl/dcl.meaning/dcl.fct/dcl.fct.def.default/p2.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-non-idiomatic-copy-move -std=c++11 %s
 
 // FIXME: test with non-std qualifiers
 

test/CXX/drs/dr0xx.cpp

   };
   const volatile B b = a;
 
-  struct C { C(C&); };
+  struct C { C(C&); }; // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
   struct D : C {};
   struct E { operator D&(); } e;
   const C c = e;

   struct A { operator B() const; };
   struct C {};
   struct B {
-    B(B&); // expected-note {{candidate}}
+    B(B&); // expected-note {{candidate}} \
+           // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
     B(C);
     operator C() const;
   };

test/CXX/drs/dr1xx.cpp

 namespace dr177 { // dr177: yes
   struct B {};
   struct A {
-    A(A &); // expected-note {{not viable: expects an l-value}}
+    A(A &); // expected-note {{not viable: expects an l-value}} \
+            // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
     A(const B &);
   };
   B b;

test/CXX/drs/dr3xx.cpp

 
 namespace dr331 { // dr331: yes
   struct A {
-    A(volatile A&); // expected-note {{candidate}}
+    A(volatile A&); // expected-note {{candidate}} \
+                    // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
   } const a, b(a); // expected-error {{no matching constructor}}
 }
 

 }
 
 namespace dr385 { // dr385: yes
-  struct A { protected: void f(); }; 
+  struct A { protected: void f(); };
   struct B : A { using A::f; };
   struct C : A { void g(B b) { b.f(); } };
   void h(B b) { b.f(); }

test/CXX/drs/dr5xx.cpp

   template<typename T> void b3(Base<T> *);
 
   void test(int n, const int cn, int **p, int *S::*pm) {
-    int *a[3], *S::*am[3]; 
+    int *a[3], *S::*am[3];
     const Derived cd = Derived();
     Derived d[3];
 

   };
 #if __cplusplus >= 201103L
   struct C {
-    C &operator=(const C&) &; // expected-note {{not viable}} expected-note {{nearly matches}} expected-note {{here}}
+    C &operator=(const C&) &; // expected-note {{not viable}} expected-note {{nearly matches}} expected-note {{here}} \
+                              // expected-warning {{non-idiomatic copy assignment operator declaration; consider removing ref-qualifiers instead}}
   };
   struct D {
-    D &operator=(const D&) &&; // expected-note {{not viable}} expected-note {{nearly matches}} expected-note {{here}}
+    D &operator=(const D&) &&; // expected-note {{not viable}} expected-note {{nearly matches}} expected-note {{here}} \
+                               // expected-warning {{non-idiomatic copy assignment operator declaration; consider removing ref-qualifiers instead}}
   };
   void test(C c, D d) {
     c = c;

test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp

 
 class NonConstCopy {
 public:
-  NonConstCopy(NonConstCopy&); // expected-note{{would lose const}}
+  NonConstCopy(NonConstCopy&); // expected-note{{would lose const}} \
+                               // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
 };
 
 void capture_by_copy(NonCopyable nc, NonCopyable &ncr, const NonConstCopy nco) {
   (void)[nc] { }; // expected-error{{capture of variable 'nc' as type 'NonCopyable' calls private copy constructor}}
   (void)[=] {
-    ncr.foo(); // expected-error{{capture of variable 'ncr' as type 'NonCopyable' calls private copy constructor}} 
+    ncr.foo(); // expected-error{{capture of variable 'ncr' as type 'NonCopyable' calls private copy constructor}}
   }();
 
   [nco] {}(); // expected-error{{no matching constructor for initialization of 'const NonConstCopy'}}

test/CXX/expr/expr.prim/expr.prim.lambda/p21.cpp

-// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -verify -Wno-non-idiomatic-copy-move
 // expected-no-diagnostics
 
 struct DirectInitOnly {

test/CXX/special/class.copy/implicit-move.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-non-idiomatic-copy-move %s
 
 // Tests for implicit (non-)declaration of move constructor and
 // assignment: p9, p11, p20, p23.

   static_assert(!noexcept(cntco = ContainsNonTrivialCopyOnly()), "");
 
   ContainsConst cc;
-  cc = ContainsConst(); // expected-error {{no viable}} 
+  cc = ContainsConst(); // expected-error {{no viable}}
 
   ContainsRef cr;
-  cr = ContainsRef(); // expected-error {{no viable}} 
+  cr = ContainsRef(); // expected-error {{no viable}}
 
   DirectVirtualBase dvb;
-  dvb = DirectVirtualBase(); // expected-error {{no viable}} 
+  dvb = DirectVirtualBase(); // expected-error {{no viable}}
 
   IndirectVirtualBase ivb;
-  ivb = IndirectVirtualBase(); // expected-error {{no viable}} 
+  ivb = IndirectVirtualBase(); // expected-error {{no viable}}
 }
 
 struct ContainsRValueRef {

test/CXX/special/class.copy/p11.0x.copy.cpp

 HasAccess HAb(HAa);
 
 struct NonConst {
-  NonConst(NonConst&);
+  NonConst(NonConst&); // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
 };
 struct Ambiguity {
   Ambiguity(const Ambiguity&);

test/CXX/special/class.copy/p12-0x.cpp

-// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s -Wno-non-idiomatic-copy-move
 
 // expected-no-diagnostics
 

test/CXX/special/class.copy/p18-cxx11.cpp

-// RUN: %clang_cc1 -std=c++11 %s -verify
+// RUN: %clang_cc1 -std=c++11 %s -verify -Wno-non-idiomatic-copy-move
 // expected-no-diagnostics
 
 // C++98 [class.copy]p10 / C++11 [class.copy]p18.

test/CXX/special/class.copy/p20.cpp

 
 struct NonConstCopy {
   NonConstCopy();
-  NonConstCopy &operator=(NonConstCopy&);
+  NonConstCopy &operator=(NonConstCopy&); // expected-warning {{non-idiomatic copy assignment operator declaration; consider 'const T&' instead}}
 };
 
-struct VirtualInheritsNonConstCopy : virtual NonConstCopy { 
+struct VirtualInheritsNonConstCopy : virtual NonConstCopy {
   VirtualInheritsNonConstCopy();
   VirtualInheritsNonConstCopy &operator=(const VirtualInheritsNonConstCopy&);
 };

   NonConstCopy ncc_array[2][3];
 };
 
-struct ImplicitNonConstCopy4 : VirtualInheritsNonConstCopy { 
+struct ImplicitNonConstCopy4 : VirtualInheritsNonConstCopy {
   ImplicitNonConstCopy4();
 };
 

test/CXX/special/class.copy/p23-cxx11.cpp

   InaccessibleMoveAssign &operator=(InaccessibleMoveAssign &&);
 };
 class NonConstCopyAssign {
-  NonConstCopyAssign &operator=(NonConstCopyAssign &);
+  NonConstCopyAssign &operator=(NonConstCopyAssign &); // expected-warning {{non-idiomatic copy assignment operator declaration; consider 'const T&' instead}}
 };
 
 // A defaulted copy/move assignment operator for class X is defined as deleted

test/CXX/special/class.copy/p25-0x.cpp

-// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s -Wno-non-idiomatic-copy-move
 
 // expected-no-diagnostics
 

test/CXX/special/class.copy/p8-cxx11.cpp

-// RUN: %clang_cc1 -std=c++11 %s -verify
+// RUN: %clang_cc1 -std=c++11 %s -verify -Wno-non-idiomatic-copy-move
 // expected-no-diagnostics
 
 // C++98 [class.copy]p5 / C++11 [class.copy]p8.

test/CXX/special/class.copy/p9.cpp

 
 struct NonConstCopy {
   NonConstCopy();
-  NonConstCopy(NonConstCopy&);
+  NonConstCopy(NonConstCopy&); // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
 };
 
-struct VirtualInheritsNonConstCopy : virtual NonConstCopy { 
+struct VirtualInheritsNonConstCopy : virtual NonConstCopy {
   VirtualInheritsNonConstCopy();
   VirtualInheritsNonConstCopy(const VirtualInheritsNonConstCopy&);
 };

test/CXX/stmt.stmt/stmt.dcl/p3-0x.cpp

   {
     Y y2; // expected-note{{jump bypasses variable with a non-trivial destructor}}
   inner:
-    f();    
+    f();
   }
   return;
 }
 
 struct Z {
-  Z operator=(const Z&);
+  Z& operator=(const Z&);
 };
 
 void test_Z() {

test/CXX/stmt.stmt/stmt.dcl/p3.cpp

 }
 
 struct Z {
-  Z operator=(const Z&);
+  Z& operator=(const Z&);
 };
 
 void test_Z() {

test/CXX/temp/temp.fct.spec/temp.deduct/cwg1170.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wno-non-idiomatic-copy-move %s
 // expected-no-diagnostics
 
 #if !__has_feature(cxx_access_control_sfinae)

test/CodeGenCXX/copy-assign-synthesis-3.cpp

-// RUN: %clang_cc1 -emit-llvm-only -verify %s
+// RUN: %clang_cc1 -emit-llvm-only -verify -Wno-non-idiomatic-copy-move %s
 // expected-no-diagnostics
 
 struct A {

test/OpenMP/for_firstprivate_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
 
 void foo() {
 }

test/OpenMP/for_lastprivate_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -fopenmp -Wno-non-idiomatic-copy-move %s
 
 void foo() {
 }

test/OpenMP/for_reduction_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 150 -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++98 -ferror-limit 150 -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++11 -ferror-limit 150 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -ferror-limit 150 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++98 -ferror-limit 150 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++11 -ferror-limit 150 -o - %s
 
 void foo() {
 }

test/OpenMP/for_simd_lastprivate_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
 
 void foo() {
 }

test/OpenMP/for_simd_reduction_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++98 %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++11 %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++98 %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++11 %s
 
 void foo() {
 }

test/OpenMP/parallel_copyin_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -ferror-limit 100 -o - %s
 
 void foo() {
 }

test/OpenMP/parallel_for_copyin_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -ferror-limit 100 -o - %s
 
 void foo() {
 }

test/OpenMP/parallel_for_lastprivate_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
 
 void foo() {
 }

test/OpenMP/parallel_for_reduction_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++98 -ferror-limit 100 -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++11 -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++98 -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++11 -ferror-limit 100 -o - %s
 
 void foo() {
 }

test/OpenMP/parallel_for_simd_copyin_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -o - %s
 
 void foo() {
 }

test/OpenMP/parallel_for_simd_lastprivate_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
 
 void foo() {
 }

test/OpenMP/parallel_for_simd_reduction_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++98 -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++11 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++98 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++11 -o - %s
 
 void foo() {
 }

test/OpenMP/parallel_reduction_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++98 -ferror-limit 100 -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++11 -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++98 -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++11 -ferror-limit 100 -o - %s
 
 void foo() {
 }

test/OpenMP/parallel_sections_copyin_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -ferror-limit 100 -o - %s
 
 void foo() {
 }

test/OpenMP/parallel_sections_lastprivate_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
 
 void foo() {
 }

test/OpenMP/parallel_sections_reduction_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++98 -ferror-limit 100 -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++11 -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++98 -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++11 -ferror-limit 100 -o - %s
 
 void foo() {
 }

test/OpenMP/parallel_sections_shared_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -ferror-limit 100 %s
 
 void foo() {
 }

test/OpenMP/parallel_shared_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -ferror-limit 100 %s
 
 void foo() {
 }

test/OpenMP/sections_lastprivate_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
 
 void foo() {
 }

test/OpenMP/sections_reduction_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 150 -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++98 -ferror-limit 150 -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++11 -ferror-limit 150 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -ferror-limit 150 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++98 -ferror-limit 150 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++11 -ferror-limit 150 -o - %s
 
 void foo() {
 }

test/OpenMP/simd_lastprivate_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
 
 void foo() {
 }

test/OpenMP/simd_reduction_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++98 %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++11 %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++98 %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++11 %s
 
 void foo() {
 }

test/OpenMP/single_copyprivate_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
 
 void foo() {
 }

test/OpenMP/target_map_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -ferror-limit 100 %s
 
 void foo() {
 }

 #pragma omp target map(to, x)
   foo();
 #pragma omp target map(to x) // expected-error {{expected ',' or ')' in 'map' clause}}
-#pragma omp target map(tofrom: argc > 0 ? x : y) // expected-error 2 {{expected variable name, array element or array section}} 
+#pragma omp target map(tofrom: argc > 0 ? x : y) // expected-error 2 {{expected variable name, array element or array section}}
 #pragma omp target map(argc)
 #pragma omp target map(S1) // expected-error {{'S1' does not refer to a value}}
 #pragma omp target map(a, b, c, d, f) // expected-error {{incomplete type 'S1' where a complete type is required}} expected-error 2 {{type 'S2' is not mappable to target}}

test/OpenMP/task_shared_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -ferror-limit 100 %s
 
 void foo() {
 }

test/OpenMP/taskloop_firstprivate_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
 
 void foo() {
 }

test/OpenMP/taskloop_lastprivate_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
 
 void foo() {
 }

test/OpenMP/taskloop_simd_firstprivate_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
 
 void foo() {
 }

test/OpenMP/taskloop_simd_lastprivate_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
 
 void foo() {
 }

test/OpenMP/teams_reduction_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++98 -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -std=c++11 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++98 -o - %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp -std=c++11 -o - %s
 
 void foo() {
 }

test/OpenMP/teams_shared_messages.cpp

-// RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -Wno-non-idiomatic-copy-move -fopenmp %s
 
 void foo() {
 }

test/Parser/cxx0x-ambig.cpp

     constexpr T() {}
     constexpr T(int) {}
     constexpr T(T, T, T, T) {}
-    constexpr T operator=(T) const { return *this; }
+    constexpr T operator=(T) const { return *this; } // expected-warning {{non-idiomatic copy assignment operator declaration; consider returning 'T&' instead}}
     constexpr operator int() const { return 4; }
   };
   constexpr T a, b, c, d;

test/SemaCUDA/implicit-copy.cu

 // RUN: %clang_cc1 -std=gnu++11 -triple nvptx64-unknown-unknown -fcuda-is-device -fsyntax-only -verify %s
 
 struct CopyableH {
-  const CopyableH& operator=(const CopyableH& x) { return *this; }
+  const CopyableH& operator=(const CopyableH& x) { return *this; } // expected-warning {{non-idiomatic copy assignment operator declaration; consider returning 'T&' instead}}
 };
 struct CopyableD {
-  __attribute__((device)) const CopyableD& operator=(const CopyableD x) { return *this; }
+  __attribute__((device)) const CopyableD& operator=(const CopyableD x) { return *this; } // expected-warning {{non-idiomatic copy assignment operator declaration; consider returning 'T&' instead}}
 };
 
 struct SimpleH {

test/SemaCUDA/implicit-member-target.cu

 
 struct A5_copy_ctor_constness {
   __host__ A5_copy_ctor_constness() {}
-  __host__ A5_copy_ctor_constness(A5_copy_ctor_constness&) {}
+  __host__ A5_copy_ctor_constness(A5_copy_ctor_constness&) {} // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
 };
 
 struct B5_copy_ctor_constness : A5_copy_ctor_constness {

test/SemaCXX/MicrosoftCompatibility.cpp

     }
   }
 
- 
 void exception_jump() {
   goto l2; // expected-error {{cannot jump}}
   try { // expected-note {{jump bypasses initialization of try block}}

  a0:
   return 0;
 }
-  
 }
 
 namespace PR11826 {
   struct pair {
     pair(int v) { }
-    void operator=(pair&& rhs) { }
+    pair& operator=(pair&& rhs) { return *this; }
   };
   void f() {
     pair p0(3);

 namespace ms_using_declaration_bug {
 
 class A {
-public: 
-  int f(); 
+public:
+  int f();
 };
 
 class B : public A {
-private:   
+private:
   using A::f;
   void g() {
     f(); // no diagnostic

   }
 };
 
-class C : public B { 
-private:   
+class C : public B {
+private:
   using B::f; // expected-warning {{using declaration referring to inaccessible member 'ms_using_declaration_bug::B::f' (which refers to accessible member 'ms_using_declaration_bug::A::f') is a Microsoft compatibility extension}}
 };
 

test/SemaCXX/conditional-expr.cpp

 
 struct B;
 struct A {
-  A(); 
+  A();
   A(const B&); // expected-note 2 {{candidate constructor}}
-}; 
+};
 struct B { operator A() const; }; // expected-note 2 {{candidate function}}
 struct I { operator int(); };
 struct J { operator I(); };

 
   (void)&(i1 ? flds.b1 : flds.i1); // expected-error {{address of bit-field requested}}
   (void)&(i1 ? flds.i1 : flds.b1); // expected-error {{address of bit-field requested}}
-  
 
+
   unsigned long test0 = 5;
   test0 = test0 ? (long) test0 : test0; // expected-warning {{operand of ? changes signedness: 'long' to 'unsigned long'}}
   test0 = test0 ? (int) test0 : test0; // expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}

 
   struct Foo3 {
     Foo3();
-    Foo3(Foo3&); // expected-note{{would lose const qualifier}}
+    Foo3(Foo3&); // expected-note{{would lose const qualifier}} \
+                 // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
   };
 
   struct Bar {

 }
 
 namespace rdar7998817 {
-  class X { 
-    X(X&); // expected-note{{declared private here}}
+  class X {
+    X(X&); // expected-note{{declared private here}} \
+           // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
 
     struct ref { };
 

   public:
     X();
     X(ref);
-    
     operator ref();
   };
 

test/SemaCXX/conversion-function.cpp

-// RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -Wbind-to-temporary-copy -verify %s 
-class X { 
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -Wbind-to-temporary-copy -Wno-non-idiomatic-copy-move -verify %s
+class X {
 public:
   operator bool();
   operator int() const;

   // expected-error{{conversion function cannot have any parameters}}
 
   operator bool(int a = 4, int b = 6) const; // expected-error{{conversion function cannot have any parameters}}
-  
-  
   operator float(...) const;  // expected-error{{conversion function cannot be variadic}}
-  
-  
   operator func_type(); // expected-error{{conversion function cannot convert to a function type}}
   operator array_type(); // expected-error{{conversion function cannot convert to an array type}}
 };

 typedef int INT;
 typedef INT* INT_PTR;
 
-class Z { 
+class Z {
   operator int(); // expected-note {{previous declaration is here}}
   operator int**(); // expected-note {{previous declaration is here}}
-  
   operator INT();  // expected-error{{conversion function cannot be redeclared}}
   operator INT_PTR*(); // expected-error{{conversion function cannot be redeclared}}
 };

 }
 
 // Test. Conversion in base class is visible in derived class.
-class XB { 
+class XB {
 public:
   operator int(); // expected-note {{candidate function}}
 };
 
-class Yb : public XB { 
+class Yb : public XB {
 public:
   operator char(); // expected-note {{candidate function}}
 };

 
 class AutoPtr {
   AutoPtr(AutoPtr &); // expected-note{{declared private here}}
-  
 public:
   AutoPtr();
   AutoPtr(AutoPtrRef);
-  
   operator AutoPtrRef();
 };
 

 
 AutoPtr test_auto_ptr(bool Cond) {
   AutoPtr p1( make_auto_ptr() );
-  
   AutoPtr p;
   if (Cond)
     return p; // expected-error{{calling a private constructor}}
-  
   return AutoPtr();
 }
 

     const A<float, int> &caf2 = E();
   }
 
-  // Check 
+  // Check
   template<typename T>
   struct E2 {
     operator T

 }
 
 namespace smart_ptr {
-  class Y { 
+  class Y {
     class YRef { };
 
     Y(Y&);

 };
 
 struct Other {
-  Other(const Other &); 
+  Other(const Other &);
   Other();
 };
 

   struct Y {
     Y(X);
   };
-  
   Y f2(foo());
 }
 

 
   struct Derived2 : Base { };
 
-  struct SuperDerived : Derived1, Derived2 { 
+  struct SuperDerived : Derived1, Derived2 {
     using Derived1::operator int;
   };
 

     operator int();
   };
 
-  struct Derived23 : Base2, Base3 { 
+  struct Derived23 : Base2, Base3 {
     using Base2::operator int;
   };
 

   {
     template<class Container>
     operator Container()
-    { 
+    {
       Container ar;
       T* i;
       ar[0]=*i;

test/SemaCXX/copy-assignment.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify %s 
+// RUN: %clang_cc1 -fsyntax-only -verify %s
 struct A {
 };
 

 };
 
 struct B {
-  B& operator=(B&);  // expected-note 4 {{candidate function}}
+  B& operator=(B&);  // expected-note 4 {{candidate function}} \
+                     // expected-warning {{non-idiomatic copy assignment operator declaration; consider 'const T&' instead}}
 };
 
 struct ConvertibleToB {

test/SemaCXX/copy-initialization.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify %s 
+// RUN: %clang_cc1 -fsyntax-only -verify %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
 

 namespace PR6757 {
   struct Foo {
     Foo();
-    Foo(Foo&); // expected-note{{candidate constructor not viable}}
+    Foo(Foo&); // expected-note{{candidate constructor not viable}} \
+               // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
   };
 
   struct Bar {

test/SemaCXX/coroutines.cpp

     co_return 0; // expected-error {{'co_return' cannot be used in a destructor}}
   }
   // FIXME: The spec says this is ill-formed.
-  void operator=(CtorDtor&) {
+  void operator=(CtorDtor&) { // expected-warning {{non-idiomatic copy assignment operator declaration; consider returning 'T&' instead}}
     co_yield 0;
   }
 };

test/SemaCXX/cxx0x-cursory-default-delete.cpp

-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wno-non-idiomatic-copy-move -verify %s
 
 struct non_copiable {
   non_copiable(const non_copiable&) = delete; // expected-note {{marked deleted here}}

test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp

     template<typename T>
     X(std::initializer_list<int>, T);
   };
-  
   X x({}, 17);
 }
 

   struct X {
     X(int i) {}
     X(const X& x) = delete; // expected-note {{here}}
-    void operator=(const X& x) = delete;
+    X& operator=(const X& x) = delete;
   };
 
   std::initializer_list<X> x{1}; // expected-error {{invokes deleted constructor}}

test/SemaCXX/cxx98-compat-flags.cpp

 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wc++98-compat-pedantic -verify %s
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wc++98-compat-pedantic -Wno-bind-to-temporary-copy -Wno-unnamed-type-template-args -Wno-local-type-template-args -Werror %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wc++98-compat-pedantic -Wno-bind-to-temporary-copy -Wno-unnamed-type-template-args -Wno-local-type-template-args -Wno-non-idiomatic-copy-move -Werror %s
 
 template<typename T> int TemplateFn(T) { return 0; }
 void LocalTemplateArg() {

   };
   struct NoViable {
     NoViable();
-    NoViable(NoViable&); // expected-note {{not viable}}
+    NoViable(NoViable&); // expected-note {{not viable}} \
+                         // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
   };
   struct Ambiguous {
     Ambiguous();

test/SemaCXX/cxx98-compat-pedantic.cpp

 // RUN: %clang_cc1 -fsyntax-only -std=c++1y -DCXX1Y -Wc++98-compat-pedantic -verify %s -DCXX1Y2
-// RUN: %clang_cc1 -fsyntax-only -std=c++1y -DCXX1Y -Wc++98-compat -Werror %s -DCXX1Y2
+// RUN: %clang_cc1 -fsyntax-only -std=c++1y -DCXX1Y -Wc++98-compat -Werror -Wno-non-idiomatic-copy-move %s -DCXX1Y2
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wc++98-compat-pedantic -verify %s
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wc++98-compat -Werror %s
-// RUN: %clang_cc1 -fsyntax-only -std=c++98 -Werror %s -DCXX98
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wc++98-compat -Werror -Wno-non-idiomatic-copy-move %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++98 -Werror -Wno-non-idiomatic-copy-move %s -DCXX98
 
 // RUN: %clang_cc1 -fsyntax-only -std=c++1y -Wc++98-compat-pedantic -verify %s -Wno-c++98-c++11-compat-pedantic -DCXX1Y2
 

   };
   struct NoViable {
     NoViable();
-    NoViable(NoViable&); // expected-note {{not viable}}
+    NoViable(NoViable&); // expected-note {{not viable}} \
+                         // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
   };
   struct Ambiguous {
     Ambiguous();

test/SemaCXX/cxx98-compat.cpp

   };
 
   struct b {
-    void operator=(const b &it) {} // expected-note {{because type 'AssignOpUnion::b' has a user-provided copy assignment operator}}
+    b& operator=(const b &it) { return *this; } // expected-note {{because type 'AssignOpUnion::b' has a user-provided copy assignment operator}}
   };
 
   union test1 {

test/SemaCXX/default-assignment-operator.cpp

 
 class X  : Base {  // // expected-error {{cannot define the implicit copy assignment operator for 'X', because non-static const member 'cint' cannot use copy assignment operator}} \
 // expected-note{{assignment operator for 'Base' first required here}}
-public: 
+public:
   X();
   const int cint;  // expected-note {{declared here}}
-}; 
+};
 
-struct Y  : X { 
+struct Y  : X {
   Y();
   Y& operator=(const Y&);
   Y& operator=(volatile Y&);
   Y& operator=(const volatile Y&);
   Y& operator=(Y&);
-}; 
+};
 
 class Z : Y {};
 

 class V {
 public:
   V();
-  V &operator = (V &b);
+  V &operator = (V &b); // expected-warning {{non-idiomatic copy assignment operator declaration; consider 'const T&' instead}}
 };
 
 class W : V {};

 class B1 {
 public:
   B1();
-  B1 &operator = (B1 b);
+  B1 &operator = (B1 b); // expected-warning {{non-idiomatic copy assignment operator declaration; consider 'const T&' instead}}
 };
 
 class D1 : B1 {};

 
 public:
   const int a; // expected-note{{declared here}}
-  E1() : a(0) {}  
+  E1() : a(0) {}
 
 };
 

 }
 
 namespace MultiplePaths {
-  struct X0 { 
+  struct X0 {
     X0 &operator=(const X0&);
   };
 

test/SemaCXX/lambda-expressions.cpp

     (void) ^{ return []() { return f[2]; }; }; // expected-error {{variable 'f' cannot be implicitly captured in a lambda with no capture-default specified}} \
     // expected-note{{lambda expression begins here}}
 
-    struct G { G(); G(G&); int a; }; // expected-note 6 {{not viable}}
+    struct G { G(); G(G&); int a; }; // expected-note 6 {{not viable}} \
+                                     // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
     G g;
     [=]() { const G* gg = &g; return gg->a; };
     [=]() { return [=]{ const G* gg = &g; return gg->a; }(); }; // expected-error {{no matching constructor for initialization of 'G'}}

   template<class T>
   struct L {
       T t{};
-      T t2 = ([](int a) { return [](int b) { return b; };})(t)(t);    
+      T t2 = ([](int a) { return [](int b) { return b; };})(t)(t);
   };
-  L<int> l; 
-  
+  L<int> l;
   namespace non_template {
     struct L {
       int t = 0;
-      int t2 = ([](int a) { return [](int b) { return b; };})(t)(t);    
+      int t2 = ([](int a) { return [](int b) { return b; };})(t)(t);
     };
-    L l; 
+    L l;
   }
 }
 

test/SemaCXX/overload-call-copycon.cpp

 class A {
 public:
   A(A&); // expected-note{{would lose const qualifier}} \
-         // expected-note{{no known conversion}}
+         // expected-note{{no known conversion}} \
+         // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
 };
 
 class B : public A { }; // expected-note{{would lose const qualifier}} \

test/SemaCXX/static-cast.cpp

 
 struct X1 {
   X1();
-  X1(X1&);
+  X1(const X1&);
   X1(const X0&);
-  
   operator X0() const;
 };
 

 
 struct X4 {
   typedef const X3 X3_typedef;
-  
   void f() const {
     (void)static_cast<X3_typedef*>(x2);
   }
-  
   const X2 *x2;
 };
 

 void PR5897() { (void)static_cast<const int(*)[1]>((const void*)0); }
 
 namespace PR6072 {
-  struct A { }; 
+  struct A { };
   struct B : A { void f(int); void f(); };  // expected-note 2{{candidate function}}
   struct C : B { };
   struct D { };

test/SemaCXX/type-traits.cpp

-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++11 -fms-extensions -Wno-microsoft %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++11 -fms-extensions -Wno-microsoft -Wno-non-idiomatic-copy-move %s
 #define T(b) (b) ? 1 : -1
 #define F(b) (b) ? -1 : 1
 

 struct HasCons { HasCons(int); };
 struct HasCopyAssign { HasCopyAssign operator =(const HasCopyAssign&); };
 struct HasMoveAssign { HasMoveAssign operator =(const HasMoveAssign&&); };
-struct HasNoThrowMoveAssign { 
+struct HasNoThrowMoveAssign {
   HasNoThrowMoveAssign& operator=(
     const HasNoThrowMoveAssign&&) throw(); };
-struct HasNoExceptNoThrowMoveAssign { 
+struct HasNoExceptNoThrowMoveAssign {
   HasNoExceptNoThrowMoveAssign& operator=(
-    const HasNoExceptNoThrowMoveAssign&&) noexcept; 
+    const HasNoExceptNoThrowMoveAssign&&) noexcept;
 };
-struct HasThrowMoveAssign { 
+struct HasThrowMoveAssign {
   HasThrowMoveAssign& operator=(
     const HasThrowMoveAssign&&) throw(POD); };
-struct HasNoExceptFalseMoveAssign { 
+struct HasNoExceptFalseMoveAssign {
   HasNoExceptFalseMoveAssign& operator=(
     const HasNoExceptFalseMoveAssign&&) noexcept(false); };
 struct HasMoveCtor { HasMoveCtor(const HasMoveCtor&&); };

 struct HasStaticMemberMoveCtor { static HasMoveCtor member; };
 struct HasStaticMemberMoveAssign { static HasMoveAssign member; };
 struct HasMemberThrowMoveAssign { HasThrowMoveAssign member; };
-struct HasMemberNoExceptFalseMoveAssign { 
+struct HasMemberNoExceptFalseMoveAssign {
   HasNoExceptFalseMoveAssign member; };
 struct HasMemberNoThrowMoveAssign { HasNoThrowMoveAssign member; };
-struct HasMemberNoExceptNoThrowMoveAssign { 
+struct HasMemberNoExceptNoThrowMoveAssign {
   HasNoExceptNoThrowMoveAssign member; };
 
-struct HasDefaultTrivialCopyAssign { 
+struct HasDefaultTrivialCopyAssign {
   HasDefaultTrivialCopyAssign &operator=(
-    const HasDefaultTrivialCopyAssign&) = default; 
+    const HasDefaultTrivialCopyAssign&) = default;
 };
-struct TrivialMoveButNotCopy { 
+struct TrivialMoveButNotCopy {
   TrivialMoveButNotCopy &operator=(TrivialMoveButNotCopy&&) = default;
   TrivialMoveButNotCopy &operator=(const TrivialMoveButNotCopy&);
 };

 struct FinalClass final {
 };
 
-template<typename T> 
+template<typename T>
 struct PotentiallyFinal { };
 
 template<typename T>

   { int arr[T(__has_trivial_move_constructor(HasCons))]; }
   { int arr[T(__has_trivial_move_constructor(HasStaticMemberMoveCtor))]; }
   { int arr[T(__has_trivial_move_constructor(AllDeleted))]; }
-  
   { int arr[F(__has_trivial_move_constructor(HasVirt))]; }
   { int arr[F(__has_trivial_move_constructor(DerivesVirt))]; }
   { int arr[F(__has_trivial_move_constructor(HasMoveCtor))]; }

 };
 
 template<typename T>
-struct X0 { 
+struct X0 {
   template<typename U> X0(const X0<U>&);
 };
 

test/SemaCXX/user-defined-conversions.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify %s 
+// RUN: %clang_cc1 -fsyntax-only -verify %s
 struct X {
   operator bool();
 };

 // Test conversion followed by copy-construction
 struct FunkyDerived;
 
-struct Base { 
+struct Base {
   Base(const FunkyDerived&);
 };
 

 }
 
 struct X1 {
-  X1(X1&); // expected-note{{candidate constructor not viable: expects an l-value for 1st argument}}
+  X1(X1&); // expected-note{{candidate constructor not viable: expects an l-value for 1st argument}} \
+           // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
 };
 
 struct X2 {

test/SemaCXX/warn-consumed-analysis.cpp

 template <typename T>
 class CONSUMABLE(unconsumed) ConsumableClass {
   T var;
-  
 public:
   ConsumableClass();
   ConsumableClass(nullptr_t p) RETURN_TYPESTATE(consumed);
   ConsumableClass(T val) RETURN_TYPESTATE(unconsumed);
-  ConsumableClass(ConsumableClass<T> &other);
+  ConsumableClass(const ConsumableClass<T> &other);
   ConsumableClass(ConsumableClass<T> &&other);
-  
-  ConsumableClass<T>& operator=(ConsumableClass<T>  &other);
+  ConsumableClass<T>& operator=(const ConsumableClass<T>  &other);
   ConsumableClass<T>& operator=(ConsumableClass<T> &&other);
   ConsumableClass<T>& operator=(nullptr_t) SET_TYPESTATE(consumed);
-  
   template <typename U>
   ConsumableClass<T>& operator=(ConsumableClass<U>  &other);
-  
   template <typename U>
   ConsumableClass<T>& operator=(ConsumableClass<U> &&other);
-  
   void operator()(int a) SET_TYPESTATE(consumed);
   void operator*() const CALLABLE_WHEN("unconsumed");
   void unconsumedCall() const CALLABLE_WHEN("unconsumed");
   void callableWhenUnknown() const CALLABLE_WHEN("unconsumed", "unknown");
-  
   bool isValid() const TEST_TYPESTATE(unconsumed);
   operator bool() const TEST_TYPESTATE(unconsumed);
   bool operator!=(nullptr_t) const TEST_TYPESTATE(unconsumed);
   bool operator==(nullptr_t) const TEST_TYPESTATE(consumed);
-  
   void constCall() const;
   void nonconstCall();
-  
   void consume() SET_TYPESTATE(consumed);
   void unconsume() SET_TYPESTATE(unconsumed);
 };

 public:
   DestructorTester();
   DestructorTester(int);
-  
   void operator*() CALLABLE_WHEN("unconsumed");
-  
   ~DestructorTester() CALLABLE_WHEN("consumed");
 };
 

 
   var0 = ConsumableClass<int>(42);
   *var0;
-  
   var0 = var1;
   *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
-  
   if (var0.isValid()) {
     *var0;
     *var1;
-    
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
   }

 
 void testDestruction() {
   DestructorTester D0(42), D1(42), D2;
-  
   *D0;
   *D1;
   *D2; // expected-warning {{invalid invocation of method 'operator*' on object 'D2' while it is in the 'consumed' state}}
-  
   D0.~DestructorTester(); // expected-warning {{invalid invocation of method '~DestructorTester' on object 'D0' while it is in the 'unconsumed' state}}
-  
   return; // expected-warning {{invalid invocation of method '~DestructorTester' on object 'D0' while it is in the 'unconsumed' state}} \
              expected-warning {{invalid invocation of method '~DestructorTester' on object 'D1' while it is in the 'unconsumed' state}}
 }

 void testSimpleRValueRefs() {
   ConsumableClass<int> var0;
   ConsumableClass<int> var1(42);
-  
   *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
   *var1;
-  
   var0 = static_cast<ConsumableClass<int>&&>(var1);
-  
   *var0;
   *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
 }

 
 void testIfStmt() {
   ConsumableClass<int> var;
-  
   if (var.isValid()) {
     *var;
   } else {
     *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
   }
-  
   if (!var.isValid()) {
     *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
   } else {
     *var;
   }
-  
   if (var) {
     // Empty
   } else {
     *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
   }
-  
   if (var != nullptr) {
     // Empty
   } else {
     *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
   }
-  
   if (var == nullptr) {
     *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
   } else {

 
 void testComplexConditionals0() {
   ConsumableClass<int> var0, var1, var2;
-  
   if (var0 && var1) {
     *var0;
     *var1;
-    
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
   }
-  
   if (var0 || var1) {
     *var0;
     *var1;
-    
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
   }
-  
   if (var0 && !var1) {
     *var0;
     *var1;
-    
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
   }
-  
   if (var0 || !var1) {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
-    
   } else {
     *var0;
     *var1;
   }
-  
   if (!var0 && !var1) {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
-    
   } else {
     *var0;
     *var1;
   }
-  
   if (!var0 || !var1) {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
-    
   } else {
     *var0;
     *var1;
   }
-  
   if (!(var0 && var1)) {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
-    
   } else {
     *var0;
     *var1;
   }
-  
   if (!(var0 || var1)) {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
-    
   } else {
     *var0;
     *var1;
   }
-  
   if (var0 && var1 && var2) {
     *var0;
     *var1;
     *var2;
-    
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
     *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}
   }
-  
 #if 0
   // FIXME: Get this test to pass.
   if (var0 || var1 || var2) {

     *var0;
     *var1;
     *var2;
-    
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}

 
 void testComplexConditionals1() {
   ConsumableClass<int> var0, var1, var2;
-  
   // Coerce all variables into the unknown state.
   baf4(var0);
   baf4(var1);
   baf4(var2);
-  
   if (var0 && var1) {
     *var0;
     *var1;
-    
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
   }
-  
   if (var0 || var1) {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
-    
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
   }
-  
   if (var0 && !var1) {
     *var0;
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
-    
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
   }
-  
   if (var0 || !var1) {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
-    
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1;
   }
-  
   if (!var0 && !var1) {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
-    
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
   }
-  
   if (!(var0 || var1)) {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
-    
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
   }
-  
   if (!var0 || !var1) {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
-    
   } else {
     *var0;
     *var1;
   }
-  
   if (!(var0 && var1)) {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
-    
   } else {
     *var0;
     *var1;
   }
-  
   if (var0 && var1 && var2) {
     *var0;
     *var1;
     *var2;
-    
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
     *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'unknown' state}}
   }
-  
 #if 0
   // FIXME: Get this test to pass.
   if (var0 || var1 || var2) {

     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
     *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'unknown' state}}
-    
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}

 
 void testStateChangeInBranch() {
   ConsumableClass<int> var;
-  
   // Make var enter the 'unknown' state.
   baf4(var);
-  
   if (!var) {
     var = ConsumableClass<int>(42);
   }
-  
   *var;
 }
 
 void testFunctionParam(ConsumableClass<int> param) {
-  
   if (param.isValid()) {
     *param;
   } else {
     *param;
   }
-  
   param = nullptr;
   *param; // expected-warning {{invocation of method 'operator*' on object 'param' while it is in the 'consumed' state}}
 }
 
 void testParamReturnTypestateCallee(bool cond, ConsumableClass<int> &Param RETURN_TYPESTATE(unconsumed)) { // expected-warning {{parameter 'Param' not in expected state when the function returns: expected 'unconsumed', observed 'consumed'}}
-  
   if (cond) {
     Param.consume();
     return; // expected-warning {{parameter 'Param' not in expected state when the function returns: expected 'unconsumed', observed 'consumed'}}
   }
-  
   Param.consume();
 }
 
 void testParamReturnTypestateCaller() {
   ConsumableClass<int> var;
-  
   testParamReturnTypestateCallee(true, var);
-  
   *var;
 }
 
 void testParamTypestateCallee(ConsumableClass<int>  Param0 PARAM_TYPESTATE(consumed),
                               ConsumableClass<int> &Param1 PARAM_TYPESTATE(consumed)) {
-  
   *Param0; // expected-warning {{invalid invocation of method 'operator*' on object 'Param0' while it is in the 'consumed' state}}
   *Param1; // expected-warning {{invalid invocation of method 'operator*' on object 'Param1' while it is in the 'consumed' state}}
 }

 
 void testParamTypestateCaller() {
   ConsumableClass<int> Var0, Var1(42);
-  
   testParamTypestateCallee(Var0, Var1); // expected-warning {{argument not in expected state; expected 'consumed', observed 'unconsumed'}}
 }
 

 
 void testCallingConventions() {
   ConsumableClass<int> var(42);
-  
-  baf0(var);  
+  baf0(var);
   *var;
-  
-  baf1(var);  
+  baf1(var);
   *var;
-  
-  baf2(&var);  
+  baf2(&var);
   *var;
-  
-  baf4(var);  
+  baf4(var);
   *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}
-  
   var = ConsumableClass<int>(42);
-  baf5(&var);  
+  baf5(&var);
   *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}
-  
   var = ConsumableClass<int>(42);
-  baf6(static_cast<ConsumableClass<int>&&>(var));  
+  baf6(static_cast<ConsumableClass<int>&&>(var));
   *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
 }
 
 void testConstAndNonConstMemberFunctions() {
   ConsumableClass<int> var(42);
-  
   var.constCall();
   *var;
-  
   var.nonconstCall();
   *var;
 }

 
 void testReturnStates() {
   ConsumableClass<int> var;
-  
   var = returnsUnconsumed();
   *var;
-  
   var = returnsConsumed();
   *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
 }

 
 void testCallableWhen() {
   ConsumableClass<int> var(42);
-  
   *var;
-  
   baf4(var);
-  
   var.callableWhenUnknown();
 }
 

 void testMoveAsignmentish() {
   ConsumableClass<int>  var0;
   ConsumableClass<long> var1(42);
-  
   *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
   *var1;
-  
   var0 = static_cast<ConsumableClass<long>&&>(var1);
-  
   *var0;
   *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
-  
   var1 = ConsumableClass<long>(42);
   var1 = nullptr;
   *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}

 
 void testConditionalMerge() {
   ConsumableClass<int> var;
-  
   if (var.isValid()) {
     // Empty
   }
-  
   *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
-  
   if (var.isValid()) {
     // Empty
   } else {
     // Empty
   }
-  
   *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
 }
 
 void testSetTypestate() {
   ConsumableClass<int> var(42);
-  
   *var;
-  
   var.consume();
-  
   *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
-  
   var.unconsume();
-  
   *var;
 }
 
 void testConsumes0() {
   ConsumableClass<int> var(nullptr);
-  
   *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
 }
 
 void testConsumes1() {
   ConsumableClass<int> var(42);
-  
   var.unconsumedCall();
   var(6);
-  
   var.unconsumedCall(); // expected-warning {{invalid invocation of method 'unconsumedCall' on object 'var' while it is in the 'consumed' state}}
 }
 
 void testUnreachableBlock() {
   ConsumableClass<int> var(42);
-  
   if (var) {
     *var;
   } else {
     *var;
   }
-  
   *var;
 }
 

 
 void testForLoop1() {
   ConsumableClass<int> var0, var1(42);
-  
   for (int i = 0; i < 10; ++i) { // expected-warning {{state of variable 'var1' must match at the entry and exit of loop}}
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
-    
     *var1;
     var1.consume();
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
   }
-  
   *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
 }
 
 void testWhileLoop1() {
   int i = 10;
-  
   ConsumableClass<int> var0, var1(42);
-  
   while (i-- > 0) { // expected-warning {{state of variable 'var1' must match at the entry and exit of loop}}
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
-    
     *var1;
     var1.consume();
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
   }
-  
   *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
 }
 

 
 void testTemporariesWithConditionals1() {
   int a;
-  
   Status s = doSomething();
   if (cond()) a = 0;
   else        a = 1;

 
 void testTemporariesWithConditionals2() {
   int a;
-  
   Status s = doSomething();
   s.ignore();
   if (cond()) a = 0;

test/SemaCXX/warn-non-idiomatic-copy-move.cpp

+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -verify %s
+
+struct A1 {
+  A1(A1&); // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
+  A1& operator=(A1&); // expected-warning {{non-idiomatic copy assignment operator declaration; consider 'const T&' instead}}
+};
+
+struct A2 {
+  A2(volatile A2&); // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
+  A2& operator=(volatile A2&); // expected-warning {{non-idiomatic copy assignment operator declaration; consider 'const T&' instead}}
+};
+
+struct A3 {
+  A3(const volatile A3&); // expected-warning {{non-idiomatic copy constructor declaration; consider 'const T&' instead}}
+  A3& operator=(const volatile A3&); // expected-warning {{non-idiomatic copy assignment operator declaration; consider 'const T&' instead}}
+};
+
+struct A4 {
+  A4(const A4&); // ok
+  A4& operator=(const A4&); // ok
+};
+
+struct A5 {
+  A5(const A5&&); // expected-warning {{non-idiomatic move constructor declaration; consider 'T&&' instead}}
+  A5& operator=(const A5&&); // expected-warning {{non-idiomatic move assignment operator declaration; consider 'T&&' instead}}
+};
+
+struct A6 {
+  A6(volatile A6&&); // expected-warning {{non-idiomatic move constructor declaration; consider 'T&&' instead}}
+  A6& operator=(volatile A6&&); // expected-warning {{non-idiomatic move assignment operator declaration; consider 'T&&' instead}}
+};
+
+struct A7 {
+  A7(const volatile A7&&); // expected-warning {{non-idiomatic move constructor declaration; consider 'T&&' instead}}
+  A7& operator=(const volatile A7&&); // expected-warning {{non-idiomatic move assignment operator declaration; consider 'T&&' instead}}
+};
+
+struct A8 {
+  A8(A8&&); // ok
+  A8& operator=(A8&&); // ok
+};
+
+struct A9 {
+  A9& operator=(A9); // expected-warning {{non-idiomatic copy assignment operator declaration; consider 'const T&' instead}}
+};
+
+struct B {
+  B&& operator=(const B&); // expected-warning {{non-idiomatic copy assignment operator declaration; consider returning 'T&' instead}}
+  B&& operator=(B&&); // expected-warning {{non-idiomatic move assignment operator declaration; consider returning 'T&' instead}}
+};
+
+struct C {
+  C operator=(const C&); // expected-warning {{non-idiomatic copy assignment operator declaration; consider returning 'T&' instead}}
+  C operator=(C&&); // expected-warning {{non-idiomatic move assignment operator declaration; consider returning 'T&' instead}}
+};
+
+struct D {
+  C& operator=(const D&); // expected-warning {{non-idiomatic copy assignment operator declaration; consider returning 'T&' instead}}
+  C& operator=(D&&); // expected-warning {{non-idiomatic move assignment operator declaration; consider returning 'T&' instead}}
+};
+
+struct E {
+  E& operator=(const E&) &; // expected-warning {{non-idiomatic copy assignment operator declaration; consider removing ref-qualifiers instead}}
+  E& operator=(E&&) &; // expected-warning {{non-idiomatic move assignment operator declaration; consider removing ref-qualifiers instead}}
+};
+
+struct F {
+  F& operator=(F&) = default; // expected-warning {{non-idiomatic copy assignment operator declaration; consider 'const T&' instead}}
+  F& operator=(const F&&) = delete; // expected-warning {{non-idiomatic move assignment operator declaration; consider 'T&&' instead}}
+};
+
+struct G {
+  // Do not diagnose if the declaration is invalid anyway.
+  auto operator=(const G&) = default; // expected-error {{must return 'G &'}}
+};
+
+struct H {
+  // Do not diagnose non-idiomatic copy/move operations in the presence of
+  // idiomatic ones.
+  H(H&); // ok
+  H(const H&); // ok
+
+  H(const H&&); // ok
+  H(H&&); // ok
+
+  H& operator=(H&) &; // ok
+  H& operator=(const H&); // ok
+
+  H& operator=(const volatile H&&) &&; // ok
+  H& operator=(H&&); // ok
+};
+
+struct I {
+  // Do not diagnose things that are not copy or move operations, but are
+  // instead converting operations.
+  I(int); // ok
+  I(int&); // ok
+  I(int&&); // ok
+  I(H&); // ok
+
+  I& operator=(int); // ok
+  I& operator=(int&); // ok
+  I& operator=(int&&); // ok
+  I& operator=(H&); // ok
+  I& operator=(H&&); // ok
+};
+
+struct K {
+  // Do not diagnose non-idiomatic operations in the presence of other non-
+  // idiomatic operations of the same type.
+  K(K&); // ok
+  K(volatile K&); // ok
+
+  K(const K&&); // ok
+  K(volatile K&&); // ok
+
+  K& operator=(const K&) &; // ok
+  K& operator=(K&); // ok
+
+  K operator=(K&&); // ok
+  K& operator=(const K&&); // ok
+};
+
+struct L {
+  // Do not diagnose as non-idiomatic if the constructor uses default arguments;
+  // TODO: that should be diagnosed differently.
+  L(L&, int = 0); // ok
+  L(const L&&, int = 0); // ok
+};

test/SemaCXX/warn-unused-filescoped.cpp

     void m2();  // expected-warning{{unused}}
     void m3();
     S(const S&);
-    void operator=(const S&);
+    S& operator=(const S&);
   };
 
   template <typename T>

   template <typename T>
   void tf() { }
   template <> void tf<int>() { }  // expected-warning{{unused}}
-  
   struct VS {
     virtual void vm() { }
   };
-  
   struct SVS : public VS {
     void vm() { }
   };

 
 namespace {
   int x2;  // expected-warning{{unused}}
-  
   struct S2 {
     static int x;  // expected-warning{{unused}}
   };

test/SemaObjCXX/property-synthesis-error.mm

-// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class -Wno-non-idiomatic-copy-move %s
 // rdar: //8550657
 
 @interface NSArray @end

test/SemaTemplate/constructor-template.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-non-idiomatic-copy-move -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-non-idiomatic-copy-move -verify -std=c++98 %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-non-idiomatic-copy-move -verify -std=c++11 %s
 
 struct X0 { // expected-note {{candidate constructor (the implicit copy constructor) not viable}}
 #if __cplusplus >= 201103L // C++11 or later

   X0(int); // expected-note{{candidate}}
   template<typename T> X0(T); // expected-note {{candidate}}
   template<typename T, typename U> X0(T*, U*); // expected-note {{candidate}}
-  
   // PR4761
   template<typename T> X0() : f0(T::foo) {} // expected-note {{candidate}}
   int f0;

   accept_X0(&f);
   X0 x0e(&i, &f);
   X0 x0f(&f, &i);
-  
   X0 x0g(f, &i); // expected-error{{no matching constructor}}
 }
 

 template<typename T>
 struct Outer {
   typedef X1<T> A;
-  
   A alloc;
-  
   explicit Outer(const A& a) : alloc(a) { }
 };
 

 X2 test(bool Cond, X2 x2) {
   if (Cond)
     return x2; // okay, uses copy constructor
-  
   return X2(); // expected-error{{no matching constructor}}
 }
 

test/SemaTemplate/instantiate-decl-init.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-non-idiomatic-copy-move -verify %s
 // expected-no-diagnostics
 
 // PR5426 - the non-dependent obj would be fully processed and wrapped in a