[cfe-commits] [cfe-dev] anchoring explicit template instantiations

[Douglas Gregor]

On Dec 7, 2011, at 3:02 PM, David Blaikie wrote:

> On Sat, Dec 3, 2011 at 1:07 PM, Douglas Gregor <dgregor at apple.com> wrote:
>> 
>> On Dec 1, 2011, at 12:12 AM, David Blaikie wrote:
>> 
>>> [Forwarding to cfe-dev for the purpose of discussing what to do with
>>> weak-vtables in explicit template instantiations - when/how should the
>>> warning be emitted in this case, etc, details below]
>>> 
>>> For a bit of an experiment I've been trying to compile LLVM & Clang
>>> with -Weverything (disabling any errors that seem like more noise/less
>>> interesting). One warning I've recently hit a few instances of is
>>> -Wweak-vtable which is, in fact, an explicitly documented LLVM coding
>>> standard ( http://llvm.org/docs/CodingStandards.html#ll_virtual_anch
>>> ). Some instances of this have been easy to fix but a particular set of
>>> them have been a little more problematic.
>>> 
>>> If you take a look at CommandLine.h/CommandLine.cpp you'll see some
>>> code that basically amounts to this:
>>> 
>>> header:
>>> 
>>>    template<class DataType>
>>>    class basic_parser {
>>>      virtual ~basic_parser() {}
>>>    };
>>> 
>>>    __extension__ extern template class basic_parser<bool>;
>>> 
>>> implementation:
>>> 
>>>    template class basic_parser<bool>;
>>> 
>>> (both lines are wrapped in a macro (Compiler.h:77-88) & are no-ops in
>>> non-GNUC compilers (where the __extension__ extern is not available))
>>> 
>>> Adding in a virtual anchor function with an out-of-line (but still
>>> template) definition (either in the header or the cpp file) does not
>>> remove the warning.
>> 
>> Right. The emitted vtable still has to be weak.
>> 
>>> So the question is - is there any way to anchor these explicit
>>> instantiations? Should the warning (& possibly even the underlying
>>> implementation/codegen) be fixed to not flag this particular case of
>>> the GNUC extension - since these vtables should be able to be anchored
>>> (with the addition of such an out of line definition - either in the
>>> header or cpp file (though in this case I don't think it should be
>>> necessary in the header - since only these explicit instantiations of
>>> basic_parser are used))? Is there a portable way to address the
>>> warning?
>> 
>> I don't think there is a portable way to deal with this. I've attached a proposal I wrote a while back to introduce a new attribute allowing us to emit strong symbols for the cases where the user tells us we can. I never implemented it, though.
> 
> Ah, right - so using a explicit template declaration + definition in
> C++11 means that any TU seeing the declaration can avoid emitting the
> template instantiation (& rely on some other TU to provide it) but it
> doesn't guarantee that some 3rd TU might never see the declaration and
> go ahead and instantiate it anyway - thus the symbols must remain
> weak.

Exactly.

> Curious - in your proposal you required that the declaration be
> annotated as well as the definition. Why is that? It seems like it
> could just be an implicit constraint "if you have a unique explicit
> definition then all uses must be after explicit declarations".

I'm being paranoid. Because the failure case when there are both strong and weak definitions of the same external entity is pretty horrible, I want to make sure that people have the explicit instantiation declaration in place when they add the attribute to the definition. It's also a bit of a reminder that this is a global property of the specialization in question ("this specialization is never implicitly instantiated, anywhere in the code"), rather than just some isolated tweak to the definition.

>>>  If not, should the warning just be silent, or have a separate
>>> group/warning for this case so the actionable warning can remain while
>>> this one can be disabled?
>> 
>> 
>> Yes, it makes sense for explicit instantiations to have a different warning group, so the non-actionable warnings can be silenced separately.
> 
> I wonder how much value the specific warning has - but I guess some
> people might want to know that they're incurring this cost even if
> there's no immediate fix (perhaps it's so important they'd consider
> making template specializations instead so they could anchor their
> vtables).

That's a reasonable point. Why warn if the fix is something that users won't be likely to do?

It's possible that people could make the fix easy on themselves, e.g, by ensuring that the destructor is the key method, since that's easiest to specialize individually. Or, if we actually added the attribute I proposed, they'd have a reasonable way forward.

It seems to me like your proposed patch is the best way forward, and I should make my proposal real at some point. 

> I've attached a proposed patch - it's fairly short & of course the
> wording (of the warning & the warning flag) are open to suggestions.


Patch looks great to me, thanks!

	- Doug