From 9b2c5e7c44fd28c31f4a0b088d0bf71da4fddfa5 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Fri, 28 Sep 2018 01:16:43 +0000 Subject: [PATCH] [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only render the function deleted instead of rendering the program ill-formed. This change also adds an enabled-by-default warning for the case where an explicitly-defaulted special member function of a non-template class is implicitly deleted by the type checking rules. (This fires either due to this language change or due to pre-C++20 reasons for the member being implicitly deleted). I've tested this on a large codebase and found only bugs (where the program means something that's clearly different from what the programmer intended), so this is enabled by default, but we should revisit this if there are problems with this being enabled by default. llvm-svn: 343285 --- .../clang/Basic/DiagnosticSemaKinds.td | 10 ++++ clang/lib/Sema/SemaDeclCXX.cpp | 59 +++++++++++++++---- .../CXX/class.derived/class.abstract/p16.cpp | 4 +- .../dcl.fct.def/dcl.fct.def.default/p1.cpp | 51 +++++++++++++++- clang/test/CXX/drs/dr6xx.cpp | 4 +- clang/test/CXX/special/class.copy/p12-0x.cpp | 2 +- .../test/CXX/special/class.copy/p23-cxx11.cpp | 2 +- clang/test/CXX/special/class.ctor/p5-0x.cpp | 2 +- clang/test/CXX/special/class.dtor/p5-0x.cpp | 2 +- clang/test/SemaCUDA/implicit-member-target.cu | 2 +- .../SemaCXX/cxx0x-deleted-default-ctor.cpp | 6 +- clang/test/SemaCXX/cxx17-compat.cpp | 14 ++++- clang/test/SemaCXX/dr1301.cpp | 6 +- .../SemaCXX/microsoft-dtor-lookup-cxx11.cpp | 4 +- .../SemaTemplate/exception-spec-crash.cpp | 4 +- clang/www/cxx_status.html | 2 +- 16 files changed, 138 insertions(+), 36 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 508a0c41af7cc4..7c2f3d1833de7f 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7760,9 +7760,19 @@ def err_incorrect_defaulted_exception_spec : Error< def err_incorrect_defaulted_constexpr : Error< "defaulted definition of %sub{select_special_member_kind}0 " "is not constexpr">; +def warn_defaulted_method_deleted : Warning< + "explicitly defaulted %sub{select_special_member_kind}0 is implicitly " + "deleted">, InGroup>; def err_out_of_line_default_deletes : Error< "defaulting this %sub{select_special_member_kind}0 " "would delete it after its first declaration">; +def note_deleted_type_mismatch : Note< + "function is implicitly deleted because its declared type does not match " + "the type of an implicit %sub{select_special_member_kind}0">; +def warn_cxx17_compat_defaulted_method_type_mismatch : Warning< + "explicitly defaulting this %sub{select_special_member_kind}0 with a type " + "different from the implicit type is incompatible with C++ standards before " + "C++2a">, InGroup, DefaultIgnore; def warn_vbase_moved_multiple_times : Warning< "defaulted move assignment operator of %0 will move assign virtual base " "class %1 multiple times">, InGroup>; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index a18b8dd59676f8..3261a7031d5853 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -6451,20 +6451,29 @@ void Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD) { // copy operation can take a non-const reference) as an implicit // declaration, and // -- not have default arguments. + // C++2a changes the second bullet to instead delete the function if it's + // defaulted on its first declaration, unless it's "an assignment operator, + // and its return type differs or its parameter type is not a reference". + bool DeleteOnTypeMismatch = getLangOpts().CPlusPlus2a && First; + bool ShouldDeleteForTypeMismatch = false; unsigned ExpectedParams = 1; if (CSM == CXXDefaultConstructor || CSM == CXXDestructor) ExpectedParams = 0; if (MD->getNumParams() != ExpectedParams) { - // This also checks for default arguments: a copy or move constructor with a + // This checks for default arguments: a copy or move constructor with a // default argument is classified as a default constructor, and assignment // operations and destructors can't have default arguments. Diag(MD->getLocation(), diag::err_defaulted_special_member_params) << CSM << MD->getSourceRange(); HadError = true; } else if (MD->isVariadic()) { - Diag(MD->getLocation(), diag::err_defaulted_special_member_variadic) - << CSM << MD->getSourceRange(); - HadError = true; + if (DeleteOnTypeMismatch) + ShouldDeleteForTypeMismatch = true; + else { + Diag(MD->getLocation(), diag::err_defaulted_special_member_variadic) + << CSM << MD->getSourceRange(); + HadError = true; + } } const FunctionProtoType *Type = MD->getType()->getAs(); @@ -6489,9 +6498,13 @@ void Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD) { // A defaulted special member cannot have cv-qualifiers. if (Type->getTypeQuals()) { - Diag(MD->getLocation(), diag::err_defaulted_special_member_quals) - << (CSM == CXXMoveAssignment) << getLangOpts().CPlusPlus14; - HadError = true; + if (DeleteOnTypeMismatch) + ShouldDeleteForTypeMismatch = true; + else { + Diag(MD->getLocation(), diag::err_defaulted_special_member_quals) + << (CSM == CXXMoveAssignment) << getLangOpts().CPlusPlus14; + HadError = true; + } } } @@ -6504,23 +6517,30 @@ void Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD) { HasConstParam = ReferentType.isConstQualified(); if (ReferentType.isVolatileQualified()) { - Diag(MD->getLocation(), - diag::err_defaulted_special_member_volatile_param) << CSM; - HadError = true; + if (DeleteOnTypeMismatch) + ShouldDeleteForTypeMismatch = true; + else { + Diag(MD->getLocation(), + diag::err_defaulted_special_member_volatile_param) << CSM; + HadError = true; + } } if (HasConstParam && !CanHaveConstParam) { - if (CSM == CXXCopyConstructor || CSM == CXXCopyAssignment) { + if (DeleteOnTypeMismatch) + ShouldDeleteForTypeMismatch = true; + else if (CSM == CXXCopyConstructor || CSM == CXXCopyAssignment) { Diag(MD->getLocation(), diag::err_defaulted_special_member_copy_const_param) << (CSM == CXXCopyAssignment); // FIXME: Explain why this special member can't be const. + HadError = true; } else { Diag(MD->getLocation(), diag::err_defaulted_special_member_move_const_param) << (CSM == CXXMoveAssignment); + HadError = true; } - HadError = true; } } else if (ExpectedParams) { // A copy assignment operator can take its argument by value, but a @@ -6582,14 +6602,27 @@ void Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD) { EPI)); } - if (ShouldDeleteSpecialMember(MD, CSM)) { + if (ShouldDeleteForTypeMismatch || ShouldDeleteSpecialMember(MD, CSM)) { if (First) { SetDeclDeleted(MD, MD->getLocation()); + if (!inTemplateInstantiation() && !HadError) { + Diag(MD->getLocation(), diag::warn_defaulted_method_deleted) << CSM; + if (ShouldDeleteForTypeMismatch) { + Diag(MD->getLocation(), diag::note_deleted_type_mismatch) << CSM; + } else { + ShouldDeleteSpecialMember(MD, CSM, nullptr, /*Diagnose*/true); + } + } + if (ShouldDeleteForTypeMismatch && !HadError) { + Diag(MD->getLocation(), + diag::warn_cxx17_compat_defaulted_method_type_mismatch) << CSM; + } } else { // C++11 [dcl.fct.def.default]p4: // [For a] user-provided explicitly-defaulted function [...] if such a // function is implicitly defined as deleted, the program is ill-formed. Diag(MD->getLocation(), diag::err_out_of_line_default_deletes) << CSM; + assert(!ShouldDeleteForTypeMismatch && "deleted non-first decl"); ShouldDeleteSpecialMember(MD, CSM, nullptr, /*Diagnose*/true); HadError = true; } diff --git a/clang/test/CXX/class.derived/class.abstract/p16.cpp b/clang/test/CXX/class.derived/class.abstract/p16.cpp index 80396a96d9ec41..fe31b7321bd0f2 100644 --- a/clang/test/CXX/class.derived/class.abstract/p16.cpp +++ b/clang/test/CXX/class.derived/class.abstract/p16.cpp @@ -44,8 +44,8 @@ struct G : D {}; // expected-error@-3 {{deleted function 'operator=' cannot override a non-deleted function}} // expected-note@-4 {{while declaring the implicit move assignment operator for 'G'}} // expected-note@-5 {{move assignment operator of 'G' is implicitly deleted because base class 'D' has an inaccessible move assignment operator}} -struct H : D { - H &operator=(H&&) = default; +struct H : D { // expected-note {{deleted because base class 'D' has an inaccessible move assignment}} + H &operator=(H&&) = default; // expected-warning {{implicitly deleted}} // expected-error@-1 {{deleted function 'operator=' cannot override a non-deleted function}} // expected-note@-3 {{move assignment operator of 'H' is implicitly deleted because base class 'D' has an inaccessible move assignment operator}} ~H(); diff --git a/clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p1.cpp b/clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p1.cpp index 51993307cfff4d..3f2bc569edf62f 100644 --- a/clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p1.cpp +++ b/clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p1.cpp @@ -1,15 +1,30 @@ // RUN: %clang_cc1 -verify %s -std=c++11 +// RUN: %clang_cc1 -verify %s -std=c++17 +// RUN: %clang_cc1 -verify %s -std=c++2a // A function that is explicitly defaulted shall struct A { // -- be a special member function, A(int) = default; // expected-error {{only special member functions may be defaulted}} + A(A) = default; // expected-error {{must pass its first argument by reference}} // -- have the same declared function type as if it had been implicitly // declared void operator=(const A &) = default; // expected-error {{must return 'A &'}} - A(...) = default; // expected-error {{cannot be variadic}} - A(const A &, ...) = default; // expected-error {{cannot be variadic}} + A(...) = default; + A(const A &, ...) = default; + A &operator=(const A&) const = default; + A &operator=(A) const = default; // expected-error {{must be an lvalue refe}} +#if __cplusplus <= 201703L + // expected-error@-5 {{cannot be variadic}} + // expected-error@-5 {{cannot be variadic}} + // expected-error@-5 {{may not have 'const'}} + // expected-error@-5 {{may not have 'const'}} +#else + // expected-warning@-10 {{implicitly deleted}} expected-note@-10 {{declared type does not match the type of an implicit default constructor}} + // expected-warning@-10 {{implicitly deleted}} expected-note@-10 {{declared type does not match the type of an implicit copy constructor}} + // expected-warning@-10 {{implicitly deleted}} expected-note@-10 {{declared type does not match the type of an implicit copy assignment}} +#endif // (except for possibly differing ref-qualifiers A &operator=(A &&) & = default; @@ -23,3 +38,35 @@ struct A { A(double = 0.0) = default; // expected-error {{cannot have default arguments}} A(const A & = 0) = default; // expected-error {{cannot have default arguments}} }; + +struct A2 { + A2(...); + A2(const A2 &, ...); + A2 &operator=(const A2&) const; +}; +A2::A2(...) = default; // expected-error {{cannot be variadic}} +A2::A2(const A2&, ...) = default; // expected-error {{cannot be variadic}} +A2 &A2::operator=(const A2&) const = default; // expected-error {{may not have 'const'}} + +struct B { + B(B&); + B &operator=(B&); +}; +struct C : B { + C(const C&) = default; + C &operator=(const C&) = default; +#if __cplusplus <= 201703L + // expected-error@-3 {{is const, but a member or base requires it to be non-const}} + // expected-error@-3 {{is const, but a member or base requires it to be non-const}} +#else + // expected-warning@-6 {{implicitly deleted}} expected-note@-6 {{type does not match}} + // expected-warning@-6 {{implicitly deleted}} expected-note@-6 {{type does not match}} +#endif +}; + +struct D : B { // expected-note 2{{base class}} + D(const D&); + D &operator=(const D&); +}; +D::D(const D&) = default; // expected-error {{would delete}} expected-error {{is const, but}} +D &D::operator=(const D&) = default; // expected-error {{would delete}} expected-error {{is const, but}} diff --git a/clang/test/CXX/drs/dr6xx.cpp b/clang/test/CXX/drs/dr6xx.cpp index c3c867b7a54d2f..0f072268ab57b4 100644 --- a/clang/test/CXX/drs/dr6xx.cpp +++ b/clang/test/CXX/drs/dr6xx.cpp @@ -757,8 +757,8 @@ namespace dr666 { // dr666: yes #if __cplusplus >= 201103L namespace dr667 { // dr667: yes struct A { - A() = default; - int &r; + A() = default; // expected-warning {{explicitly defaulted default constructor is implicitly deleted}} + int &r; // expected-note {{because field 'r' of reference type 'int &' would not be initialized}} }; static_assert(!__is_trivially_constructible(A), ""); diff --git a/clang/test/CXX/special/class.copy/p12-0x.cpp b/clang/test/CXX/special/class.copy/p12-0x.cpp index 1b23b5a4b179ce..a0ef49d9b64db6 100644 --- a/clang/test/CXX/special/class.copy/p12-0x.cpp +++ b/clang/test/CXX/special/class.copy/p12-0x.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -std=c++11 -verify %s +// RUN: %clang_cc1 -std=c++11 -verify %s -Wno-defaulted-function-deleted // expected-no-diagnostics diff --git a/clang/test/CXX/special/class.copy/p23-cxx11.cpp b/clang/test/CXX/special/class.copy/p23-cxx11.cpp index ac21cc61a2c556..0b9652b50fcfb0 100644 --- a/clang/test/CXX/special/class.copy/p23-cxx11.cpp +++ b/clang/test/CXX/special/class.copy/p23-cxx11.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -verify %s -std=c++11 +// RUN: %clang_cc1 -verify %s -std=c++11 -Wno-defaulted-function-deleted struct Trivial {}; diff --git a/clang/test/CXX/special/class.ctor/p5-0x.cpp b/clang/test/CXX/special/class.ctor/p5-0x.cpp index 061a3d1f0738a2..5fa61008e8b774 100644 --- a/clang/test/CXX/special/class.ctor/p5-0x.cpp +++ b/clang/test/CXX/special/class.ctor/p5-0x.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 +// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -Wno-defaulted-function-deleted struct DefaultedDefCtor1 {}; struct DefaultedDefCtor2 { DefaultedDefCtor2() = default; }; diff --git a/clang/test/CXX/special/class.dtor/p5-0x.cpp b/clang/test/CXX/special/class.dtor/p5-0x.cpp index 595784f0d5f9d3..f69baca53bf339 100644 --- a/clang/test/CXX/special/class.dtor/p5-0x.cpp +++ b/clang/test/CXX/special/class.dtor/p5-0x.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -verify -std=c++11 %s +// RUN: %clang_cc1 -verify -std=c++11 %s -Wno-defaulted-function-deleted struct NonTrivDtor { ~NonTrivDtor(); diff --git a/clang/test/SemaCUDA/implicit-member-target.cu b/clang/test/SemaCUDA/implicit-member-target.cu index 242d345fb93ec4..d87e6962404341 100644 --- a/clang/test/SemaCUDA/implicit-member-target.cu +++ b/clang/test/SemaCUDA/implicit-member-target.cu @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -std=gnu++11 -fsyntax-only -verify %s +// RUN: %clang_cc1 -std=gnu++11 -fsyntax-only -verify %s -Wno-defaulted-function-deleted #include "Inputs/cuda.h" diff --git a/clang/test/SemaCXX/cxx0x-deleted-default-ctor.cpp b/clang/test/SemaCXX/cxx0x-deleted-default-ctor.cpp index b9af67948bfc9e..80efd810f09842 100644 --- a/clang/test/SemaCXX/cxx0x-deleted-default-ctor.cpp +++ b/clang/test/SemaCXX/cxx0x-deleted-default-ctor.cpp @@ -59,7 +59,7 @@ struct good_const { good_const gc; struct no_default { - no_default() = delete; // expected-note 4{{deleted here}} + no_default() = delete; // expected-note 5{{deleted here}} }; struct no_dtor { ~no_dtor() = delete; // expected-note 2{{deleted here}} @@ -108,8 +108,8 @@ struct has_friend { has_friend hf; struct defaulted_delete { - no_default nd; // expected-note {{because field 'nd' has a deleted default constructor}} - defaulted_delete() = default; // expected-note{{implicitly deleted here}} + no_default nd; // expected-note 2{{because field 'nd' has a deleted default constructor}} + defaulted_delete() = default; // expected-note{{implicitly deleted here}} expected-warning {{implicitly deleted}} }; defaulted_delete dd; // expected-error {{call to implicitly-deleted default constructor}} diff --git a/clang/test/SemaCXX/cxx17-compat.cpp b/clang/test/SemaCXX/cxx17-compat.cpp index 9d628da662b8b6..93e052fe7e73b8 100644 --- a/clang/test/SemaCXX/cxx17-compat.cpp +++ b/clang/test/SemaCXX/cxx17-compat.cpp @@ -1,5 +1,5 @@ // RUN: %clang_cc1 -fsyntax-only -std=c++17 -pedantic -verify %s -// RUN: %clang_cc1 -fsyntax-only -std=c++2a -Wc++17-compat-pedantic -verify %s +// RUN: %clang_cc1 -fsyntax-only -std=c++2a -Wc++17-compat-pedantic -verify %s -Wno-defaulted-function-deleted struct A {}; int (A::*pa)() const&; @@ -42,3 +42,15 @@ void copy_lambda() { Lambda = Lambda; } #else // expected-warning@-4 {{assignment of lambda is incompatible with C++ standards before C++2a}} #endif + +struct DefaultDeleteWrongTypeBase { + DefaultDeleteWrongTypeBase(DefaultDeleteWrongTypeBase&); +}; +struct DefaultDeleteWrongType : DefaultDeleteWrongTypeBase { + DefaultDeleteWrongType(const DefaultDeleteWrongType&) = default; +#if __cplusplus <= 201703L + // expected-error@-2 {{a member or base requires it to be non-const}} +#else + // expected-warning@-4 {{explicitly defaulting this copy constructor with a type different from the implicit type is incompatible with C++ standards before C++2a}} +#endif +}; diff --git a/clang/test/SemaCXX/dr1301.cpp b/clang/test/SemaCXX/dr1301.cpp index ec0db74554a870..b7dc91d5633000 100644 --- a/clang/test/SemaCXX/dr1301.cpp +++ b/clang/test/SemaCXX/dr1301.cpp @@ -6,7 +6,7 @@ struct A { // expected-note 2{{candidate}} int a = A().n; // expected-error {{no matching constructor}} struct B { - B() = delete; // expected-note 3{{here}} + B() = delete; // expected-note 4{{here}} int n; }; int b = B().n; // expected-error {{call to deleted}} @@ -17,8 +17,8 @@ struct C { int c = C().b.n; // expected-error {{call to implicitly-deleted default}} struct D { - D() = default; // expected-note {{here}} - B b; // expected-note {{'b' has a deleted default constructor}} + D() = default; // expected-note {{here}} expected-warning {{implicitly deleted}} + B b; // expected-note 2{{'b' has a deleted default constructor}} }; int d = D().b.n; // expected-error {{call to implicitly-deleted default}} diff --git a/clang/test/SemaCXX/microsoft-dtor-lookup-cxx11.cpp b/clang/test/SemaCXX/microsoft-dtor-lookup-cxx11.cpp index 5a399aa7eac22e..000492e0edf195 100644 --- a/clang/test/SemaCXX/microsoft-dtor-lookup-cxx11.cpp +++ b/clang/test/SemaCXX/microsoft-dtor-lookup-cxx11.cpp @@ -6,8 +6,8 @@ struct S { void operator delete(void*, double); } s; // expected-error {{attempt to use a deleted function}} -struct T { // expected-note{{virtual destructor requires an unambiguous, accessible 'operator delete'}} - virtual ~T() = default; // expected-note {{explicitly defaulted function was implicitly deleted here}} +struct T { // expected-note 2{{virtual destructor requires an unambiguous, accessible 'operator delete'}} + virtual ~T() = default; // expected-note {{explicitly defaulted function was implicitly deleted here}} expected-warning {{implicitly deleted}} void operator delete(void*, int); void operator delete(void*, double); } t; // expected-error {{attempt to use a deleted function}} diff --git a/clang/test/SemaTemplate/exception-spec-crash.cpp b/clang/test/SemaTemplate/exception-spec-crash.cpp index 4d9355974c9e6e..ebbb30a2c23b6c 100644 --- a/clang/test/SemaTemplate/exception-spec-crash.cpp +++ b/clang/test/SemaTemplate/exception-spec-crash.cpp @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s -// RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -DCXX_EXCEPTIONS -fsyntax-only -verify %s +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s -Wno-defaulted-function-deleted +// RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -DCXX_EXCEPTIONS -fsyntax-only -verify %s -Wno-defaulted-function-deleted template struct is_nothrow_move_constructible { static const bool value = false; diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html index aed443c4f6c66e..b7dbb2a36c847c 100755 --- a/clang/www/cxx_status.html +++ b/clang/www/cxx_status.html @@ -884,7 +884,7 @@

C++2a implementation status

const mismatch with defaulted copy constructor P0641R2 - No + SVN Consistent comparison (operator<=>)