dblaikie added a comment. In D101566#2730555 <https://reviews.llvm.org/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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101566/new/ https://reviews.llvm.org/D101566 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
