Let -Wweak-template-vtables warn on implicit instantiations

[aaronpuchert]

Summary

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.

[dblaikie]

Along time ago Clang had a fairly strong aversion to implementing "off by default" warnings (though clearly weak-vtables was an exception to that - it's a pretty esoteric warning even at the best of times/without this special case) because they would tend to go unused and unmaintained. I'm sort of inclined towards this subset of the warning (either the poorly implemented one originally, or this version) being that sort of category, and that it'd be better to delete it.

Honestly in retrospect I think it was a bug in the warning that should've been fixed by not warning in this case, rather than splitting it out into its own warning group. The warning was originally written to ignore implicit template instantiations - and should've ignored explicit instantiations too, I think.

@doug.gregor - any interest in revisiting this (there's some discussion on the linked bug, which links back to a discussion we had many many years ago... )?

@aaronpuchert - do you have plans to use this warning, if it's implemented/changed in this way?
@respindola - do you have any interest in using this warning if it were implemented/changed in this way?

[aaronpuchert]

In D101566#2726820, @dblaikie wrote:

Along time ago Clang had a fairly strong aversion to implementing "off by default" warnings ([...]) because they would tend to go unused and unmaintained.

That was a valid reason, but now there are code bases that work with -Weverything and disable the warnings they are not interested in.

I'm sort of inclined towards this subset of the warning (either the poorly implemented one originally, or this version) being that sort of category, and that it'd be better to delete it.

We have other warnings that are basically just about "cleaner" object files, like -Wmissing-prototypes or -Wmissing-variable-declarations. (These enforce adding static on functions/variables not previously declared. Though together with -Wunused-{function,variable} one could find more unused functions/variables, which could be seen as improving code quality.)

The warning was originally written to ignore implicit template instantiations - and should've ignored explicit instantiations too, I think.

Looking at the warning name I think you're right. There is no way to make the vtables strong. But I don't think anybody cares about this technicality, the warning is there to reduce compile time and object file sizes.

In fact it's probably not so much about emitting the vtable, but about emitting all virtual functions, even if they end up unused. In the template case this also implies instantiation.

@aaronpuchert - do you have plans to use this warning, if it's implemented/changed in this way?

Let's say that I'm considering it. I'll need to see how often this fires and whether the explicit instantiations make sense to me.

[dblaikie]

In D101566#2727244, @aaronpuchert wrote:

In D101566#2726820, @dblaikie wrote:

Along time ago Clang had a fairly strong aversion to implementing "off by default" warnings ([...]) because they would tend to go unused and unmaintained.

That was a valid reason, but now there are code bases that work with -Weverything and disable the warnings they are not interested in.

Sure - though that's likely a pretty small proportion of the userbase/not something we encourage/support/etc. So still not necessarily getting a lot of value from new warnings - especially this one, I'd imagine (I'd expect most users would turn this warning off and never think about it again - the intersection of people using -Weverything (small population) and people who will want to put explicit instantiations of all their templates with vtables (small as well) seems vanishingly small.

I'm sort of inclined towards this subset of the warning (either the poorly implemented one originally, or this version) being that sort of category, and that it'd be better to delete it.

We have other warnings that are basically just about "cleaner" object files, like -Wmissing-prototypes or -Wmissing-variable-declarations. (These enforce adding static on functions/variables not previously declared. Though together with -Wunused-{function,variable} one could find more unused functions/variables, which could be seen as improving code quality.)

Missing declarations can be helpful for finding errors in source code earlier, rather than getting harder to understand linker errors (if you miss "const" when defining a function and create an overload instead of defining the thing you declared in the header, for instance).

And at least both those flags were probably implemented for GCC compatibility, which I don't think applies to this warning.

The warning was originally written to ignore implicit template instantiations - and should've ignored explicit instantiations too, I think.

Looking at the warning name I think you're right. There is no way to make the vtables strong. But I don't think anybody cares about this technicality, the warning is there to reduce compile time and object file sizes.

In fact it's probably not so much about emitting the vtable, but about emitting all virtual functions, even if they end up unused. In the template case this also implies instantiation.

@aaronpuchert - do you have plans to use this warning, if it's implemented/changed in this way?

Let's say that I'm considering it. I'll need to see how often this fires and whether the explicit instantiations make sense to me.

I think it'd be good to gather that data first before committing it to Clang - that's usually what we try to do to justify the warning.

[aaronpuchert]

In D101566#2727312, @dblaikie wrote:

I think it'd be good to gather that data first before committing it to Clang - that's usually what we try to do to justify the warning.

Well, except that this isn't a new warning. But yeah, it does make sense to look at some actual code bases.

[aaronpuchert]

• Fix punctuation in expected warning message.
• Fix test failure: the instantiated class might not be a template, it could also be the member of another template.

[aaronpuchert]

So I tried this in two code bases, both of which don't have -Wweak-vtables enabled though.

In the first (~500 TUs) there were a couple of interesting finds. But they are hard to fix, even where explicit instantiations were already there: the corresponding instantiation declarations would require additional includes in the headers because many of the templates are constrained, and concepts don't work well with forward declarations. That's also why we had to disable -Wundefined-func-templates there, which is otherwise a useful warning.

In the second code base (~30,000 TUs) it looks a lot more useful. There are many occurrences, but deduplicating and sorting by number of files they occur in finds a couple of templates where explicit instantiation could improve compile times and build sizes. To give some examples, its own standard library has instantiations basic_ios<char, char_traits<char>> plus the wchar_t equivalent or basic_ostream<char, char_traits<char>> and so on coming up in most TUs. Instantiating them explicitly would be natural and likely beneficial.

Overall it's not a warning that I would enable in regular builds, but rather like -Wweak-vtables collect the most common occurrences in special runs and do something about them. (The total number of warnings, not deduplicated, runs into the millions for both warnings on the second code base.)

[dblaikie]

In D101566#2730555, @aaronpuchert wrote:

So I tried this in two code bases, both of which don't have -Wweak-vtables enabled though.

In the first (~500 TUs) there were a couple of interesting finds. But they are hard to fix, even where explicit instantiations were already there: the corresponding instantiation declarations would require additional includes in the headers because many of the templates are constrained, and concepts don't work well with forward declarations. That's also why we had to disable -Wundefined-func-templates there, which is otherwise a useful warning.

In the second code base (~30,000 TUs) it looks a lot more useful. There are many occurrences, but deduplicating and sorting by number of files they occur in finds a couple of templates where explicit instantiation could improve compile times and build sizes. To give some examples, its own standard library has instantiations basic_ios<char, char_traits<char>> plus the wchar_t equivalent or basic_ostream<char, char_traits<char>> and so on coming up in most TUs. Instantiating them explicitly would be natural and likely beneficial.

Out of curiosity - have you tried it & measured any significant improvement/value in build times/sizes/etc?

Overall it's not a warning that I would enable in regular builds, but rather like -Wweak-vtables collect the most common occurrences in special runs and do something about them. (The total number of warnings, not deduplicated, runs into the millions for both warnings on the second code base.)

That sort of use case sounds more like what we'd use clang-tidy for these days.

This doesn't sound especially compelling for a warning (& still seems pretty much not what the original weak-vtables warning was intending to do for templates - and an inversion of the weak-template-vtables (& so I think, if we are going to have this new thing as a warning, it should have a different name and the existing name should be removed)).

I still really think the best thing is to delete the existing weak-template-vtables warning entirely.

[aaronpuchert]

In D101566#2730746, @dblaikie wrote:

Out of curiosity - have you tried it & measured any significant improvement/value in build times/sizes/etc?

No, I fear that would take too much time. (Not so much the benchmarking, but making a number of fixes that can be expected to make a dent.)

This doesn't sound especially compelling for a warning (& still seems pretty much not what the original weak-vtables warning was intending to do for templates - and an inversion of the weak-template-vtables (& so I think, if we are going to have this new thing as a warning, it should have a different name and the existing name should be removed)).

I still really think the best thing is to delete the existing weak-template-vtables warning entirely.

I still don't understand the difference. You can of course argue that explicit instantiations are still weak, but I'd be curious why anyone would care about that. What is the reason behind -Wweak-vtables if not compile time or build size reductions? I can just guess, because the original bug 6116 doesn't state a reason.

[dblaikie]

In D101566#2733172, @aaronpuchert wrote:

In D101566#2730746, @dblaikie wrote:

Out of curiosity - have you tried it & measured any significant improvement/value in build times/sizes/etc?

No, I fear that would take too much time. (Not so much the benchmarking, but making a number of fixes that can be expected to make a dent.)

Makes it hard to justify the complexity in the compiler if it's hard to justify/support the value of the warning.

This doesn't sound especially compelling for a warning (& still seems pretty much not what the original weak-vtables warning was intending to do for templates - and an inversion of the weak-template-vtables (& so I think, if we are going to have this new thing as a warning, it should have a different name and the existing name should be removed)).

I still really think the best thing is to delete the existing weak-template-vtables warning entirely.

I still don't understand the difference. You can of course argue that explicit instantiations are still weak, but I'd be curious why anyone would care about that. What is the reason behind -Wweak-vtables if not compile time or build size reductions? I can just guess, because the original bug 6116 doesn't state a reason.

I believe it's compile time/build time, yes - but yeah, it's pretty questionable/suspect. LLVM's the only project I know of with it as a coding convention/guideline/rule - and even we haven't even remotely tried to enforce it. (& when I did do a bit of work to add more key functions people reasonably questioned the value of them - and I didn't really have data to support it, I could only point to the fact that I was implementing the stated policy/style guide)

[aaronpuchert]

In D101566#2734948, @dblaikie wrote:

Makes it hard to justify the complexity in the compiler if it's hard to justify/support the value of the warning.

The complexity for -Wweak-template-vtables is just 10 lines of code. We're just using information that's already there.

I believe it's compile time/build time, yes - but yeah, it's pretty questionable/suspect. LLVM's the only project I know of with it as a coding convention/guideline/rule - and even we haven't even remotely tried to enforce it. (& when I did do a bit of work to add more key functions people reasonably questioned the value of them - and I didn't really have data to support it, I could only point to the fact that I was implementing the stated policy/style guide)

Just to make sure I got this right, you're suggesting to remove both warnings? I agree that their value is pretty limited and it would be consistent. I'm just not sure if maybe someone is using -Wweak-vtables (and would consider using -Wweak-template-vtables if it did the right thing).

[dblaikie]

In D101566#2785190, @aaronpuchert wrote:

In D101566#2734948, @dblaikie wrote:

Makes it hard to justify the complexity in the compiler if it's hard to justify/support the value of the warning.

The complexity for -Wweak-template-vtables is just 10 lines of code. We're just using information that's already there.

I believe it's compile time/build time, yes - but yeah, it's pretty questionable/suspect. LLVM's the only project I know of with it as a coding convention/guideline/rule - and even we haven't even remotely tried to enforce it. (& when I did do a bit of work to add more key functions people reasonably questioned the value of them - and I didn't really have data to support it, I could only point to the fact that I was implementing the stated policy/style guide)

Just to make sure I got this right, you're suggesting to remove both warnings?

Right - to remove -Wweak-template-vtable in its entirety. The original implementation explicitly didn't warn on implicit instantiations and I think the fact that it warned on explicit instantiations is more bug than feature - and we should treat it that way.

[aaronpuchert]

In D101566#2785271, @dblaikie wrote:

Right - to remove -Wweak-template-vtable in its entirety. The original implementation explicitly didn't warn on implicit instantiations and I think the fact that it warned on explicit instantiations is more bug than feature - and we should treat it that way.

What about -Wweak-vtables, what's the reasoning for keeping that?

[dblaikie]

In D101566#2786112, @aaronpuchert wrote:

In D101566#2785271, @dblaikie wrote:

Right - to remove -Wweak-template-vtable in its entirety. The original implementation explicitly didn't warn on implicit instantiations and I think the fact that it warned on explicit instantiations is more bug than feature - and we should treat it that way.

What about -Wweak-vtables, what's the reasoning for keeping that?

That it doesn't seem to have major issues to me - so I'm willing to leave it there, even if it's questionable as to whether anyone uses it.

[aaronpuchert]

@dblaikie, does https://bugs.llvm.org/show_bug.cgi?id=18733#c17 or the following comment change anything about your position?

[dblaikie]

In D101566#2891699, @aaronpuchert wrote:

@dblaikie, does https://bugs.llvm.org/show_bug.cgi?id=18733#c17 or the following comment change anything about your position?

No, not really.

This patch is still conflating two things - effectively removing an existing warning (which I agree with) and adding a new one (which I think is questionable at best - but in any case should be evaluated on its own merits (& if we did that, we might consider generalizing it beyond only templates with vtables, for instance (why are they more deserving of explicit instantiations than other templates?)), not in relation to the existing broken warning). The two warnings (the buggy one being removed, and the new one being proposed) are exact opposites of each other, with no overlap - it doesn't seem suitable to conflate the two in one review, and potentially not to reuse the same warning flag.

[aaronpuchert]

In D101566#2891764, @dblaikie wrote:

This patch is still conflating two things - effectively removing an existing warning (which I agree with) and adding a new one (which I think is questionable at best - but in any case should be evaluated on its own merits [...], not in relation to the existing broken warning).

There are different ways to look at the situation. I understand your argument, but we have a bug report against this warning, and everyone we've heard who wanted to use the warning seems to have wanted what it does after this fix. (I'm including the original two commenters on the bug, @respindola and @dfaure-kdab.) That is an argument to just fix the warning and let it do what they expected.

If those people thought the warning shouldn't exist at all, why were they using it? Or if they thought the name was misleading, why didn't their comments say that? The comments all said that they think it does the wrong thing.

if we did that, we might consider generalizing it beyond only templates with vtables, for instance (why are they more deserving of explicit instantiations than other templates?)

They behave differently:

• For template classes without vtable, instantiating the class doesn't instantiate any member functions, which are instantiated when odr-used.
• For template classes with vtable, instantiating the constructor requires instantiating the vtable, which requires instantiating all virtual members.

The situation is similar to non-templates actually. If there is no vtable, we can emit functions on demand, but if there is one, we have to emit all virtual functions if we need the constructor, and if there is no key function.

[...] and potentially not to reuse the same warning flag.

My point is though, and the Bugzilla commenter seems to agree, that both address the same issue for different entities: one for non-template classes, the other for template classes. So I think the name is absolutely fine.

As I've pointed out earlier, disruption isn't a good argument for separately removing this warning and adding another: removing the warning will be disruptive for users of -Weverything, while for obvious reasons we don't expect anyone to have it active and care about it, as it doesn't really make sense right now.

[dblaikie]

In D101566#2891923, @aaronpuchert wrote:

In D101566#2891764, @dblaikie wrote:

This patch is still conflating two things - effectively removing an existing warning (which I agree with) and adding a new one (which I think is questionable at best - but in any case should be evaluated on its own merits [...], not in relation to the existing broken warning).

There are different ways to look at the situation. I understand your argument, but we have a bug report against this warning, and everyone we've heard who wanted to use the warning seems to have wanted what it does after this fix. (I'm including the original two commenters on the bug, @respindola and @dfaure-kdab.) That is an argument to just fix the warning and let it do what they expected.

If those people thought the warning shouldn't exist at all, why were they using it?

I assume the only way they came across it was because of using -Weverything.

Or if they thought the name was misleading, why didn't their comments say that? The comments all said that they think it does the wrong thing.

For Rafael - probably because he didn't look at all the cases the warning does catch & see that it's pretty much entirely no use - his suggestion was to only detect/warn on explicit instantiations in headers (where they could produce duplication), which would still be a subset of the existing warning behavior & a subset that's actionable at least. That's different from your proposal to invert the warning, which I think is quite different & not suitable to tie together like this.

if we did that, we might consider generalizing it beyond only templates with vtables, for instance (why are they more deserving of explicit instantiations than other templates?)

They behave differently:

• For template classes without vtable, instantiating the class doesn't instantiate any member functions, which are instantiated when odr-used.
• For template classes with vtable, instantiating the constructor requires instantiating the vtable, which requires instantiating all virtual members.

Lots of classes have many non-virtual functions in addition to the virtual ones - I'd bet on average probably significantly more, and the explicit instantiation will instantiate all those non-virtual ones too, which might not be what the user wants if they similarly don't want to explicitly instantiate their non-polymorphic templates.

The situation is similar to non-templates actually. If there is no vtable, we can emit functions on demand, but if there is one, we have to emit all virtual functions if we need the constructor, and if there is no key function.

[...] and potentially not to reuse the same warning flag.

My point is though, and the Bugzilla commenter seems to agree, that both address the same issue for different entities: one for non-template classes, the other for template classes. So I think the name is absolutely fine.

As I've pointed out earlier, disruption isn't a good argument for separately removing this warning and adding another: removing the warning will be disruptive for users of -Weverything, while for obvious reasons we don't expect anyone to have it active and care about it, as it doesn't really make sense right now.

-Weverything is not intended for this use ( https://quuxplusone.github.io/blog/2018/12/06/dont-use-weverything/ for a good write up and various other comments on the issue) - breaking -Weverything users is to some degree a good thing: it's a reminder that what they're doing is unsupported and discouraged.

[aaronpuchert]

In D101566#2891972, @dblaikie wrote:

For Rafael - probably because he didn't look at all the cases the warning does catch & see that it's pretty much entirely no use

Right, he didn't suggest this particular fix but another one.

his suggestion was to only detect/warn on explicit instantiations in headers (where they could produce duplication), which would still be a subset of the existing warning behavior & a subset that's actionable at least. That's different from your proposal to invert the warning, which I think is quite different & not suitable to tie together like this.

Though that's an ODR violation independent of the existence of virtual functions. It doesn't have anything to do with weak vtables.

[aaronpuchert]

Let's add @aaron.ballman to get a third opinion. The discussion is meandering a bit, but you should understand the gist from the first comments or the bug report. The question is whether it's legitimate to fix this warning or whether the only legitimate course of action is to remove it entirely (and possibly re-propose it independently).

[aaron.ballman]

In D101566#2892163, @aaronpuchert wrote:

Let's add @aaron.ballman to get a third opinion. The discussion is meandering a bit, but you should understand the gist from the first comments or the bug report. The question is whether it's legitimate to fix this warning or whether the only legitimate course of action is to remove it entirely (and possibly re-propose it independently).

A few random thoughts.

Along time ago Clang had a fairly strong aversion to implementing "off by default" warnings

That's not a long time ago, that's the status quo. Off-by-default warnings are generally discouraged unless there's a very compelling reason to have them. A long time ago we used to be more relaxed about that, but experience taught us that off-by-default warnings are a poor value proposition in general.

That was a valid reason, but now there are code bases that work with -Weverything and disable the warnings they are not interested in.

-Weverything is not a particularly compelling use case IMO. We tell people not to enable it and expect good things to happen over time.

Honestly in retrospect I think it was a bug in the warning that should've been fixed by not warning in this case, rather than splitting it out into its own warning group. The warning was originally written to ignore implicit template instantiations - and should've ignored explicit instantiations too, I think.

I tend to agree, but don't have strong opinions on it. If I'm following along properly, my suggestion would be to commit the uncontentious bit (silencing the diagnostic in the problematic cases) in this review. If still desired, we can always start a new review to see if there's sentiment for the new warning behavior (under the existing diagnostic name or under a new one), but hopefully the proposed new behavior would be palatable for an on-by-default warning as that's more compelling than tweaks to an off-by-default warning that the original author isn't sure should even exist.

[aaronpuchert]

In D101566#2896911, @aaron.ballman wrote:

Off-by-default warnings are generally discouraged unless there's a very compelling reason to have them.

There are IMO valuable warnings that can be noisy initially and will probably never be on-by-default, like -Wmissing-prototypes. There are controversial warnings like -Wshadow (some people actually like shadowing, others do not). There are contradictory warnings like GCC's -Wswitch-default and our -Wcovered-switch-default. Warnings for compatibility with older standards. Warnings about certain implicit casts that are way too noisy in many code bases, but some might want them. Only having on-by-default warnings is just not realistic, there will always be many exceptions. Partly because the language moves on, but code needs time to catch up.

But in any event, my goal here wasn't to introduce this warning, just to fix it, given that it's there.

That was a valid reason, but now there are code bases that work with -Weverything and disable the warnings they are not interested in.

-Weverything is not a particularly compelling use case IMO. We tell people not to enable it and expect good things to happen over time.

That was in response to off-by-default warnings remaining unused.

If I'm following along properly, my suggestion would be to commit the uncontentious bit (silencing the diagnostic in the problematic cases) in this review.

Silencing it in problematic cases means never emitting it at all. So you agree with @dblaikie that it should be removed.

If still desired, we can always start a new review to see if there's sentiment for the new warning behavior (under the existing diagnostic name or under a new one), but hopefully the proposed new behavior would be palatable for an on-by-default warning as that's more compelling than tweaks to an off-by-default warning that the original author isn't sure should even exist.

Likely not.

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def err_non_consteval_override : Error<
   "non-consteval function %0 cannot override a consteval function">;
 
 def warn_weak_vtable : Warning<
   "%0 has no out-of-line virtual method definitions; its vtable will be "
   "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<
   "using directive refers to implicitly-defined namespace 'std'">;
 
 // C++ exception specifications
 def err_exception_spec_in_typedef : Error<
   "exception specifications are not allowed in %select{typedefs|type aliases}0">;

clang/lib/Sema/SemaDeclCXX.cpp

for (unsigned I = 0; I != VTableUses.size(); ++I) {
     CXXRecordDecl *Canonical = Class->getCanonicalDecl();
     if (VTablesUsed[Canonical])
       Consumer.HandleVTable(Class);
 
     // Warn if we're emitting a weak vtable. The vtable will be weak if there is
     // 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) {
+          Diag(Class->getLocation(), diag::warn_weak_template_vtable) << Class;
+
+          SourceLocation InstLoc;
+          if (const auto *CTSD =
+                  dyn_cast<ClassTemplateSpecializationDecl>(Class))
+            InstLoc = CTSD->getPointOfInstantiation();
+          else
+            InstLoc =
+                Class->getMemberSpecializationInfo()->getPointOfInstantiation();
+          Diag(InstLoc, diag::note_instantiation_required_here) << false;
+        } else
+          Diag(Class->getLocation(), diag::warn_weak_vtable) << Class;
       }
     }
   }
   VTableUses.clear();
 
   return DefinedAnything;
 }
 

clang/test/SemaCXX/warn-weak-vtables.cpp

 // RUN: %clang_cc1 %s -fsyntax-only -verify -triple %itanium_abi_triple -Wweak-vtables -Wweak-template-vtables
 //
 // Check that this warning is disabled on MS ABI targets which don't have key
 // functions.
 // RUN: %clang_cc1 %s -fsyntax-only -triple %ms_abi_triple -Werror -Wweak-vtables -Wweak-template-vtables
 
 struct A { // expected-warning {{'A' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit}}
   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() { } 
 };
 
 namespace {
   struct C { 
     virtual void f() { }
   };
 }
 
 void f() {
   struct A {
     virtual void f() { }
   };
 
   A a;
 }
 
 // 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;
 }
 
 // <rdar://problem/9979458>
 class Parent {
 public:
   Parent() {}
   virtual ~Parent();
void uses_derived() {
   Derived d;
   VeryDerived vd;
 }
 
 template<typename T> struct TemplVirt {
   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();
 };
 
 template<> struct TemplVirt<long> { // expected-warning{{'TemplVirt<long>' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit}}
   virtual void f() {}
 };
 
 void uses_templ() {
   TemplVirt<float> f;
   TemplVirt<bool> b;
   TemplVirt<long> l;
 }