From cbc820af7193e3ad9c71fbe6e3c8bd3c1166e540 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Mon, 22 Jul 2013 02:56:56 +0000 Subject: [PATCH] Implement DR257 / fix PR16659: A constructor for an abstract class does not call constructors for virtual base classes, so it is not an error if no initializer is present for the virtual base and the virtual base cannot be default initialized. Also provide a (disabled by default, for now) warning for the case where a virtual base class's initializer is ignored in an abstract class's constructor, and address a defect in DR257 where it was not carried through to C++11's rules for implicit deletion of special member functions. Based on a patch by Maurice Bos. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@186803 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/Diagnostic.h | 9 ++++- include/clang/Basic/DiagnosticSemaKinds.td | 4 ++ lib/Sema/SemaDeclCXX.cpp | 43 ++++++++++++++++++---- test/SemaCXX/abstract.cpp | 29 ++++++++++++++- 4 files changed, 76 insertions(+), 9 deletions(-) diff --git a/include/clang/Basic/Diagnostic.h b/include/clang/Basic/Diagnostic.h index 1354120ea5..0fcfdf7cb3 100644 --- a/include/clang/Basic/Diagnostic.h +++ b/include/clang/Basic/Diagnostic.h @@ -490,7 +490,14 @@ public: FatalErrorOccurred = true; LastDiagLevel = DiagnosticIDs::Ignored; } - + + /// \brief Determine whether the previous diagnostic was ignored. This can + /// be used by clients that want to determine whether notes attached to a + /// diagnostic will be suppressed. + bool isLastDiagnosticIgnored() const { + return LastDiagLevel == DiagnosticIDs::Ignored; + } + /// \brief Controls whether otherwise-unmapped extension diagnostics are /// mapped onto ignore/warning/error. /// diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 8acdf58dbc..1ca349920e 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -5613,6 +5613,10 @@ def warn_initializer_out_of_order : Warning< "%select{field|base class}0 %1 will be initialized after " "%select{field|base}2 %3">, InGroup, DefaultIgnore; +def warn_abstract_vbase_init_ignored : Warning< + "initializer for virtual base class %0 of abstract class %1 " + "will never be used">, + InGroup>, DefaultIgnore; def err_base_init_does_not_name_class : Error< "constructor initializer %0 does not name a class">; diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 25dbd07b41..9b85c10d59 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -3346,7 +3346,7 @@ bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, bool AnyErrors, for (unsigned i = 0; i < Initializers.size(); i++) { CXXCtorInitializer *Member = Initializers[i]; - + if (Member->isBaseInitializer()) Info.AllBaseFields[Member->getBaseClass()->getAs()] = Member; else @@ -3367,12 +3367,28 @@ bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, bool AnyErrors, if (CXXCtorInitializer *Value = Info.AllBaseFields.lookup(VBase->getType()->getAs())) { + // [class.base.init]p7, per DR257: + // A mem-initializer where the mem-initializer-id names a virtual base + // class is ignored during execution of a constructor of any class that + // is not the most derived class. + if (ClassDecl->isAbstract()) { + // FIXME: Provide a fixit to remove the base specifier. This requires + // tracking the location of the associated comma for a base specifier. + Diag(Value->getSourceLocation(), diag::warn_abstract_vbase_init_ignored) + << VBase->getType() << ClassDecl; + DiagnoseAbstractType(ClassDecl); + } + Info.AllToInit.push_back(Value); - } else if (!AnyErrors) { + } else if (!AnyErrors && !ClassDecl->isAbstract()) { + // [class.base.init]p8, per DR257: + // If a given [...] base class is not named by a mem-initializer-id + // [...] and the entity is not a virtual base class of an abstract + // class, then [...] the entity is default-initialized. bool IsInheritedVirtualBase = !DirectVBases.count(VBase); CXXCtorInitializer *CXXBaseInit; if (BuildImplicitBaseInitializer(*this, Constructor, Info.IIK, - VBase, IsInheritedVirtualBase, + VBase, IsInheritedVirtualBase, CXXBaseInit)) { HadError = true; continue; @@ -3913,6 +3929,12 @@ void Sema::DiagnoseAbstractType(const CXXRecordDecl *RD) { if (PureVirtualClassDiagSet && PureVirtualClassDiagSet->count(RD)) return; + // If the diagnostic is suppressed, don't emit the notes. We're only + // going to emit them once, so try to attach them to a diagnostic we're + // actually going to show. + if (Diags.isLastDiagnosticIgnored()) + return; + CXXFinalOverriderMap FinalOverriders; RD->getFinalOverriders(FinalOverriders); @@ -5055,10 +5077,17 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM, SMI.shouldDeleteForBase(BI)) return true; - for (CXXRecordDecl::base_class_iterator BI = RD->vbases_begin(), - BE = RD->vbases_end(); BI != BE; ++BI) - if (SMI.shouldDeleteForBase(BI)) - return true; + // Defect report (no number yet): do not consider virtual bases of + // constructors of abstract classes, since we are not going to construct + // them. This is an extension of DR257 into the C++11 behavior for special + // members. + if (!RD->isAbstract() || !SMI.IsConstructor) { + for (CXXRecordDecl::base_class_iterator BI = RD->vbases_begin(), + BE = RD->vbases_end(); + BI != BE; ++BI) + if (SMI.shouldDeleteForBase(BI)) + return true; + } for (CXXRecordDecl::field_iterator FI = RD->field_begin(), FE = RD->field_end(); FI != FE; ++FI) diff --git a/test/SemaCXX/abstract.cpp b/test/SemaCXX/abstract.cpp index 1c5b715775..d7e2d0a3dc 100644 --- a/test/SemaCXX/abstract.cpp +++ b/test/SemaCXX/abstract.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 -Wabstract-vbase-init #ifndef __GXX_EXPERIMENTAL_CXX0X__ #define __CONCAT(__X, __Y) __CONCAT1(__X, __Y) @@ -280,3 +280,30 @@ namespace pr12658 { foo(C(99)); // expected-error {{allocating an object of abstract class type 'pr12658::C'}} } } + +namespace pr16659 { + struct A { + A(int); + virtual void x() = 0; // expected-note {{unimplemented pure virtual method 'x' in 'RedundantInit'}} + }; + struct B : virtual A {}; + struct C : B { + C() : A(37) {} + void x() override {} + }; + + struct X { + friend class Z; + private: + X &operator=(const X&); + }; + struct Y : virtual X { // expected-note {{::X' has an inaccessible copy assignment}} + virtual ~Y() = 0; + }; + struct Z : Y {}; // expected-note {{::Y' has a deleted copy assignment}} + void f(Z &a, const Z &b) { a = b; } // expected-error {{copy assignment operator is implicitly deleted}} + + struct RedundantInit : virtual A { + RedundantInit() : A(0) {} // expected-warning {{initializer for virtual base class 'pr16659::A' of abstract class 'RedundantInit' will never be used}} + }; +} -- 2.40.0