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