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

[Louis Dionne via Phabricator via cfe-commits]

ldionne removed reviewers: ldionne, libc++.
ldionne added a comment.
Herald added a subscriber: wangpc.

[Github PR transition cleanup]

Dropping libc++ since this shouldn't affect libc++ since D144667 
.


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev planned changes to this revision.
v.g.vassilev added inline comments.



Comment at: clang/test/SemaCXX/warn-func-not-needed.cpp:13
 namespace test1_template {
-template  static void f() {}
+template  static void f() {} // expected-warning {{unused function 
template}}
 template <> void f() {} // expected-warning {{function 'f' is not 
needed and will not be emitted}}

ldionne wrote:
> aaron.ballman wrote:
> > v.g.vassilev wrote:
> > > aaron.ballman wrote:
> > > > v.g.vassilev wrote:
> > > > > aaron.ballman wrote:
> > > > > > Why is this unused? `f()` 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.
> Don't y'all find it weird to have to use `[[maybe_unused]]` on something that 
> is only a declaration like those CTAD guides? And I agree with @aaron.ballman 
> here: we provide headers that are used in various TUs, and we obviously never 
> expect that the entirety of our headers is going to be used by every single 
> TU.
> 
> In other words, we totally expect that those deduction guides will be unused 
> in some cases, since it's entirely fine for a user not to use them but for us 
> to still provide them. If I understood this correctly, this seems like a flaw 
> in the warning that we should fix in Clang.
Yes, I agree I am pretty sure we can fix the CTAD guides. I just need a few 
spare cycles...


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via Phabricator via cfe-commits]

ldionne added inline comments.



Comment at: clang/test/SemaCXX/warn-func-not-needed.cpp:13
 namespace test1_template {
-template  static void f() {}
+template  static void f() {} // expected-warning {{unused function 
template}}
 template <> void f() {} // expected-warning {{function 'f' is not 
needed and will not be emitted}}

aaron.ballman wrote:
> v.g.vassilev wrote:
> > aaron.ballman wrote:
> > > v.g.vassilev wrote:
> > > > aaron.ballman wrote:
> > > > > Why is this unused? `f()` 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.
Don't y'all find it weird to have to use `[[maybe_unused]]` on something that 
is only a declaration like those CTAD guides? And I agree with @aaron.ballman 
here: we provide headers that are used in various TUs, and we obviously never 
expect that the entirety of our headers is going to be used by every single TU.

In other words, we totally expect that those deduction guides will be unused in 
some cases, since it's entirely fine for a user not to use them but for us to 
still provide them. If I understood this correctly, this seems like a flaw in 
the warning that we should fix in Clang.


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D143524#4150289 , @v.g.vassilev 
wrote:

> In D143524#4150286 , @aaron.ballman 
> wrote:
>
>> In 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  static void f() {}
+template  static void f() {} // expected-warning {{unused function 
template}}
 template <> void f() {} // expected-warning {{function 'f' 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()` 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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D143524#4150286 , @aaron.ballman 
wrote:

> In 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?




Comment at: clang/test/SemaCXX/warn-func-not-needed.cpp:13
 namespace test1_template {
-template  static void f() {}
+template  static void f() {} // expected-warning {{unused function 
template}}
 template <> void f() {} // expected-warning {{function 'f' is not 
needed and will not be emitted}}

aaron.ballman wrote:
> v.g.vassilev wrote:
> > aaron.ballman wrote:
> > > Why is this unused? `f()` 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


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In 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.




Comment at: clang/test/SemaCXX/warn-func-not-needed.cpp:13
 namespace test1_template {
-template  static void f() {}
+template  static void f() {} // expected-warning {{unused function 
template}}
 template <> void f() {} // expected-warning {{function 'f' is not 
needed and will not be emitted}}

v.g.vassilev wrote:
> aaron.ballman wrote:
> > Why is this unused? `f()` 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).


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D143524#4148344 , @v.g.vassilev 
wrote:

> In D143524#4148271 , @philnik wrote:
>
>> It looks like this warning is incompatible with `-Wctad-maybe-unsupported`. 
>> It warns that the deduction guide is unused, but the deduction guide is 
>> required suppress `-Wctad-maybe-unsupported`. https://godbolt.org/z/G8bMjYsbn
>
> That should not be too hard to fix. It seems that just need to ignore 
> `CXXDeductionGuideDecl` in `Sema::ShouldRemoveFromUnused`. Is that blocking 
> you?

No, I just added a `[[maybe_unused]]`. D144667 
 should fix any libc++ problems.


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D143524#4148271 , @philnik wrote:

> It looks like this warning is incompatible with `-Wctad-maybe-unsupported`. 
> It warns that the deduction guide is unused, but the deduction guide is 
> required suppress `-Wctad-maybe-unsupported`. https://godbolt.org/z/G8bMjYsbn

That should not be too hard to fix. It seems that just need to ignore 
`CXXDeductionGuideDecl` in `Sema::ShouldRemoveFromUnused`. Is that blocking you?


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

It looks like this warning is incompatible with `-Wctad-maybe-unsupported`. It 
warns that the deduction guide is unused, but the deduction guide is required 
suppress `-Wctad-maybe-unsupported`. https://godbolt.org/z/G8bMjYsbn


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vassil Vassilev via Phabricator via cfe-commits]

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

In D143524#4148039 , @philnik wrote:

> In D143524#4148024 , @v.g.vassilev 
> wrote:
>
>> In 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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D143524#4148024 , @v.g.vassilev 
wrote:

