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

Mon May 3 16:14:03 PDT 2021

dblaikie added a comment.

In D101566#2733172 <https://reviews.llvm.org/D101566#2733172>, @aaronpuchert wrote:

> In D101566#2730746 <https://reviews.llvm.org/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 <https://bugs.llvm.org/show_bug.cgi?id=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)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101566/new/

https://reviews.llvm.org/D101566