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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D143524#4150289 <https://reviews.llvm.org/D143524#4150289>, @v.g.vassilev wrote:

> In D143524#4150286 <https://reviews.llvm.org/D143524#4150286>, @aaron.ballman wrote:
>
>> In D143524#4148024 <https://reviews.llvm.org/D143524#4148024>, @v.g.vassilev wrote:
>>
>>> 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.
>>
>> We need to ensure the diagnostic is not so noisy that people disable it, though. That means a low false positive rate and a straight-forward way to silence the diagnostic on a case-by-case basis.
>
> I agree, so far I have not seen false positives. All issues (except for `-Wctad-maybe-unsupported` as mentioned above) seemed real. Do you have a particular example in mind?

The concrete example I have is the one under discussion from the test cases, but I was also speaking about the libc++ work that's going on as well. In general, it's best to try the diagnostic on some large-scale C++ projects to see what kind of results you get (compiling projects like LLVM, Qt, Kokkos, Chromium, etc). That usually helps spot other situations like `-Wctad-maybe-unused` etc.



================
Comment at: clang/test/SemaCXX/warn-func-not-needed.cpp:13
 namespace test1_template {
-template <typename T> static void f() {}
+template <typename T> static void f() {} // expected-warning {{unused function template}}
 template <> void f<int>() {} // expected-warning {{function 'f<int>' is not needed and will not be emitted}}
----------------
v.g.vassilev wrote:
> aaron.ballman wrote:
> > v.g.vassilev wrote:
> > > aaron.ballman wrote:
> > > > Why is this unused? `f<long>()` in `foo()` should cause this to be used, right?
> > > > 
> > > > How should a user silence this diagnostic without disabling it entirely?
> > > Nobody uses `foo`.
> > Ah, good point on `foo` not being used, but the question still stands -- how does the user silence this diagnostic? It's not at all uncommon to have a primary template with specializations where the TU only uses either the primary or a specialization, but not both (and certainly not all specializations).
> @philnik used `[[maybe_unused]]` which seemed reasonable to me for silencing the diagnostic. Maybe take a look at the changes done here: https://reviews.llvm.org/D144667
That's reasonable if the interface is one the user controls, such as one within a .cpp file. But the situation I'm worried about is where the primary template and specializations live in a header file that's shared between multiple TUs. I don't think it's reasonable to expect users to put `[[maybe_unused]]` on the primary template and all specializations in that situation.


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524