Re: [PATCH] attribs: Implement -Wno-attributes=vendor::attr [PR101940]

[Marek Polacek <polacek@redhat.com>]

On Mon, Sep 20, 2021 at 07:38:59PM +0200, Jakub Jelinek wrote:
> On Mon, Sep 20, 2021 at 01:06:58PM -0400, Marek Polacek via Gcc-patches wrote:
> 
> Not a review, just a few nits:
> 
> I think it would be useful to clarify that -Wno-attributes=list doesn't
> actually imply -Wno-attributes

Agreed, fixed with:

+Note that @option{-Wno-attributes=} does not imply @option{-Wno-attributes}.
 
> > --- a/gcc/common.opt
> > +++ b/gcc/common.opt
> > @@ -88,6 +88,9 @@ void *flag_instrument_functions_exclude_functions
> >  Variable
> >  void *flag_instrument_functions_exclude_files
> >  
> > +Variable
> > +void *flag_ignored_attributes
> > +
> >  ; Generic structs (e.g. templates not explicitly specialized)
> >  ; may not have a compilation unit associated with them, and so
> >  ; may need to be treated differently from ordinary structs.
> > @@ -546,6 +549,10 @@ Wattributes
> >  Common Var(warn_attributes) Init(1) Warning
> >  Warn about inappropriate attribute usage.
> >  
> > +Wattributes=
> > +Common Joined
> > +Do not warn about specified attributes.
> > +
> 
> So, wouldn't be this better specified as
> Wno-attributes=
> Common Joined RejectNegative
> (not sure if RejectNegative is actually needed for an option
> starting with Wno- )?

Looks like RejectNegative is not needed.  I could do that, but I think
it regresses the diagnostic:

error: unrecognized command-line option '-Wattributes=attr::'
vs
error: arguments ignored for ‘-Wattributes=’; use ‘-Wno-attributes=’ instead

I prefer the latter.  (I've changed the warning into error.)
 
> > +/* { dg-additional-options "-std=c++11" { target c++ } } */
> > +/* { dg-additional-options "-Wno-attributes=company::,yoyodyne::attr" } */
> > +/* { dg-additional-options "-Wno-attributes=c1::attr,c1::attr,c1::__attr__" } */
> > +/* { dg-additional-options "-Wno-attributes=clang" } */
> > +/* { dg-additional-options "-Wno-attributes=c2::,c2::attr" } */
> > +/* { dg-additional-options "-Wno-attributes=c3::attr,c3::" } */
> > +/* { dg-additional-options "-Wno-attributes=x::," } */
> 
> Should the above be accepted (I mean trailing , ?)  What does that mean?
 
I'm thinking it should: the arguments to -Wno-attributes= could be
generated by a tool so accepting the trailing , might help, like in
enums etc.

> > +/* { dg-additional-options "-Wno-attributes=yoyodyne::attr_new" } */
> > +/* { dg-additional-options "-Wno-attributes=c4::__attr__" } */
> 
> When writing __attr__, does that imply we won't warn about both
> c4::attr and c4::__attr__ (and __c4__::attr and __c4__::__attr__) like
> it would when writing -Wno-attributes=c4::attr ?

"__attr__" and "attr" ought to be interchangeable.  __c4__:: and c4:: currently
are not, but I guess they should.  I'll post a v2 with that fixed.

Thanks!
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA