[PATCH] D143524: Make the -Wunused-template default.

Thu Feb 23 10:40:10 PST 2023

v.g.vassilev marked an inline comment as done.
v.g.vassilev added a comment.

In D143524#4148039 <https://reviews.llvm.org/D143524#4148039>, @philnik wrote:

> In D143524#4148024 <https://reviews.llvm.org/D143524#4148024>, @v.g.vassilev wrote:
>
>> In D143524#4148006 <https://reviews.llvm.org/D143524#4148006>, @philnik wrote:
>>
>>> The emitted warnings from the libc++ CI look like a false-positive to me. While the functions are never called, they are used in an unevaluated context. I would expect `-Wunused` warnings to only be emitted when I can just remove the code without problems, which doesn't seem to be the case here. It would probably just get turned off again by lots of people if there are too many false-positives, which I don't think is the goal here.
>>
>> From what I see is that most of the templates are marked with static which means internal linkage. Entities with internal linkage in header files are essentially different across translation units which is an ODR violation. I believe the discussion here gives more insights of how this works: https://reviews.llvm.org/D29877
>>
>> Indeed the warning is noisy but it will potentially fix broken code which were unable to diagnose before. Especially that in some cases where static templates in header files are used as an idiom. In theory this approach can extend to cases described in https://wg21.link/p2691 where our inability to diagnose/fix these cases leads to some design decisions which may not be optimal.
>
> I missed the `static` at the beginning. That explains the warning, thanks! I agree this should be fixed. I'll look into making a patch to enable `-Wunused-template` and fix any problems. Hopefully there aren't too many.

This would be awesome as I don't have a lot of bandwidth right now and that potentially will have quite positive impact (once people understand what they need to do). I am not sure if we could somehow emit a fix-it suggestion. So far I have failed in figuring out what's a good suggestion when using this static template idiom...

Repository:
  rC Clang

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

https://reviews.llvm.org/D143524