From: Jeffrey Yasskin Date: Mon, 7 Jun 2010 15:58:05 +0000 (+0000) Subject: PR7245: Make binding a reference to a temporary without a usable copy X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=57d12fd4a2bc739c4a4d62a364b7f08cd483c59e;p=clang PR7245: Make binding a reference to a temporary without a usable copy constructor into an extension warning into the error that C++98 requires. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@105529 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/docs/UsersManual.html b/docs/UsersManual.html index a53a1af5ff..c19c96b4bc 100644 --- a/docs/UsersManual.html +++ b/docs/UsersManual.html @@ -410,6 +410,42 @@ because it's hard to work around, Clang downgrades it to a warning as an extension.

+ +
-Wbind-to-temporary-copy: Warn about +an unusable copy constructor when binding a reference to a temporary.
+
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:

+ +
+  struct NonCopyable {
+    NonCopyable();
+  private:
+    NonCopyable(const NonCopyable&);
+  };
+  void foo(const NonCopyable&);
+  void bar() {
+    foo(NonCopyable());  // Disallowed in C++98; allowed in C++0x.
+  }
+
+
+  struct NonCopyable2 {
+    NonCopyable2();
+    NonCopyable2(NonCopyable2&);
+  };
+  void foo(const NonCopyable2&);
+  void bar() {
+    foo(NonCopyable2());  // Disallowed in C++98; allowed in C++0x.
+  }
+
+ +

Note that if NonCopyable2::NonCopyable2() 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.

+ +
+ diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index e507733dfa..430ad96efb 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/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">; diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index ce28fd6d0d..2412267bb6 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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; 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; def err_temp_copy_ambiguous : Error< "ambiguous constructor call when %select{copying variable|copying " "parameter|returning object|throwing object|copying member subobject|copying " diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index 10ff5ad8b9..fed5e7895c 100644 --- a/lib/Sema/Sema.h +++ b/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); diff --git a/lib/Sema/SemaAccess.cpp b/lib/Sema/SemaAccess.cpp index 54d06f531c..7845f6d0a9 100644 --- a/lib/Sema/SemaAccess.cpp +++ b/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: diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp index ef99d1bd69..e34e1683e8 100644 --- a/lib/Sema/SemaInit.cpp +++ b/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 diff --git a/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp b/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp index 16394110d0..ae59598f69 100644 --- a/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp +++ b/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 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&); 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()); - 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 struct int_c { }; + +template T f(const T&); + +// Would be ambiguous with the next g(), except the instantiation failure in +// sizeof() prevents that. +template +int &g(int_c * = 0); + +template float &g(); + +void h() { + float &fp2 = g(); // Not ambiguous. +} diff --git a/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx0x-no-extra-copy.cpp b/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx0x-no-extra-copy.cpp index 5a342d423a..5cfb11a1fc 100644 --- a/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx0x-no-extra-copy.cpp +++ b/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()); } + +// Check that unavailable copy constructors do not cause SFINAE failures. +template struct int_c { }; + +template T f(const T&); + +template +int &g(int_c * = 0); // expected-note{{candidate function [with T = X3]}} + +template float &g(); // expected-note{{candidate function [with T = X3]}} + +void h() { + float &fp = g(); // expected-error{{call to 'g' is ambiguous}} +}