PR7245: Make binding a reference to a temporary without a usable copy

constructor into an extension warning into the error that C++98 requires.

llvm-svn: 105529

clang/docs/UsersManual.html

@@ -410,6 +410,42 @@ because it's hard to work around, Clang downgrades it to a warning as
 an extension.</p>
 </dd>
 
+<!-- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -->
+<dt id="opt_Wbind-to-temporary-copy"><b>-Wbind-to-temporary-copy</b>: Warn about
+an unusable copy constructor when binding a reference to a temporary.</dt>
+<dd>This option, which defaults to on, enables warnings about binding a
+reference to a temporary when the temporary doesn't have a usable copy
+constructor.  For example:</p>
+
+<pre>
+  struct NonCopyable {
+    NonCopyable();
+  private:
+    NonCopyable(const NonCopyable&);
+  };
+  void foo(const NonCopyable&);
+  void bar() {
+    foo(NonCopyable());  // Disallowed in C++98; allowed in C++0x.
+  }
+</pre>
+<pre>
+  struct NonCopyable2 {
+    NonCopyable2();
+    NonCopyable2(NonCopyable2&);
+  };
+  void foo(const NonCopyable2&);
+  void bar() {
+    foo(NonCopyable2());  // Disallowed in C++98; allowed in C++0x.
+  }
+</pre>
+
+<p>Note that if <tt>NonCopyable2::NonCopyable2()</tt> has a default
+argument whose instantiation produces a compile error, that error will
+still be a hard error in C++98 mode even if this warning is turned
+off.</p>
+
+</dd>
+
 </dl>
 
 <!-- ======================================================================= -->

clang/include/clang/Basic/DiagnosticGroups.td

@@ -76,6 +76,7 @@ def PoundWarning : DiagGroup<"#warnings">,
 def : DiagGroup<"pointer-to-int-cast">;
 def : DiagGroup<"redundant-decls">;
 def ReturnType : DiagGroup<"return-type">;
+def BindToTemporaryCopy : DiagGroup<"bind-to-temporary-copy">;
 def SemiBeforeMethodBody : DiagGroup<"semicolon-before-method-body">;
 def : DiagGroup<"sequence-point">;
 def Shadow : DiagGroup<"shadow">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -486,6 +486,10 @@ def err_access : Error<
   "%1 is a %select{private|protected}0 member of %3">, NoSFINAE;
 def err_access_ctor : Error<
   "calling a %select{private|protected}0 constructor of class %2">, NoSFINAE;
+def ext_rvalue_to_reference_access_ctor : ExtWarn<
+  "C++98 requires an accessible copy constructor for class %2 when binding "
+  "a reference to a temporary; was %select{private|protected}0">,
+  NoSFINAE, InGroup<BindToTemporaryCopy>;
 def err_access_base : Error<
   "%select{base class|inherited virtual base class}0 %1 has %select{private|"
   "protected}3 %select{constructor|copy constructor|copy assignment operator|"
@@ -747,6 +751,13 @@ def err_temp_copy_no_viable : Error<
   "returning object|throwing object|copying member subobject|copying array "
   "element|allocating object|copying temporary|initializing base subobject|"
   "initializing vector element}0 of type %1">;
+def ext_rvalue_to_reference_temp_copy_no_viable : ExtWarn<
+  "no viable constructor %select{copying variable|copying parameter|"
+  "returning object|throwing object|copying member subobject|copying array "
+  "element|allocating object|copying temporary|initializing base subobject|"
+  "initializing vector element}0 of type %1; C++98 requires a copy "
+  "constructor when binding a reference to a temporary">,
+  InGroup<BindToTemporaryCopy>;
 def err_temp_copy_ambiguous : Error<
   "ambiguous constructor call when %select{copying variable|copying "
   "parameter|returning object|throwing object|copying member subobject|copying "

clang/lib/Sema/Sema.h

@@ -2767,7 +2767,8 @@ public:
   AccessResult CheckConstructorAccess(SourceLocation Loc,
                                       CXXConstructorDecl *D,
                                       const InitializedEntity &Entity,
-                                      AccessSpecifier Access);
+                                      AccessSpecifier Access,
+                                      bool IsCopyBindingRefToTemp = false);
   AccessResult CheckDestructorAccess(SourceLocation Loc,
                                      CXXDestructorDecl *Dtor,
                                      const PartialDiagnostic &PDiag);

clang/lib/Sema/SemaAccess.cpp

