aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM!
================
Comment at: clang/test/SemaCXX/warn-unused-private-field.cpp:291-292
+enum [[maybe_unused]] MaybeUnusedEnum {};
+typedef int MaybeUnusedTypedef [[maybe_unused]];
+class C {
+ MaybeUnusedClass c; // no-warning
----------------
hans wrote:
> aaron.ballman wrote:
> > hans wrote:
> > > aaron.ballman wrote:
> > > > Let's try one more thing:
> > > > ```
> > > > template <typename Ty, typename Uy>
> > > > class C2 {
> > > > Ty c; // no-warning
> > > > Uy d; // warning
> > > > };
> > > >
> > > > class [[maybe_unused]] Good {};
> > > > class Bad {};
> > > >
> > > > C2<Good, Bad> c;
> > > > ```
> > > The `!FD->getParent()->isDependentContext()` condition prevents the
> > > warning from firing on either `C2<>::c` or `d`. (It seems that was there
> > > from the beginning:
> > > https://github.com/llvm/llvm-project/commit/0baec549a3f49d8c04a0092917f758cc89ef238d#diff-9ced4fa47ee2b9c03b6996ce89a1d131c0f5b71013993bc582209f50d5e934daR1649)
> > Hmmm. I think that's intended to ensure we don't diagnose before we've
> > instantiated the template, but once we instantiate with `C2<Good, Bad> c;`,
> > the field decl is no longer in a dependent context and we should still
> > diagnose when instantiating, right?
> I tried, and it's not diagnosing: https://godbolt.org/z/osnWbofY5
>
> I'm guessing the motivation was that we shouldn't warn about things that
> depend on how the template is instantiated, since that could make it hard to
> write a warning-free template definition.
I don't think this is your bug to fix, but I still think this is a bug.
`[[maybe_unused]]` is precisely how you would get warning-free template
definitions -- you'd put the attribute on the field itself instead of on the
type passed in as a template argument.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D159083/new/
https://reviews.llvm.org/D159083
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
