[PATCH] D101566: Let -Wweak-template-vtables warn on implicit instantiations

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added reviewers: dblaikie, rsmith.
aaronpuchert requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Implicit instantiations of template classes with virtual methods might
cause the vtable (and all virtual functions) to be instantiated and
emitted in multiple translation units. An explicit instantiation
declaration next to the template definition would ensure that only the
translation unit with the corresponding definition emits the vtable.

There are two places where we could emit the warning, on the class or
the implicit instantiation. I decided for emitting the warning on the
class, because that's likely where the fix would need to be. (Unless the
programmer decides to drop the instantiation or give a template argument
internal linkage.)

Fixes PR18733.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101566

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/warn-weak-vtables.cpp


Index: clang/test/SemaCXX/warn-weak-vtables.cpp
===================================================================
--- clang/test/SemaCXX/warn-weak-vtables.cpp
+++ clang/test/SemaCXX/warn-weak-vtables.cpp
@@ -8,7 +8,7 @@
   virtual void f() { } 
 };
 
-template<typename T> struct B {
+template<typename T> struct B { // expected-warning {{there is no explicit instantiation declaration for 'B<int>', its vtable will be emitted in every translation unit}}
   virtual void f() { } 
 };
 
@@ -29,7 +29,8 @@
 // Use the vtables
 void uses_abc() {
   A a;
-  B<int> b;
+  B<int> b; // expected-note{{implicit instantiation first required here}}
+  B<C> b_internal;
   C c;
 }
 
@@ -63,7 +64,7 @@
   virtual void f();
 };
 
-template class TemplVirt<float>; // expected-warning{{explicit template instantiation 'TemplVirt<float>' will emit a vtable in every translation unit}}
+template class TemplVirt<float>;
 
 template<> struct TemplVirt<bool> {
   virtual void f();
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -17359,15 +17359,19 @@
     // no key function or the key function is inlined. Don't warn in C++ ABIs
     // that lack key functions, since the user won't be able to make one.
     if (Context.getTargetInfo().getCXXABI().hasKeyFunctions() &&
-        Class->isExternallyVisible() && ClassTSK != TSK_ImplicitInstantiation) {
+        Class->isExternallyVisible() &&
+        ClassTSK != TSK_ExplicitInstantiationDefinition) {
       const FunctionDecl *KeyFunctionDef = nullptr;
       if (!KeyFunction || (KeyFunction->hasBody(KeyFunctionDef) &&
                            KeyFunctionDef->isInlined())) {
-        Diag(Class->getLocation(),
-             ClassTSK == TSK_ExplicitInstantiationDefinition
-                 ? diag::warn_weak_template_vtable
-                 : diag::warn_weak_vtable)
-            << Class;
+        if (ClassTSK == TSK_ImplicitInstantiation) {
+          const auto *CTSD = cast<ClassTemplateSpecializationDecl>(Class);
+          Diag(Class->getLocation(), diag::warn_weak_template_vtable) << Class;
+          Diag(CTSD->getPointOfInstantiation(),
+               diag::note_instantiation_required_here)
+              << /* explicit */ false;
+        } else
+          Diag(Class->getLocation(), diag::warn_weak_vtable) << Class;
       }
     }
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1583,8 +1583,8 @@
   "emitted in every translation unit">,
   InGroup<DiagGroup<"weak-vtables">>, DefaultIgnore;
 def warn_weak_template_vtable : Warning<
-  "explicit template instantiation %0 will emit a vtable in every "
-  "translation unit">,
+  "there is no explicit instantiation declaration for %0; its vtable will be "
+  "emitted in every translation unit">,
   InGroup<DiagGroup<"weak-template-vtables">>, DefaultIgnore;
 
 def ext_using_undefined_std : ExtWarn<


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D101566.341624.patch
Type: text/x-patch
Size: 3143 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210429/a3ed02ae/attachment.bin>