> In 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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vassil Vassilev via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaCXX/warn-func-not-needed.cpp:13
 namespace test1_template {
-template  static void f() {}
+template  static void f() {} // expected-warning {{unused function 
template}}
 template <> void f() {} // expected-warning {{function 'f' is not 
needed and will not be emitted}}

aaron.ballman wrote:
> Why is this unused? `f()` in `foo()` should cause this to be used, 
> right?
> 
> How should a user silence this diagnostic without disabling it entirely?
Nobody uses `foo`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In 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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: libc++.
aaron.ballman added a comment.

Precommit CI shows issues with the libc++ tests that need to be addressed. This 
should also come with a release note.




Comment at: clang/test/SemaCXX/warn-func-not-needed.cpp:13
 namespace test1_template {
-template  static void f() {}
+template  static void f() {} // expected-warning {{unused function 
template}}
 template <> void f() {} // expected-warning {{function 'f' is not 
needed and will not be emitted}}

Why is this unused? `f()` in `foo()` should cause this to be used, right?

How should a user silence this diagnostic without disabling it entirely?


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev created this revision.
v.g.vassilev added reviewers: aaron.ballman, ldionne.
Herald added a project: All.
v.g.vassilev requested review of this revision.

https://reviews.llvm.org/D29877 implements a useful -Wunused-template 
diagnostic detects unused internal linkage templates. This helps finding 
potential ODR issues in headers.

Many years ago we made the diagnostic optional because libcxx was not ready. I 
am hoping @ldionne to help with that if we have not cleaned up libcxx already.


Repository:
  rC Clang

https://reviews.llvm.org/D143524

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/test/Misc/warning-wall.c
  clang/test/SemaCXX/warn-func-not-needed.cpp
  clang/test/SemaCXX/warn-variable-not-needed.cpp


Index: clang/test/SemaCXX/warn-variable-not-needed.cpp
===
--- clang/test/SemaCXX/warn-variable-not-needed.cpp
+++ clang/test/SemaCXX/warn-variable-not-needed.cpp
@@ -4,7 +4,7 @@
   static int abc = 42; // expected-warning {{variable 'abc' is not needed and 
will not be emitted}}
 
   namespace {
-  template  int abc_template = 0;
+  template  int abc_template = 0; //expected-warning {{unused 
variable template}}
   template <> int abc_template = 0; // expected-warning {{variable 
'abc_template' is not needed and will not be emitted}}
   }  // namespace
   template 
Index: clang/test/SemaCXX/warn-func-not-needed.cpp
===
--- clang/test/SemaCXX/warn-func-not-needed.cpp
+++ clang/test/SemaCXX/warn-func-not-needed.cpp
@@ -10,7 +10,7 @@
 }
 
 namespace test1_template {
-template  static void f() {}
+template  static void f() {} // expected-warning {{unused function 
template}}
 template <> void f() {} // expected-warning {{function 'f' is not 
needed and will not be emitted}}
 template 
 void foo() {
Index: clang/test/Misc/warning-wall.c
===
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -66,6 +66,8 @@
 CHECK-NEXT:  -Wunused-function
 CHECK-NEXT:-Wunneeded-internal-declaration
 CHECK-NEXT:  -Wunused-label
+CHECK-NEXT:  -Wunused-template
+CHECK-NEXT:-Wunneeded-internal-declaration
 CHECK-NEXT:  -Wunused-private-field
 CHECK-NEXT:  -Wunused-lambda-capture
 CHECK-NEXT:  -Wunused-local-typedef
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -929,7 +929,7 @@
 def Unused : DiagGroup<"unused",
[UnusedArgument, UnusedFunction, UnusedLabel,
 // UnusedParameter, (matches GCC's behavior)
-// UnusedTemplate, (clean-up libc++ before enabling)
+UnusedTemplate,
 // UnusedMemberFunction, (clean-up llvm before 
enabling)
 UnusedPrivateField, UnusedLambdaCapture,
 UnusedLocalTypedef, UnusedValue, UnusedVariable,


Index: clang/test/SemaCXX/warn-variable-not-needed.cpp
===
--- clang/test/SemaCXX/warn-variable-not-needed.cpp
+++ clang/test/SemaCXX/warn-variable-not-needed.cpp
@@ -4,7 +4,7 @@
   static int abc = 42; // expected-warning {{variable 'abc' is not needed and will not be emitted}}
 
   namespace {
-  template  int abc_template = 0;
+  template  int abc_template = 0; //expected-warning {{unused variable template}}
   template <> int abc_template = 0; // expected-warning {{variable 'abc_template' is not needed and will not be emitted}}
   }  // namespace
   template 
Index: clang/test/SemaCXX/warn-func-not-needed.cpp
===
--- clang/test/SemaCXX/warn-func-not-needed.cpp
+++ clang/test/SemaCXX/warn-func-not-needed.cpp
@@ -10,7 +10,7 @@
 }
 
 namespace test1_template {
-template  static void f() {}
+template  static void f() {} // expected-warning {{unused function template}}
 template <> void f() {} // expected-warning {{function 'f' is not needed and will not be emitted}}
 template 
 void foo() {
Index: clang/test/Misc/warning-wall.c
===
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -66,6 +66,8 @@
 CHECK-NEXT:  -Wunused-function
 CHECK-NEXT:-Wunneeded-internal-declaration
 CHECK-NEXT:  -Wunused-label
+CHECK-NEXT:  -Wunused-template
+CHECK-NEXT:-Wunneeded-internal-declaration
 CHECK-NEXT:  -Wunused-private-field
 CHECK-NEXT:  -Wunused-lambda-capture
 CHECK-NEXT:  -Wunused-local-typedef
Index: clang/include/clang/Basic/DiagnosticGroups.td