From a318efd1f21a21358f45a201f7b6cb2b8b118ec0 Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Tue, 5 Jan 2010 19:06:31 +0000 Subject: [PATCH] Improve key-function computation for templates. In particular: - All classes can have a key function; templates don't change that. non-template classes when computing the key function. - We always mark all of the virtual member functions of class template instantiations. - The vtable for an instantiation of a class template has weak linkage. We could probably use available_externally linkage for vtables of classes instantiated by explicit instantiation declarations (extern templates), but GCC doesn't do this and I'm not 100% that the ABI permits it. llvm-svn: 92753 --- clang/lib/AST/DeclCXX.cpp | 22 +++------ clang/lib/AST/RecordLayoutBuilder.cpp | 11 ++--- clang/lib/CodeGen/CGVtable.cpp | 18 +++++++- clang/lib/CodeGen/CodeGenModule.cpp | 3 +- clang/lib/Sema/SemaDecl.cpp | 2 +- clang/lib/Sema/SemaDeclCXX.cpp | 32 +++++++++---- clang/test/CodeGenCXX/vtable-key-function.cpp | 18 ++++++++ clang/test/CodeGenCXX/vtable-linkage.cpp | 46 +++++++++++++++++++ 8 files changed, 115 insertions(+), 37 deletions(-) diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index bbbb19a35b4a..b30d335918ab 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -643,23 +643,15 @@ QualType CXXMethodDecl::getThisType(ASTContext &C) const { return C.getPointerType(ClassTy); } -static bool MethodHasBody(const CXXMethodDecl *MD, const FunctionDecl *&fn) { - // Simple case: function has a body - if (MD->getBody(fn)) - return true; - - // Complex case: function is an instantiation of a function which has a - // body, but the definition hasn't been instantiated. - const FunctionDecl *PatternDecl = MD->getTemplateInstantiationPattern(); - if (PatternDecl && PatternDecl->getBody(fn)) - return true; - - return false; -} - bool CXXMethodDecl::hasInlineBody() const { + // If this function is a template instantiation, look at the template from + // which it was instantiated. + const FunctionDecl *CheckFn = getTemplateInstantiationPattern(); + if (!CheckFn) + CheckFn = this; + const FunctionDecl *fn; - return MethodHasBody(this, fn) && !fn->isOutOfLine(); + return CheckFn->getBody(fn) && !fn->isOutOfLine(); } CXXBaseOrMemberInitializer:: diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index c914f3f82e8e..cfd89eaed350 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -719,11 +719,6 @@ ASTRecordLayoutBuilder::ComputeKeyFunction(const CXXRecordDecl *RD) { // If a class isnt' polymorphic it doesn't have a key function. if (!RD->isPolymorphic()) return 0; - - // A class template specialization or instantation does not have a key - // function. - if (RD->getTemplateSpecializationKind() != TSK_Undeclared) - return 0; // A class inside an anonymous namespace doesn't have a key function. (Or // at least, there's no point to assigning a key function to such a class; @@ -741,13 +736,13 @@ ASTRecordLayoutBuilder::ComputeKeyFunction(const CXXRecordDecl *RD) { if (MD->isPure()) continue; - if (MD->isInlineSpecified()) - continue; - // Ignore implicit member functions, they are always marked as inline, but // they don't have a body until they're defined. if (MD->isImplicit()) continue; + + if (MD->isInlineSpecified()) + continue; if (MD->hasInlineBody()) continue; diff --git a/clang/lib/CodeGen/CGVtable.cpp b/clang/lib/CodeGen/CGVtable.cpp index ba5a0c35439b..ca148da95606 100644 --- a/clang/lib/CodeGen/CGVtable.cpp +++ b/clang/lib/CodeGen/CGVtable.cpp @@ -1493,8 +1493,22 @@ void CGVtableInfo::MaybeEmitVtable(GlobalDecl GD) { llvm::GlobalVariable::LinkageTypes Linkage; if (RD->isInAnonymousNamespace() || !RD->hasLinkage()) Linkage = llvm::GlobalVariable::InternalLinkage; - else if (KeyFunction && !MD->isInlined()) - Linkage = llvm::GlobalVariable::ExternalLinkage; + else if (KeyFunction && !MD->isInlined()) { + switch (MD->getTemplateSpecializationKind()) { + case TSK_Undeclared: + case TSK_ExplicitSpecialization: + Linkage = llvm::GlobalVariable::ExternalLinkage; + break; + + case TSK_ImplicitInstantiation: + case TSK_ExplicitInstantiationDeclaration: + // FIXME: could an explicit instantiation declaration imply + // available_externally linkage? + case TSK_ExplicitInstantiationDefinition: + Linkage = llvm::GlobalVariable::WeakODRLinkage; + break; + } + } else Linkage = llvm::GlobalVariable::WeakODRLinkage; diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 16a01cfd58f3..85d57e7cbec3 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -547,7 +547,8 @@ bool CodeGenModule::MayDeferGeneration(const ValueDecl *Global) { const CXXRecordDecl *RD = MD->getParent(); if (MD->isOutOfLine() && RD->isDynamicClass()) { const CXXMethodDecl *KeyFunction = getContext().getKeyFunction(RD); - if (KeyFunction == MD->getCanonicalDecl()) + if (KeyFunction && + KeyFunction->getCanonicalDecl() == MD->getCanonicalDecl()) return false; } } diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index a15c5fea213a..0fc61e3b5877 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -3749,7 +3749,7 @@ void Sema::AddInitializerToDecl(DeclPtrTy dcl, ExprArg init, bool DirectInit) { if (getLangOptions().CPlusPlus) { // Make sure we mark the destructor as used if necessary. QualType InitType = VDecl->getType(); - if (const ArrayType *Array = Context.getAsArrayType(InitType)) + while (const ArrayType *Array = Context.getAsArrayType(InitType)) InitType = Context.getBaseElementType(Array); if (InitType->isRecordType()) FinalizeVarWithDestructor(VDecl, InitType); diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 204d7764682b..ecf95d7eb662 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -5702,19 +5702,31 @@ void Sema::MaybeMarkVirtualMembersReferenced(SourceLocation Loc, ClassesWithUnmarkedVirtualMembers.insert(std::make_pair(RD, Loc)); return; } - - const CXXMethodDecl *KeyFunction = Context.getKeyFunction(RD); - if (!KeyFunction) { - // This record does not have a key function, so we assume that the vtable - // will be emitted when it's used by the constructor. - if (!isa(MD)) + switch (RD->getTemplateSpecializationKind()) { + case TSK_Undeclared: + case TSK_ExplicitSpecialization: { + const CXXMethodDecl *KeyFunction = Context.getKeyFunction(RD); + + if (!KeyFunction) { + // This record does not have a key function, so we assume that the vtable + // will be emitted when it's used by the constructor. + if (!isa(MD)) + return; + } else if (KeyFunction->getCanonicalDecl() != MD->getCanonicalDecl()) { + // We don't have the right key function. return; - } else if (KeyFunction->getCanonicalDecl() != MD->getCanonicalDecl()) { - // We don't have the right key function. - return; + } + break; } - + + case TSK_ImplicitInstantiation: + case TSK_ExplicitInstantiationDeclaration: + case TSK_ExplicitInstantiationDefinition: + // Always mark the virtual members of an instantiated template. + break; + } + // Mark the members as referenced. MarkVirtualMembersReferenced(Loc, RD); ClassesWithUnmarkedVirtualMembers.erase(RD); diff --git a/clang/test/CodeGenCXX/vtable-key-function.cpp b/clang/test/CodeGenCXX/vtable-key-function.cpp index 90e8ea66f769..97a546f8c932 100644 --- a/clang/test/CodeGenCXX/vtable-key-function.cpp +++ b/clang/test/CodeGenCXX/vtable-key-function.cpp @@ -13,3 +13,21 @@ struct A { A::A() { } A::A(int) { } } + +// Make sure that we don't assert when building the vtable for a class +// template specialization or explicit instantiation with a key +// function. +template +struct Base { + virtual ~Base(); +}; + +template +struct Derived : public Base { }; + +template<> +struct Derived : public Base { + virtual void anchor(); +}; + +void Derived::anchor() { } diff --git a/clang/test/CodeGenCXX/vtable-linkage.cpp b/clang/test/CodeGenCXX/vtable-linkage.cpp index 6d3cf240096b..23d78aadf7eb 100644 --- a/clang/test/CodeGenCXX/vtable-linkage.cpp +++ b/clang/test/CodeGenCXX/vtable-linkage.cpp @@ -30,6 +30,30 @@ void D::f() { } static struct : D { } e; +template +struct E { + virtual ~E(); +}; + +template E::~E() { } + +template<> +struct E { + virtual void anchor(); +}; + +void E::anchor() { } + +template struct E; +extern template struct E; + +void use_E() { + E ei; + (void)ei; + E el; + (void)el; +} + // B has a key function that is not defined in this translation unit so its vtable // has external linkage. // CHECK: @_ZTV1B = external constant @@ -45,14 +69,36 @@ static struct : D { } e; // CHECK: @_ZTI1D = constant // CHECK: @_ZTV1D = constant +// E is an explicit specialization with a key function defined +// in this translation unit, so its vtable should have external +// linkage. +// CHECK: @_ZTV1EIcE = constant + +// E is an explicit template instantiation with a key function +// defined in this translation unit, so its vtable should have +// weak_odr linkage. +// CHECK: @_ZTV1EIsE = weak_odr constant + +// E is an implicit template instantiation with a key function +// defined in this translation unit, so its vtable should have +// weak_odr linkage. +// CHECK: @_ZTV1EIlE = weak_odr constant + // The anonymous struct for e has no linkage, so the vtable should have // internal linkage. // CHECK: @"_ZTS3$_0" = internal constant // CHECK: @"_ZTI3$_0" = internal constant // CHECK: @"_ZTV3$_0" = internal constant +// E is an explicit template instantiation declaration. It has a +// key function that is not instantiation, so we should only reference +// its vtable, not define it. +// CHECK: @_ZTV1EIiE = external constant + // The A vtable should have internal linkage since it is inside an anonymous // namespace. // CHECK: @_ZTSN12_GLOBAL__N_11AE = internal constant // CHECK: @_ZTIN12_GLOBAL__N_11AE = internal constant // CHECK: @_ZTVN12_GLOBAL__N_11AE = internal constant + +