@@ -1157,9 +1157,10 @@ Sema::AccessResult Sema::CheckDestructorAccess(SourceLocation Loc,
 
 /// Checks access to a constructor.
 Sema::AccessResult Sema::CheckConstructorAccess(SourceLocation UseLoc,
-                                  CXXConstructorDecl *Constructor,
-                                  const InitializedEntity &Entity,
-                                  AccessSpecifier Access) {
+                                                CXXConstructorDecl *Constructor,
+                                                const InitializedEntity &Entity,
+                                                AccessSpecifier Access,
+                                                bool IsCopyBindingRefToTemp) {
   if (!getLangOptions().AccessControl ||
       Access == AS_public)
     return AR_accessible;
@@ -1170,7 +1171,9 @@ Sema::AccessResult Sema::CheckConstructorAccess(SourceLocation UseLoc,
                             QualType());
   switch (Entity.getKind()) {
   default:
-    AccessEntity.setDiag(diag::err_access_ctor);
+    AccessEntity.setDiag(IsCopyBindingRefToTemp
+                         ? diag::ext_rvalue_to_reference_access_ctor
+                         : diag::err_access_ctor);
     break;
 
   case InitializedEntity::EK_Base:

clang/lib/Sema/SemaInit.cpp

@@ -3326,12 +3326,16 @@ static Sema::OwningExprResult CopyObject(Sema &S,
     break;
       
   case OR_No_Viable_Function:
-    S.Diag(Loc, diag::err_temp_copy_no_viable)
+    S.Diag(Loc, IsExtraneousCopy && !S.isSFINAEContext()
+           ? diag::ext_rvalue_to_reference_temp_copy_no_viable
+           : diag::err_temp_copy_no_viable)
       << (int)Entity.getKind() << CurInitExpr->getType()
       << CurInitExpr->getSourceRange();
     S.PrintOverloadCandidates(CandidateSet, Sema::OCD_AllCandidates,
                               &CurInitExpr, 1);
-    return S.ExprError();
+    if (!IsExtraneousCopy || S.isSFINAEContext())
+      return S.ExprError();
+    return move(CurInit);
       
   case OR_Ambiguous:
     S.Diag(Loc, diag::err_temp_copy_ambiguous)
@@ -3355,7 +3359,7 @@ static Sema::OwningExprResult CopyObject(Sema &S,
   CurInit.release(); // Ownership transferred into MultiExprArg, below.
 
   S.CheckConstructorAccess(Loc, Constructor, Entity,
-                           Best->FoundDecl.getAccess());
+                           Best->FoundDecl.getAccess(), IsExtraneousCopy);
 
   if (IsExtraneousCopy) {
     // If this is a totally extraneous copy for C++03 reference

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

@@ -1,9 +1,10 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-show-option -verify %s
 
 // C++03 requires that we check for a copy constructor when binding a
 // reference to a temporary, since we are allowed to make a copy, Even
 // though we don't actually make that copy, make sure that we diagnose
-// cases where that copy constructor is somehow unavailable.
+// cases where that copy constructor is somehow unavailable.  As an
+// extension, this is only a warning.
 
 struct X1 {
   X1();
@@ -28,6 +29,7 @@ private:
 template<typename T>
 T get_value_badly() {
   double *dp = 0;
+  // The extension doesn't extend far enough to turn this error into a warning.
   T *tp = dp; // expected-error{{ cannot initialize a variable of type 'int *' with an lvalue of type 'double *'}}
   return T();
 }
@@ -41,7 +43,7 @@ struct X4 {
 // Check for "dangerous" default arguments that could cause recursion.
 struct X5 {
   X5();
-  X5(const X5&, const X5& = X5()); // expected-error{{no viable constructor copying parameter of type 'X5'}}
+  X5(const X5&, const X5& = X5()); // expected-warning{{no viable constructor copying parameter of type 'X5'}}
 };
 
 void g1(const X1&);
@@ -51,11 +53,25 @@ void g4(const X4<int>&);
 void g5(const X5&);
 
 void test() {
-  g1(X1()); // expected-error{{no viable constructor copying parameter of type 'X1'}}
-  g2(X2()); // expected-error{{calling a private constructor of class 'X2'}}
-  g3(X3()); // expected-error{{no viable constructor copying parameter of type 'X3'}}
+  g1(X1()); // expected-warning{{no viable constructor copying parameter of type 'X1'; C++98 requires a copy constructor when binding a reference to a temporary [-Wbind-to-temporary-copy]}}
+  g2(X2()); // expected-warning{{C++98 requires an accessible copy constructor for class 'X2' when binding a reference to a temporary; was private [-Wbind-to-temporary-copy]}}
+  g3(X3()); // expected-warning{{no viable constructor copying parameter of type 'X3'}}
   g4(X4<int>());
-  g5(X5()); // expected-error{{no viable constructor copying parameter of type 'X5'}}
+  g5(X5());  // Generates a warning in the default argument.
 }
 
-// Check for dangerous recursion in default arguments.
+// Check that unavailable copy constructors still cause SFINAE failures.
+template<int> struct int_c { };
+
+template<typename T> T f(const T&);
+
+// Would be ambiguous with the next g(), except the instantiation failure in
+// sizeof() prevents that.
+template<typename T>
+int &g(int_c<sizeof(f(T()))> * = 0);
+
+template<typename T> float &g();
+
+void h() {
+  float &fp2 = g<X3>();  // Not ambiguous.
+}

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

@@ -48,3 +48,17 @@ void test() {
   g3(X3());
   g4(X4<int>());
 }
+
+// Check that unavailable copy constructors do not cause SFINAE failures.
+template<int> struct int_c { };
+
+template<typename T> T f(const T&);
+
+template<typename T>
+int &g(int_c<sizeof(f(T()))> * = 0);  // expected-note{{candidate function [with T = X3]}}
+
+template<typename T> float &g();  // expected-note{{candidate function [with T = X3]}}
+
+void h() {
+  float &fp = g<X3>();  // expected-error{{call to 'g' is ambiguous}}
+}