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

@@ -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::

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;

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;
   

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;
       }
     }

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);

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<CXXConstructorDecl>(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<CXXConstructorDecl>(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);

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<typename T>
+struct Base {
+  virtual ~Base();
+};
+
+template<typename T>
+struct Derived : public Base<T> { };
+
+template<>
+struct Derived<char> : public Base<char> {
+  virtual void anchor();
+};
+
+void Derived<char>::anchor() { }

clang/test/CodeGenCXX/vtable-linkage.cpp

@@ -30,6 +30,30 @@ void D::f() { }
 
 static struct : D { } e;
 
+template<typename T>
+struct E {
+  virtual ~E();
+};
+
+template<typename T> E<T>::~E() { }
+
+template<>
+struct E<char> {
+  virtual void anchor();
+};
+
+void E<char>::anchor() { }
+
+template struct E<short>;
+extern template struct E<int>;
+
+void use_E() {
+  E<int> ei;
+  (void)ei;
+  E<long> 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<char> is an explicit specialization with a key function defined
+// in this translation unit, so its vtable should have external
+// linkage.
+// CHECK: @_ZTV1EIcE = constant
+
+// E<short> 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<long> 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<int> 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
+
+