[PATCH] PR6677 and explicit specializations of "key functions"

[Richard Smith]

r99443 for PR6677 added this test (now
in test/CodeGenCXX/template-instantiation.cpp):

// CHECK: @_ZTVN5test018stdio_sync_filebufIwEE = unnamed_addr constant

namespace test0 {
  struct  basic_streambuf   {
    virtual       ~basic_streambuf();
  };
  template<typename _CharT >
  struct stdio_sync_filebuf : public basic_streambuf {
    virtual void      xsgetn();
  };

  // This specialization should cause the vtable to be emitted, even with
  // the following extern template declaration.
  template<> void stdio_sync_filebuf<wchar_t>::xsgetn()  {
  }
  extern template class stdio_sync_filebuf<wchar_t>;
}

This looks bogus to me. Per computeKeyFunction:

  // Template instantiations don't have key functions,see Itanium C++ ABI
5.2.6.

... and indeed 5.2.6 in the ABI says we emit the vtable in every TU.

Especially troubling is that if you remove the explicit instantiation
declaration, we no longer emit the vtable, so if this *is* necessary for
some reason, we're not doing it right.

So... I think this was originally incorrect, and still works only due to a
bug. The attached patch fixes the glitch, but I wanted to make sure I'm not
missing some subtlety here before going ahead with this.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140318/68098594/attachment.html>
-------------- next part --------------
Index: lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- lib/AST/RecordLayoutBuilder.cpp	(revision 204194)
+++ lib/AST/RecordLayoutBuilder.cpp	(working copy)
@@ -1957,10 +1957,11 @@
   if (!RD->isExternallyVisible())
     return 0;
 
-  // Template instantiations don't have key functions,see Itanium C++ ABI 5.2.6.
+  // Template instantiations don't have key functions per Itanium C++ ABI 5.2.6.
   // Same behavior as GCC.
   TemplateSpecializationKind TSK = RD->getTemplateSpecializationKind();
   if (TSK == TSK_ImplicitInstantiation ||
+      TSK == TSK_ExplicitInstantiationDeclaration ||
       TSK == TSK_ExplicitInstantiationDefinition)
     return 0;
 
Index: test/CodeGenCXX/template-instantiation.cpp
===================================================================
--- test/CodeGenCXX/template-instantiation.cpp	(revision 204194)
+++ test/CodeGenCXX/template-instantiation.cpp	(working copy)
@@ -5,11 +5,15 @@
 //
 // CHECK-NOT: @_ZTVN5test118stdio_sync_filebufIwEE = constant
 // CHECK-NOT: _ZTVN5test315basic_fstreamXXIcEE
-// CHECK: @_ZTVN5test018stdio_sync_filebufIwEE = unnamed_addr constant
+// CHECK-NOT: @_ZTVN5test018stdio_sync_filebufIA1_iEE
+// CHECK-NOT: @_ZTVN5test018stdio_sync_filebufIA2_iEE
+// CHECK:     @_ZTVN5test018stdio_sync_filebufIA3_iEE = weak_odr unnamed_addr constant
 
 // CHECK: @_ZN7PR100011SIiE3arrE = weak_odr global [3 x i32]
 // CHECK-NOT: @_ZN7PR100011SIiE3arr2E = weak_odr global [3 x i32]A
 
+// CHECK:     @_ZTVN5test018stdio_sync_filebufIA4_iEE = linkonce_odr unnamed_addr constant
+
 // CHECK-NOT: _ZTVN5test31SIiEE
 // CHECK-NOT: _ZTSN5test31SIiEE
 
@@ -39,11 +43,21 @@
     virtual void      xsgetn();
   };
 
-  // This specialization should cause the vtable to be emitted, even with
-  // the following extern template declaration.
-  template<> void stdio_sync_filebuf<wchar_t>::xsgetn()  {
+  // This specialization is not a key function, so doesn't cause the vtable to
+  // be instantiated unless we're instantiating a class definition anyway.
+  template<> void stdio_sync_filebuf<int[1]>::xsgetn()  {
   }
-  extern template class stdio_sync_filebuf<wchar_t>;
+  template<> void stdio_sync_filebuf<int[2]>::xsgetn()  {
+  }
+  template<> void stdio_sync_filebuf<int[3]>::xsgetn()  {
+  }
+  template<> void stdio_sync_filebuf<int[4]>::xsgetn()  {
+  }
+  extern template class stdio_sync_filebuf<int[2]>;
+
+  // These two both cause vtables to be emitted.
+  template class stdio_sync_filebuf<int[3]>;
+  stdio_sync_filebuf<int[4]> implicit_instantiation;
 }
 
 namespace test1 {