Message ID | YYnR4UeOEix+D/OM@redhat.com |
---|---|
State | New |
Headers | show |
Series | [v6] attribs: Implement -Wno-attributes=vendor::attr [PR101940] | expand |
On 11/8/21 20:41, Marek Polacek wrote: > On Sat, Nov 06, 2021 at 03:29:57PM -0400, Jason Merrill wrote: >> On 11/6/21 14:28, Marek Polacek wrote: >>> On Sat, Nov 06, 2021 at 02:32:26AM +0100, Bernhard Reutner-Fischer wrote: >>>> On 6 November 2021 01:21:43 CET, Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: >>>> >>>>> >>>>> Thanks, so like this? I'm including an incremental diff so that it's >>>>> clear what changed: >>>>> >>>>> diff --git a/gcc/attribs.c b/gcc/attribs.c >>>>> index d5fba7f4bbb..addfe6f6c80 100644 >>>>> --- a/gcc/attribs.c >>>>> +++ b/gcc/attribs.c >>>>> @@ -237,7 +237,7 @@ check_attribute_tables (void) >>>>> the end of parsing of all TUs. */ >>>>> static vec<attribute_spec *> ignored_attributes_table; >>>>> >>>>> -/* Parse arguments ARGS of -Wno-attributes=. >>>>> +/* Parse arguments V of -Wno-attributes=. >>>>> Currently we accept: >>>>> vendor::attr >>>>> vendor:: >>>>> @@ -252,12 +252,15 @@ handle_ignored_attributes_option (vec<char *> *v) >>>>> >>>>> for (auto opt : v) >>>>> { >>>>> + /* We're going to be modifying the string. */ >>>>> + opt = xstrdup (opt); >>>>> char *q = strstr (opt, "::"); >>>>> /* We don't accept '::attr'. */ >>>>> if (q == nullptr || q == opt) >>>>> { >>>>> error ("wrong argument to ignored attributes"); >>>>> inform (input_location, "valid format is %<ns::attr%> or %<ns::%>"); >>>>> + free (opt); >>>>> continue; >>>>> } >>>> >>>> Only xstrdup here, after the strstr check? >>>> Should maybe strdup the rest here, not full opt.. >>> >>> No, I want q to point into the copy of the string, since I'm about >>> to modify it. And I'd prefer a single call to xstrdup rather than >>> two. >> >> It occurs to me that instead of calling xstrdup at all, since you're already >> passing the strings to get_identifier you could use >> get_identifier_with_length instead, and then refer to IDENTIFIER_POINTER of >> the result. > > Ah, clever. I didn't think it would work because I didn't expect that > get_identifier_with_length works when it gets a string that isn't > nul-terminated but it does. So how about the following? > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > -- >8 -- > It is desirable for -Wattributes to warn about e.g. > > [[deprecate]] void g(); // typo, should warn > > However, -Wattributes also warns about vendor-specific attributes > (that's because lookup_scoped_attribute_spec -> find_attribute_namespace > finds nothing), which, with -Werror, causes grief. We don't want the > -Wattributes warning for > > [[company::attr]] void f(); > > GCC warns because it doesn't know the "company" namespace; it only knows > the "gnu" and "omp" namespaces. We could entirely disable warning about > attributes in unknown scopes but then the compiler would also miss typos > like > > [[company::attrx]] void f(); > > or > > [[gmu::warn_used_result]] int write(); > > so that is not a viable solution. A workaround is to use a #pragma: > > #pragma GCC diagnostic push > #pragma GCC diagnostic ignored "-Wattributes" > [[company::attr]] void f() {} > #pragma GCC diagnostic pop > > but that's a mouthful and awkward to use and could also hide typos. In > fact, any macro-based solution doesn't seem like a way forward. > > This patch implements -Wno-attributes=, which takes these arguments: > > company::attr > company:: > > This option should go well with using @file: the user could have a file > containing > -Wno-attributes=vendor::attr1,vendor::attr2 > and then invoke gcc with '@attrs' or similar. > > I've also added a new pragma which has the same effect: > > The pragma along with the new option should help with various static > analysis tools. > > PR c++/101940 > > gcc/ChangeLog: > > * attribs.c (struct scoped_attributes): Add a bool member. > (lookup_scoped_attribute_spec): Forward declare. > (register_scoped_attributes): New bool parameter, defaulted to > false. Use it. > (handle_ignored_attributes_option): New function. > (free_attr_data): New function. > (init_attributes): Call handle_ignored_attributes_option. > (attr_namespace_ignored_p): New function. > (decl_attributes): Check attr_namespace_ignored_p before > warning. > * attribs.h (free_attr_data): Declare. > (register_scoped_attributes): Adjust declaration. > (handle_ignored_attributes_option): Declare. > * common.opt (Wattributes=): New option with a variable. > * doc/extend.texi: Document #pragma GCC diagnostic ignored_attributes. > * doc/invoke.texi: Document -Wno-attributes=. > * opts.c (common_handle_option) <case OPT_Wattributes_>: Handle. > * plugin.h (register_scoped_attributes): Adjust declaration. > * toplev.c (compile_file): Call free_attr_data. > > gcc/c-family/ChangeLog: > > * c-pragma.c (handle_pragma_diagnostic): Handle #pragma GCC diagnostic > ignored_attributes. > > gcc/testsuite/ChangeLog: > > * c-c++-common/Wno-attributes-1.c: New test. > * c-c++-common/Wno-attributes-2.c: New test. > --- > gcc/attribs.c | 123 +++++++++++++++++- > gcc/attribs.h | 5 +- > gcc/c-family/c-pragma.c | 37 +++++- > gcc/common.opt | 9 +- > gcc/doc/extend.texi | 19 +++ > gcc/doc/invoke.texi | 20 +++ > gcc/opts.c | 20 +++ > gcc/plugin.h | 4 +- > gcc/testsuite/c-c++-common/Wno-attributes-1.c | 55 ++++++++ > gcc/testsuite/c-c++-common/Wno-attributes-2.c | 56 ++++++++ > gcc/toplev.c | 2 + > 11 files changed, 341 insertions(+), 9 deletions(-) > create mode 100644 gcc/testsuite/c-c++-common/Wno-attributes-1.c > create mode 100644 gcc/testsuite/c-c++-common/Wno-attributes-2.c > > diff --git a/gcc/attribs.c b/gcc/attribs.c > index 83fafc98b7d..23d92ca9474 100644 > --- a/gcc/attribs.c > +++ b/gcc/attribs.c > @@ -87,6 +87,8 @@ struct scoped_attributes > const char *ns; > vec<attribute_spec> attributes; > hash_table<attribute_hasher> *attribute_hash; > + /* True if we should not warn about unknown attributes in this NS. */ > + bool ignored_p; > }; > > /* The table of scope attributes. */ > @@ -95,6 +97,8 @@ static vec<scoped_attributes> attributes_table; > static scoped_attributes* find_attribute_namespace (const char*); > static void register_scoped_attribute (const struct attribute_spec *, > scoped_attributes *); > +static const struct attribute_spec *lookup_scoped_attribute_spec (const_tree, > + const_tree); > > static bool attributes_initialized = false; > > @@ -121,12 +125,14 @@ extract_attribute_substring (struct substring *str) > > /* Insert an array of attributes ATTRIBUTES into a namespace. This > array must be NULL terminated. NS is the name of attribute > - namespace. The function returns the namespace into which the > - attributes have been registered. */ > + namespace. IGNORED_P is true iff all unknown attributes in this > + namespace should be ignored for the purposes of -Wattributes. The > + function returns the namespace into which the attributes have been > + registered. */ > > scoped_attributes * > register_scoped_attributes (const struct attribute_spec *attributes, > - const char *ns) > + const char *ns, bool ignored_p /*=false*/) > { > scoped_attributes *result = NULL; > > @@ -144,9 +150,12 @@ register_scoped_attributes (const struct attribute_spec *attributes, > memset (&sa, 0, sizeof (sa)); > sa.ns = ns; > sa.attributes.create (64); > + sa.ignored_p = ignored_p; > result = attributes_table.safe_push (sa); > result->attribute_hash = new hash_table<attribute_hasher> (200); > } > + else > + result->ignored_p |= ignored_p; > > /* Really add the attributes to their namespace now. */ > for (unsigned i = 0; attributes[i].name != NULL; ++i) > @@ -224,6 +233,95 @@ check_attribute_tables (void) > attribute_tables[j][l].name)); > } > > +/* Used to stash pointers to allocated memory so that we can free them at > + the end of parsing of all TUs. */ > +static vec<attribute_spec *> ignored_attributes_table; > + > +/* Parse arguments V of -Wno-attributes=. > + Currently we accept: > + vendor::attr > + vendor:: > + This functions also registers the parsed attributes so that we don't > + warn that we don't recognize them. */ > + > +void > +handle_ignored_attributes_option (vec<char *> *v) > +{ > + if (v == nullptr) > + return; > + > + for (auto opt : v) > + { > + char *cln = strstr (opt, "::"); > + /* We don't accept '::attr'. */ > + if (cln == nullptr || cln == opt) > + { > + error ("wrong argument to ignored attributes"); > + inform (input_location, "valid format is %<ns::attr%> or %<ns::%>"); > + continue; > + } > + char *vendor_start = opt; > + ptrdiff_t vendor_len = cln - opt; > + char *attr_start = cln + 2; > + /* This could really use rawmemchr :(. */ > + ptrdiff_t attr_len = strchr (attr_start, '\0') - attr_start; > + /* Verify that they look valid. */ > + auto valid_p = [](const char *const s, ptrdiff_t len) { > + for (int i = 0; i < len; ++i) > + if (!ISALNUM (*s) && *s != '_') > + return false; > + return true; > + }; > + if (!valid_p (vendor_start, vendor_len) > + || !valid_p (attr_start, attr_len)) > + { > + error ("wrong argument to ignored attributes"); > + continue; > + } > + /* Turn "__attr__" into "attr" so that we have a canonical form of > + attribute names. Likewise for vendor. */ > + auto strip = [](char *&s, ptrdiff_t &l) { > + if (l > 4 && s[0] == '_' && s[1] == '_' > + && s[l - 1] == '_' && s[l - 2] == '_') > + { > + s += 2; > + l -= 4; > + } > + }; > + strip (attr_start, attr_len); > + strip (vendor_start, vendor_len); > + /* We perform all this hijinks so that we don't have to copy OPT. */ > + tree vendor_id = get_identifier_with_length (vendor_start, vendor_len); > + tree attr_id = get_identifier_with_length (attr_start, attr_len); > + /* If we've already seen this vendor::attr, ignore it. Attempting to > + register it twice would lead to a crash. */ > + if (lookup_scoped_attribute_spec (vendor_id, attr_id)) > + continue; Hmm, this looks like it isn't handling the case of previously ignoring vendor::; it seems we'll look for vendor::<empty string> instead, not find it, and register again. > + /* In the "vendor::" case, we should ignore *any* attribute coming > + from this attribute namespace. */ > + const char *vendor = IDENTIFIER_POINTER (vendor_id); > + const char *attr = attr_len == 0 ? nullptr : IDENTIFIER_POINTER (attr_id); > + /* Create a table with extra attributes which we will register. > + We can't free it here, so squirrel away the pointers. */ > + attribute_spec *table = new attribute_spec[2]; > + ignored_attributes_table.safe_push (table); > + table[0] = { attr, 0, 0, false, false, false, false, nullptr, nullptr }; > + table[1] = { nullptr, 0, 0, false, false, false, false, nullptr, > + nullptr }; > + register_scoped_attributes (table, vendor, !attr); > + } > +} > + > +/* Free data we might have allocated when adding extra attributes. */ > + > +void > +free_attr_data () > +{ > + for (auto x : ignored_attributes_table) > + delete[] x; > + ignored_attributes_table.release (); > +} > + > /* Initialize attribute tables, and make some sanity checks if checking is > enabled. */ > > @@ -252,6 +350,9 @@ init_attributes (void) > /* Put all the GNU attributes into the "gnu" namespace. */ > register_scoped_attributes (attribute_tables[i], "gnu"); > > + vec<char *> *ignored = (vec<char *> *) flag_ignored_attributes; > + handle_ignored_attributes_option (ignored); > + > invoke_plugin_callbacks (PLUGIN_ATTRIBUTES, NULL); > attributes_initialized = true; > } > @@ -456,6 +557,19 @@ diag_attr_exclusions (tree last_decl, tree node, tree attrname, > return found; > } > > +/* Return true iff we should not complain about unknown attributes > + coming from the attribute namespace NS. This is the case for > + the -Wno-attributes=ns:: command-line option. */ > + > +static bool > +attr_namespace_ignored_p (tree ns) > +{ > + if (ns == NULL_TREE) > + return false; > + scoped_attributes *r = find_attribute_namespace (IDENTIFIER_POINTER (ns)); > + return r && r->ignored_p; > +} > + > /* Process the attributes listed in ATTRIBUTES and install them in *NODE, > which is either a DECL (including a TYPE_DECL) or a TYPE. If a DECL, > it should be modified in place; if a TYPE, a copy should be created > @@ -556,7 +670,8 @@ decl_attributes (tree *node, tree attributes, int flags, > > if (spec == NULL) > { > - if (!(flags & (int) ATTR_FLAG_BUILT_IN)) > + if (!(flags & (int) ATTR_FLAG_BUILT_IN) > + && !attr_namespace_ignored_p (ns)) > { > if (ns == NULL_TREE || !cxx11_attr_p) > warning (OPT_Wattributes, "%qE attribute directive ignored", > diff --git a/gcc/attribs.h b/gcc/attribs.h > index 138c509bce1..96a527f67a9 100644 > --- a/gcc/attribs.h > +++ b/gcc/attribs.h > @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see > #define GCC_ATTRIBS_H > > extern const struct attribute_spec *lookup_attribute_spec (const_tree); > +extern void free_attr_data (); > extern void init_attributes (void); > > /* Process the attributes listed in ATTRIBUTES and install them in *NODE, > @@ -40,12 +41,14 @@ extern void apply_tm_attr (tree, tree); > extern tree make_attribute (const char *, const char *, tree); > > extern struct scoped_attributes* register_scoped_attributes (const struct attribute_spec *, > - const char *); > + const char *, > + bool = false); > > extern char *sorted_attr_string (tree); > extern bool common_function_versions (tree, tree); > extern tree make_dispatcher_decl (const tree); > extern bool is_function_default_version (const tree); > +extern void handle_ignored_attributes_option (vec<char *> *); > > /* Return a type like TTYPE except that its TYPE_ATTRIBUTES > is ATTRIBUTE. > diff --git a/gcc/c-family/c-pragma.c b/gcc/c-family/c-pragma.c > index a9be8df0384..a299ceff6a2 100644 > --- a/gcc/c-family/c-pragma.c > +++ b/gcc/c-family/c-pragma.c > @@ -764,7 +764,7 @@ handle_pragma_diagnostic(cpp_reader *ARG_UNUSED(dummy)) > if (token != CPP_NAME) > { > warning_at (loc, OPT_Wpragmas, > - "missing [error|warning|ignored|push|pop]" > + "missing [error|warning|ignored|push|pop|ignored_attributes]" > " after %<#pragma GCC diagnostic%>"); > return; > } > @@ -787,10 +787,43 @@ handle_pragma_diagnostic(cpp_reader *ARG_UNUSED(dummy)) > diagnostic_pop_diagnostics (global_dc, input_location); > return; > } > + else if (strcmp (kind_string, "ignored_attributes") == 0) > + { > + token = pragma_lex (&x, &loc); > + if (token != CPP_STRING) > + { > + warning_at (loc, OPT_Wpragmas, > + "missing attribute name after %<#pragma GCC diagnostic " > + "ignored_attributes%>"); > + return; > + } > + char *args = xstrdup (TREE_STRING_POINTER (x)); > + const size_t l = strlen (args); > + if (l == 0) > + { > + warning_at (loc, OPT_Wpragmas, "missing argument to %<#pragma GCC " > + "diagnostic ignored_attributes%>"); > + free (args); > + return; > + } > + else if (args[l - 1] == ',') > + { > + warning_at (loc, OPT_Wpragmas, "trailing %<,%> in arguments for " > + "%<#pragma GCC diagnostic ignored_attributes%>"); > + free (args); > + return; > + } > + auto_vec<char *> v; > + for (char *p = strtok (args, ","); p; p = strtok (NULL, ",")) > + v.safe_push (p); > + handle_ignored_attributes_option (&v); > + free (args); > + return; > + } > else > { > warning_at (loc, OPT_Wpragmas, > - "expected [error|warning|ignored|push|pop]" > + "expected [error|warning|ignored|push|pop|ignored_attributes]" > " after %<#pragma GCC diagnostic%>"); > return; > } > diff --git a/gcc/common.opt b/gcc/common.opt > index 1a5b9bfcca9..de9b848eda5 100644 > --- a/gcc/common.opt > +++ b/gcc/common.opt > @@ -83,7 +83,7 @@ int flag_gen_aux_info = 0 > Variable > int flag_shlib > > -; These two are really VEC(char_p,heap) *. > +; These three are really VEC(char_p,heap) *. > > Variable > void *flag_instrument_functions_exclude_functions > @@ -91,6 +91,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. > @@ -549,6 +552,10 @@ Wattributes > Common Var(warn_attributes) Init(1) Warning > Warn about inappropriate attribute usage. > > +Wattributes= > +Common Joined > +Do not warn about specified attributes. > + > Wattribute-alias > Common Alias(Wattribute_alias=, 1, 0) Warning > Warn about type safety and similar errors and mismatches in declarations with alias attributes. > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > index eee4c6737bb..6e6c580e329 100644 > --- a/gcc/doc/extend.texi > +++ b/gcc/doc/extend.texi > @@ -23767,6 +23767,25 @@ restored. > foo(d); /* depends on command-line options */ > @end smallexample > > +@item #pragma GCC diagnostic ignored_attributes > + > +Similarly to @option{-Wno-attributes=}, this pragma allows users to suppress > +warnings about unknown scoped attributes (in C++11 and C2X). For example, > +@code{#pragma GCC diagnostic ignored_attributes "vendor::attr"} disables > +warning about the following declaration: > + > +@smallexample > +[[vendor::attr]] void f(); > +@end smallexample > + > +whereas @code{#pragma GCC diagnostic ignored_attributes "vendor::"} prevents > +warning about both of these declarations: > + > +@smallexample > +[[vendor::safe]] void f(); > +[[vendor::unsafe]] void f2(); > +@end smallexample > + > @end table > > GCC also offers a simple mechanism for printing messages during > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index f206cff1221..339b8d7597b 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -8707,6 +8707,26 @@ unrecognized attributes, function attributes applied to variables, > etc. This does not stop errors for incorrect use of supported > attributes. > > +Additionally, using @option{-Wno-attributes=}, it is possible to suppress > +warnings about unknown scoped attributes (in C++11 and C2X). For example, > +@option{-Wno-attributes=vendor::attr} disables warning about the following > +declaration: > + > +@smallexample > +[[vendor::attr]] void f(); > +@end smallexample > + > +It is also possible to disable warning about all attributes in a namespace > +using @option{-Wno-attributes=vendor::} which prevents warning about both > +of these declarations: > + > +@smallexample > +[[vendor::safe]] void f(); > +[[vendor::unsafe]] void f2(); > +@end smallexample > + > +Note that @option{-Wno-attributes=} does not imply @option{-Wno-attributes}. > + > @item -Wno-builtin-declaration-mismatch > @opindex Wno-builtin-declaration-mismatch > @opindex Wbuiltin-declaration-mismatch > diff --git a/gcc/opts.c b/gcc/opts.c > index caed6255500..175b4635bb4 100644 > --- a/gcc/opts.c > +++ b/gcc/opts.c > @@ -2611,6 +2611,26 @@ common_handle_option (struct gcc_options *opts, > /* Currently handled in a prescan. */ > break; > > + case OPT_Wattributes_: > + if (lang_mask == CL_DRIVER) > + break; > + > + if (value) > + { > + error_at (loc, "arguments ignored for %<-Wattributes=%>; use " > + "%<-Wno-attributes=%> instead"); > + break; > + } > + else if (arg[strlen (arg) - 1] == ',') > + { > + error_at (loc, "trailing %<,%> in arguments for " > + "%<-Wno-attributes=%>"); > + break; > + } > + > + add_comma_separated_to_vector (&opts->x_flag_ignored_attributes, arg); > + break; > + > case OPT_Werror: > dc->warning_as_error_requested = value; > break; > diff --git a/gcc/plugin.h b/gcc/plugin.h > index 1640e253ca5..5556763d1bf 100644 > --- a/gcc/plugin.h > +++ b/gcc/plugin.h > @@ -197,7 +197,9 @@ invoke_plugin_callbacks (int event ATTRIBUTE_UNUSED, > /* In attribs.c. */ > > extern void register_attribute (const struct attribute_spec *attr); > +/* The default argument for the third parameter is given in attribs.h. */ > extern struct scoped_attributes* register_scoped_attributes (const struct attribute_spec *, > - const char *); > + const char *, > + bool); > > #endif /* PLUGIN_H */ > diff --git a/gcc/testsuite/c-c++-common/Wno-attributes-1.c b/gcc/testsuite/c-c++-common/Wno-attributes-1.c > new file mode 100644 > index 00000000000..aac1a69fd85 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/Wno-attributes-1.c > @@ -0,0 +1,55 @@ > +/* PR c++/101940 */ > +/* { dg-do compile } */ > +/* { 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=c2::,c2::attr" } */ > +/* { dg-additional-options "-Wno-attributes=c3::attr,c3::" } */ > +/* { dg-additional-options "-Wno-attributes=x::" } */ > +/* { dg-additional-options "-Wno-attributes=yoyodyne::attr_new" } */ > +/* { dg-additional-options "-Wno-attributes=c4::__attr__" } */ > +/* { dg-additional-options "-Wno-attributes=c5::attr" } */ > +/* { dg-additional-options "-Wno-attributes=__c6__::attr" } */ > + > +[[company::attr]] void f1(); > +[[company::attr2]] void f2(); > + > +[[yoyodyne::attr]] void f3(); > +[[yoyodyne::__attr__]] void f3__(); > +[[yoyodyne::attrx]] void f4(); /* { dg-warning "ignored" } */ > +[[yoyodyne::__attrx__]] void f4__(); /* { dg-warning "ignored" } */ > + > +[[c1::attr]] void f5(); > + > +[[c2::attr]] void f6(); > +[[c2::attrx]] void f7(); > +[[c2::__attr__]] void f6__(); > +[[c2::__attrx__]] void f7__(); > + > +[[c3::attr]] void f8(); > +[[c3::attrx]] void f9(); > + > +[[x::x]] void f10(); > + > +[[yoyodyne::attr_new]] void f11(); > +[[yoyodyne::__attr_new__]] void f11__(); > +[[yoyodyne::attr_mew]] void f12(); /* { dg-warning "ignored" } */ > +[[yoyodyne::__attr_mew__]] void f12__(); /* { dg-warning "ignored" } */ > + > +[[c4::attr]] void f13(); > +[[c4::__attr__]] void f13__(); > +[[c4::attrx]] void f14(); /* { dg-warning "ignored" } */ > + > +[[c5::attr]] void f15(); > +[[c5::__attr__]] void f15__(); > +[[__c5__::attr]] void __f15(); > +[[__c5__::__attr__]] void __f15__(); > +[[c5::attrx]] void f15x(); /* { dg-warning "ignored" } */ > +[[__c5__::attrx]] void f15x(); /* { dg-warning "ignored" } */ > + > +[[c6::attr]] void f16(); > +[[c6::__attr__]] void f16__(); > +[[__c6__::attr]] void __f16(); > +[[__c6__::__attr__]] void __f16__(); > +[[c6::attrx]] void f16x(); /* { dg-warning "ignored" } */ > +[[__c6__::attrx]] void f16x(); /* { dg-warning "ignored" } */ > diff --git a/gcc/testsuite/c-c++-common/Wno-attributes-2.c b/gcc/testsuite/c-c++-common/Wno-attributes-2.c > new file mode 100644 > index 00000000000..4307c74b048 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/Wno-attributes-2.c > @@ -0,0 +1,56 @@ > +/* PR c++/101940 */ > +/* { dg-do compile } */ > +/* { dg-additional-options "-std=c++11" { target c++ } } */ > + > +#pragma GCC diagnostic ignored_attributes "company::,yoyodyne::attr" > +#pragma GCC diagnostic ignored_attributes "c1::attr,c1::attr,c1::__attr__" > +#pragma GCC diagnostic ignored_attributes "c2::,c2::attr" > +#pragma GCC diagnostic ignored_attributes "c3::attr,c3::" > +#pragma GCC diagnostic ignored_attributes "x::" > +#pragma GCC diagnostic ignored_attributes "yoyodyne::attr_new" > +#pragma GCC diagnostic ignored_attributes "c4::__attr__" > +#pragma GCC diagnostic ignored_attributes "c5::attr" > +#pragma GCC diagnostic ignored_attributes "__c6__::attr" > + > +[[company::attr]] void f1(); > +[[company::attr2]] void f2(); > + > +[[yoyodyne::attr]] void f3(); > +[[yoyodyne::__attr__]] void f3__(); > +[[yoyodyne::attrx]] void f4(); /* { dg-warning "ignored" } */ > +[[yoyodyne::__attrx__]] void f4__(); /* { dg-warning "ignored" } */ > + > +[[c1::attr]] void f5(); > + > +[[c2::attr]] void f6(); > +[[c2::attrx]] void f7(); > +[[c2::__attr__]] void f6__(); > +[[c2::__attrx__]] void f7__(); > + > +[[c3::attr]] void f8(); > +[[c3::attrx]] void f9(); > + > +[[x::x]] void f10(); > + > +[[yoyodyne::attr_new]] void f11(); > +[[yoyodyne::__attr_new__]] void f11__(); > +[[yoyodyne::attr_mew]] void f12(); /* { dg-warning "ignored" } */ > +[[yoyodyne::__attr_mew__]] void f12__(); /* { dg-warning "ignored" } */ > + > +[[c4::attr]] void f13(); > +[[c4::__attr__]] void f13__(); > +[[c4::attrx]] void f14(); /* { dg-warning "ignored" } */ > + > +[[c5::attr]] void f15(); > +[[c5::__attr__]] void f15__(); > +[[__c5__::attr]] void __f15(); > +[[__c5__::__attr__]] void __f15__(); > +[[c5::attrx]] void f15x(); /* { dg-warning "ignored" } */ > +[[__c5__::attrx]] void f15x(); /* { dg-warning "ignored" } */ > + > +[[c6::attr]] void f16(); > +[[c6::__attr__]] void f16__(); > +[[__c6__::attr]] void __f16(); > +[[__c6__::__attr__]] void __f16__(); > +[[c6::attrx]] void f16x(); /* { dg-warning "ignored" } */ > +[[__c6__::attrx]] void f16x(); /* { dg-warning "ignored" } */ > diff --git a/gcc/toplev.c b/gcc/toplev.c > index e91f083f8ff..99276bde87d 100644 > --- a/gcc/toplev.c > +++ b/gcc/toplev.c > @@ -457,6 +457,8 @@ compile_file (void) > if (flag_dump_locations) > dump_location_info (stderr); > > + free_attr_data (); > + > /* Compilation is now finished except for writing > what's left of the symbol table output. */ > > > base-commit: 206c08ce28a3c70afa4ecf9274611295d6369218 >
On Tue, 9 Nov 2021 00:12:10 -0500 Jason Merrill <jason@redhat.com> wrote: > On 11/8/21 20:41, Marek Polacek wrote: > > On Sat, Nov 06, 2021 at 03:29:57PM -0400, Jason Merrill wrote: > > + for (auto opt : v) > > + { > > + char *cln = strstr (opt, "::"); > > + /* We don't accept '::attr'. */ > > + if (cln == nullptr || cln == opt) > > + { > > + error ("wrong argument to ignored attributes"); > > + inform (input_location, "valid format is %<ns::attr%> or %<ns::%>"); > > + continue; > > + } > > + char *vendor_start = opt; > > + ptrdiff_t vendor_len = cln - opt; > > + char *attr_start = cln + 2; > > + /* This could really use rawmemchr :(. */ > > + ptrdiff_t attr_len = strchr (attr_start, '\0') - attr_start; > > + /* Verify that they look valid. */ > > + auto valid_p = [](const char *const s, ptrdiff_t len) { > > + for (int i = 0; i < len; ++i) > > + if (!ISALNUM (*s) && *s != '_') > > + return false; > > + return true; > > + }; > > + if (!valid_p (vendor_start, vendor_len) > > + || !valid_p (attr_start, attr_len)) > > + { > > + error ("wrong argument to ignored attributes"); > > + continue; > > + } > > + /* Turn "__attr__" into "attr" so that we have a canonical form of > > + attribute names. Likewise for vendor. */ > > + auto strip = [](char *&s, ptrdiff_t &l) { > > + if (l > 4 && s[0] == '_' && s[1] == '_' > > + && s[l - 1] == '_' && s[l - 2] == '_') > > + { > > + s += 2; > > + l -= 4; > > + } > > + }; > > + strip (attr_start, attr_len); > > + strip (vendor_start, vendor_len); > > + /* We perform all this hijinks so that we don't have to copy OPT. */ > > + tree vendor_id = get_identifier_with_length (vendor_start, vendor_len); > > + tree attr_id = get_identifier_with_length (attr_start, attr_len); > > + /* If we've already seen this vendor::attr, ignore it. Attempting to > > + register it twice would lead to a crash. */ > > + if (lookup_scoped_attribute_spec (vendor_id, attr_id)) > > + continue; > > Hmm, this looks like it isn't handling the case of previously ignoring > vendor::; it seems we'll look for vendor::<empty string> instead, not > find it, and register again. I don't know if there are __attrib with 2 leading underscores but no trailing that we accept and should normalize? And it is surprising that we do not have a simple normalize_attr_name(). There must be existing spots where we would normalize attribute names? extract_attribute_substring does about the same but with a struct substring(?). Maybe that's just to avoid a normal string hash. thanks,
On Tue, Nov 09, 2021 at 12:12:10AM -0500, Jason Merrill wrote: > On 11/8/21 20:41, Marek Polacek wrote: > > On Sat, Nov 06, 2021 at 03:29:57PM -0400, Jason Merrill wrote: > > > On 11/6/21 14:28, Marek Polacek wrote: > > > > On Sat, Nov 06, 2021 at 02:32:26AM +0100, Bernhard Reutner-Fischer wrote: > > > > > On 6 November 2021 01:21:43 CET, Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > > > > > > > > > > > > > > > Thanks, so like this? I'm including an incremental diff so that it's > > > > > > clear what changed: > > > > > > > > > > > > diff --git a/gcc/attribs.c b/gcc/attribs.c > > > > > > index d5fba7f4bbb..addfe6f6c80 100644 > > > > > > --- a/gcc/attribs.c > > > > > > +++ b/gcc/attribs.c > > > > > > @@ -237,7 +237,7 @@ check_attribute_tables (void) > > > > > > the end of parsing of all TUs. */ > > > > > > static vec<attribute_spec *> ignored_attributes_table; > > > > > > > > > > > > -/* Parse arguments ARGS of -Wno-attributes=. > > > > > > +/* Parse arguments V of -Wno-attributes=. > > > > > > Currently we accept: > > > > > > vendor::attr > > > > > > vendor:: > > > > > > @@ -252,12 +252,15 @@ handle_ignored_attributes_option (vec<char *> *v) > > > > > > > > > > > > for (auto opt : v) > > > > > > { > > > > > > + /* We're going to be modifying the string. */ > > > > > > + opt = xstrdup (opt); > > > > > > char *q = strstr (opt, "::"); > > > > > > /* We don't accept '::attr'. */ > > > > > > if (q == nullptr || q == opt) > > > > > > { > > > > > > error ("wrong argument to ignored attributes"); > > > > > > inform (input_location, "valid format is %<ns::attr%> or %<ns::%>"); > > > > > > + free (opt); > > > > > > continue; > > > > > > } > > > > > > > > > > Only xstrdup here, after the strstr check? > > > > > Should maybe strdup the rest here, not full opt.. > > > > > > > > No, I want q to point into the copy of the string, since I'm about > > > > to modify it. And I'd prefer a single call to xstrdup rather than > > > > two. > > > > > > It occurs to me that instead of calling xstrdup at all, since you're already > > > passing the strings to get_identifier you could use > > > get_identifier_with_length instead, and then refer to IDENTIFIER_POINTER of > > > the result. > > > > Ah, clever. I didn't think it would work because I didn't expect that > > get_identifier_with_length works when it gets a string that isn't > > nul-terminated but it does. So how about the following? > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > -- >8 -- > > It is desirable for -Wattributes to warn about e.g. > > > > [[deprecate]] void g(); // typo, should warn > > > > However, -Wattributes also warns about vendor-specific attributes > > (that's because lookup_scoped_attribute_spec -> find_attribute_namespace > > finds nothing), which, with -Werror, causes grief. We don't want the > > -Wattributes warning for > > > > [[company::attr]] void f(); > > > > GCC warns because it doesn't know the "company" namespace; it only knows > > the "gnu" and "omp" namespaces. We could entirely disable warning about > > attributes in unknown scopes but then the compiler would also miss typos > > like > > > > [[company::attrx]] void f(); > > > > or > > > > [[gmu::warn_used_result]] int write(); > > > > so that is not a viable solution. A workaround is to use a #pragma: > > > > #pragma GCC diagnostic push > > #pragma GCC diagnostic ignored "-Wattributes" > > [[company::attr]] void f() {} > > #pragma GCC diagnostic pop > > > > but that's a mouthful and awkward to use and could also hide typos. In > > fact, any macro-based solution doesn't seem like a way forward. > > > > This patch implements -Wno-attributes=, which takes these arguments: > > > > company::attr > > company:: > > > > This option should go well with using @file: the user could have a file > > containing > > -Wno-attributes=vendor::attr1,vendor::attr2 > > and then invoke gcc with '@attrs' or similar. > > > > I've also added a new pragma which has the same effect: > > > > The pragma along with the new option should help with various static > > analysis tools. > > > > PR c++/101940 > > > > gcc/ChangeLog: > > > > * attribs.c (struct scoped_attributes): Add a bool member. > > (lookup_scoped_attribute_spec): Forward declare. > > (register_scoped_attributes): New bool parameter, defaulted to > > false. Use it. > > (handle_ignored_attributes_option): New function. > > (free_attr_data): New function. > > (init_attributes): Call handle_ignored_attributes_option. > > (attr_namespace_ignored_p): New function. > > (decl_attributes): Check attr_namespace_ignored_p before > > warning. > > * attribs.h (free_attr_data): Declare. > > (register_scoped_attributes): Adjust declaration. > > (handle_ignored_attributes_option): Declare. > > * common.opt (Wattributes=): New option with a variable. > > * doc/extend.texi: Document #pragma GCC diagnostic ignored_attributes. > > * doc/invoke.texi: Document -Wno-attributes=. > > * opts.c (common_handle_option) <case OPT_Wattributes_>: Handle. > > * plugin.h (register_scoped_attributes): Adjust declaration. > > * toplev.c (compile_file): Call free_attr_data. > > > > gcc/c-family/ChangeLog: > > > > * c-pragma.c (handle_pragma_diagnostic): Handle #pragma GCC diagnostic > > ignored_attributes. > > > > gcc/testsuite/ChangeLog: > > > > * c-c++-common/Wno-attributes-1.c: New test. > > * c-c++-common/Wno-attributes-2.c: New test. > > --- > > gcc/attribs.c | 123 +++++++++++++++++- > > gcc/attribs.h | 5 +- > > gcc/c-family/c-pragma.c | 37 +++++- > > gcc/common.opt | 9 +- > > gcc/doc/extend.texi | 19 +++ > > gcc/doc/invoke.texi | 20 +++ > > gcc/opts.c | 20 +++ > > gcc/plugin.h | 4 +- > > gcc/testsuite/c-c++-common/Wno-attributes-1.c | 55 ++++++++ > > gcc/testsuite/c-c++-common/Wno-attributes-2.c | 56 ++++++++ > > gcc/toplev.c | 2 + > > 11 files changed, 341 insertions(+), 9 deletions(-) > > create mode 100644 gcc/testsuite/c-c++-common/Wno-attributes-1.c > > create mode 100644 gcc/testsuite/c-c++-common/Wno-attributes-2.c > > > > diff --git a/gcc/attribs.c b/gcc/attribs.c > > index 83fafc98b7d..23d92ca9474 100644 > > --- a/gcc/attribs.c > > +++ b/gcc/attribs.c > > @@ -87,6 +87,8 @@ struct scoped_attributes > > const char *ns; > > vec<attribute_spec> attributes; > > hash_table<attribute_hasher> *attribute_hash; > > + /* True if we should not warn about unknown attributes in this NS. */ > > + bool ignored_p; > > }; > > /* The table of scope attributes. */ > > @@ -95,6 +97,8 @@ static vec<scoped_attributes> attributes_table; > > static scoped_attributes* find_attribute_namespace (const char*); > > static void register_scoped_attribute (const struct attribute_spec *, > > scoped_attributes *); > > +static const struct attribute_spec *lookup_scoped_attribute_spec (const_tree, > > + const_tree); > > static bool attributes_initialized = false; > > @@ -121,12 +125,14 @@ extract_attribute_substring (struct substring *str) > > /* Insert an array of attributes ATTRIBUTES into a namespace. This > > array must be NULL terminated. NS is the name of attribute > > - namespace. The function returns the namespace into which the > > - attributes have been registered. */ > > + namespace. IGNORED_P is true iff all unknown attributes in this > > + namespace should be ignored for the purposes of -Wattributes. The > > + function returns the namespace into which the attributes have been > > + registered. */ > > scoped_attributes * > > register_scoped_attributes (const struct attribute_spec *attributes, > > - const char *ns) > > + const char *ns, bool ignored_p /*=false*/) > > { > > scoped_attributes *result = NULL; > > @@ -144,9 +150,12 @@ register_scoped_attributes (const struct attribute_spec *attributes, > > memset (&sa, 0, sizeof (sa)); > > sa.ns = ns; > > sa.attributes.create (64); > > + sa.ignored_p = ignored_p; > > result = attributes_table.safe_push (sa); > > result->attribute_hash = new hash_table<attribute_hasher> (200); > > } > > + else > > + result->ignored_p |= ignored_p; > > /* Really add the attributes to their namespace now. */ > > for (unsigned i = 0; attributes[i].name != NULL; ++i) > > @@ -224,6 +233,95 @@ check_attribute_tables (void) > > attribute_tables[j][l].name)); > > } > > +/* Used to stash pointers to allocated memory so that we can free them at > > + the end of parsing of all TUs. */ > > +static vec<attribute_spec *> ignored_attributes_table; > > + > > +/* Parse arguments V of -Wno-attributes=. > > + Currently we accept: > > + vendor::attr > > + vendor:: > > + This functions also registers the parsed attributes so that we don't > > + warn that we don't recognize them. */ > > + > > +void > > +handle_ignored_attributes_option (vec<char *> *v) > > +{ > > + if (v == nullptr) > > + return; > > + > > + for (auto opt : v) > > + { > > + char *cln = strstr (opt, "::"); > > + /* We don't accept '::attr'. */ > > + if (cln == nullptr || cln == opt) > > + { > > + error ("wrong argument to ignored attributes"); > > + inform (input_location, "valid format is %<ns::attr%> or %<ns::%>"); > > + continue; > > + } > > + char *vendor_start = opt; > > + ptrdiff_t vendor_len = cln - opt; > > + char *attr_start = cln + 2; > > + /* This could really use rawmemchr :(. */ > > + ptrdiff_t attr_len = strchr (attr_start, '\0') - attr_start; > > + /* Verify that they look valid. */ > > + auto valid_p = [](const char *const s, ptrdiff_t len) { > > + for (int i = 0; i < len; ++i) > > + if (!ISALNUM (*s) && *s != '_') > > + return false; > > + return true; > > + }; > > + if (!valid_p (vendor_start, vendor_len) > > + || !valid_p (attr_start, attr_len)) > > + { > > + error ("wrong argument to ignored attributes"); > > + continue; > > + } > > + /* Turn "__attr__" into "attr" so that we have a canonical form of > > + attribute names. Likewise for vendor. */ > > + auto strip = [](char *&s, ptrdiff_t &l) { > > + if (l > 4 && s[0] == '_' && s[1] == '_' > > + && s[l - 1] == '_' && s[l - 2] == '_') > > + { > > + s += 2; > > + l -= 4; > > + } > > + }; > > + strip (attr_start, attr_len); > > + strip (vendor_start, vendor_len); > > + /* We perform all this hijinks so that we don't have to copy OPT. */ > > + tree vendor_id = get_identifier_with_length (vendor_start, vendor_len); > > + tree attr_id = get_identifier_with_length (attr_start, attr_len); > > + /* If we've already seen this vendor::attr, ignore it. Attempting to > > + register it twice would lead to a crash. */ > > + if (lookup_scoped_attribute_spec (vendor_id, attr_id)) > > + continue; > > Hmm, this looks like it isn't handling the case of previously ignoring > vendor::; it seems we'll look for vendor::<empty string> instead, not find > it, and register again. Yes, for -Wno-attributes=vendor::,vendor:: we call register_scoped_attributes twice, but I think that's OK: register_scoped_attributes will see that find_attribute_namespace finds the namespace and returns without creating a new one. I think the current code handles -Wno-attributes=vendor::a,vendor:: well so I'm not sure if I should change it. Thanks, Marek
On Tue, Nov 09, 2021 at 08:09:10AM +0100, Bernhard Reutner-Fischer wrote: > On Tue, 9 Nov 2021 00:12:10 -0500 > Jason Merrill <jason@redhat.com> wrote: > > > On 11/8/21 20:41, Marek Polacek wrote: > > > On Sat, Nov 06, 2021 at 03:29:57PM -0400, Jason Merrill wrote: > > > > + for (auto opt : v) > > > + { > > > + char *cln = strstr (opt, "::"); > > > + /* We don't accept '::attr'. */ > > > + if (cln == nullptr || cln == opt) > > > + { > > > + error ("wrong argument to ignored attributes"); > > > + inform (input_location, "valid format is %<ns::attr%> or %<ns::%>"); > > > + continue; > > > + } > > > + char *vendor_start = opt; > > > + ptrdiff_t vendor_len = cln - opt; > > > + char *attr_start = cln + 2; > > > + /* This could really use rawmemchr :(. */ > > > + ptrdiff_t attr_len = strchr (attr_start, '\0') - attr_start; > > > + /* Verify that they look valid. */ > > > + auto valid_p = [](const char *const s, ptrdiff_t len) { > > > + for (int i = 0; i < len; ++i) > > > + if (!ISALNUM (*s) && *s != '_') > > > + return false; > > > + return true; > > > + }; > > > + if (!valid_p (vendor_start, vendor_len) > > > + || !valid_p (attr_start, attr_len)) > > > + { > > > + error ("wrong argument to ignored attributes"); > > > + continue; > > > + } > > > + /* Turn "__attr__" into "attr" so that we have a canonical form of > > > + attribute names. Likewise for vendor. */ > > > + auto strip = [](char *&s, ptrdiff_t &l) { > > > + if (l > 4 && s[0] == '_' && s[1] == '_' > > > + && s[l - 1] == '_' && s[l - 2] == '_') > > > + { > > > + s += 2; > > > + l -= 4; > > > + } > > > + }; > > > + strip (attr_start, attr_len); > > > + strip (vendor_start, vendor_len); > > > + /* We perform all this hijinks so that we don't have to copy OPT. */ > > > + tree vendor_id = get_identifier_with_length (vendor_start, vendor_len); > > > + tree attr_id = get_identifier_with_length (attr_start, attr_len); > > > + /* If we've already seen this vendor::attr, ignore it. Attempting to > > > + register it twice would lead to a crash. */ > > > + if (lookup_scoped_attribute_spec (vendor_id, attr_id)) > > > + continue; > > > > Hmm, this looks like it isn't handling the case of previously ignoring > > vendor::; it seems we'll look for vendor::<empty string> instead, not > > find it, and register again. > > I don't know if there are __attrib with 2 leading underscores but no > trailing that we accept and should normalize? > > And it is surprising that we do not have a simple normalize_attr_name(). > There must be existing spots where we would normalize attribute names? > extract_attribute_substring does about the same but with a struct substring(?). > Maybe that's just to avoid a normal string hash. We have canonicalize_attr_name which I based my code on. Marek
On 11/9/21 10:55, Marek Polacek wrote: > On Tue, Nov 09, 2021 at 08:09:10AM +0100, Bernhard Reutner-Fischer wrote: >> On Tue, 9 Nov 2021 00:12:10 -0500 >> Jason Merrill <jason@redhat.com> wrote: >> >>> On 11/8/21 20:41, Marek Polacek wrote: >>>> On Sat, Nov 06, 2021 at 03:29:57PM -0400, Jason Merrill wrote: >> >>>> + for (auto opt : v) >>>> + { >>>> + char *cln = strstr (opt, "::"); >>>> + /* We don't accept '::attr'. */ >>>> + if (cln == nullptr || cln == opt) >>>> + { >>>> + error ("wrong argument to ignored attributes"); >>>> + inform (input_location, "valid format is %<ns::attr%> or %<ns::%>"); >>>> + continue; >>>> + } >>>> + char *vendor_start = opt; >>>> + ptrdiff_t vendor_len = cln - opt; >>>> + char *attr_start = cln + 2; >>>> + /* This could really use rawmemchr :(. */ >>>> + ptrdiff_t attr_len = strchr (attr_start, '\0') - attr_start; >>>> + /* Verify that they look valid. */ >>>> + auto valid_p = [](const char *const s, ptrdiff_t len) { >>>> + for (int i = 0; i < len; ++i) >>>> + if (!ISALNUM (*s) && *s != '_') >>>> + return false; >>>> + return true; >>>> + }; >>>> + if (!valid_p (vendor_start, vendor_len) >>>> + || !valid_p (attr_start, attr_len)) >>>> + { >>>> + error ("wrong argument to ignored attributes"); >>>> + continue; >>>> + } >>>> + /* Turn "__attr__" into "attr" so that we have a canonical form of >>>> + attribute names. Likewise for vendor. */ >>>> + auto strip = [](char *&s, ptrdiff_t &l) { >>>> + if (l > 4 && s[0] == '_' && s[1] == '_' >>>> + && s[l - 1] == '_' && s[l - 2] == '_') >>>> + { >>>> + s += 2; >>>> + l -= 4; >>>> + } >>>> + }; >>>> + strip (attr_start, attr_len); >>>> + strip (vendor_start, vendor_len); >>>> + /* We perform all this hijinks so that we don't have to copy OPT. */ >>>> + tree vendor_id = get_identifier_with_length (vendor_start, vendor_len); >>>> + tree attr_id = get_identifier_with_length (attr_start, attr_len); >>>> + /* If we've already seen this vendor::attr, ignore it. Attempting to >>>> + register it twice would lead to a crash. */ >>>> + if (lookup_scoped_attribute_spec (vendor_id, attr_id)) >>>> + continue; >>> >>> Hmm, this looks like it isn't handling the case of previously ignoring >>> vendor::; it seems we'll look for vendor::<empty string> instead, not >>> find it, and register again. > Yes, for -Wno-attributes=vendor::,vendor:: we call register_scoped_attributes > twice, but I think that's OK: register_scoped_attributes will see that > find_attribute_namespace finds the namespace and returns without creating a new > one. > > I think the current code handles -Wno-attributes=vendor::a,vendor:: well so > I'm not sure if I should change it. Makes sense. But we should be able to avoid calling lookup_scoped_attribute_spec, and even get_identifier_with_length (attr...), if attr_len is 0. >> I don't know if there are __attrib with 2 leading underscores but no >> trailing that we accept and should normalize? >> >> And it is surprising that we do not have a simple normalize_attr_name(). >> There must be existing spots where we would normalize attribute names? >> extract_attribute_substring does about the same but with a struct substring(?). >> Maybe that's just to avoid a normal string hash. > > We have canonicalize_attr_name which I based my code on. We could factor out of that function an overload equivalent to your strip lambda? Jason
diff --git a/gcc/attribs.c b/gcc/attribs.c index 83fafc98b7d..23d92ca9474 100644 --- a/gcc/attribs.c +++ b/gcc/attribs.c @@ -87,6 +87,8 @@ struct scoped_attributes const char *ns; vec<attribute_spec> attributes; hash_table<attribute_hasher> *attribute_hash; + /* True if we should not warn about unknown attributes in this NS. */ + bool ignored_p; }; /* The table of scope attributes. */ @@ -95,6 +97,8 @@ static vec<scoped_attributes> attributes_table; static scoped_attributes* find_attribute_namespace (const char*); static void register_scoped_attribute (const struct attribute_spec *, scoped_attributes *); +static const struct attribute_spec *lookup_scoped_attribute_spec (const_tree, + const_tree); static bool attributes_initialized = false; @@ -121,12 +125,14 @@ extract_attribute_substring (struct substring *str) /* Insert an array of attributes ATTRIBUTES into a namespace. This array must be NULL terminated. NS is the name of attribute - namespace. The function returns the namespace into which the - attributes have been registered. */ + namespace. IGNORED_P is true iff all unknown attributes in this + namespace should be ignored for the purposes of -Wattributes. The + function returns the namespace into which the attributes have been + registered. */ scoped_attributes * register_scoped_attributes (const struct attribute_spec *attributes, - const char *ns) + const char *ns, bool ignored_p /*=false*/) { scoped_attributes *result = NULL; @@ -144,9 +150,12 @@ register_scoped_attributes (const struct attribute_spec *attributes, memset (&sa, 0, sizeof (sa)); sa.ns = ns; sa.attributes.create (64); + sa.ignored_p = ignored_p; result = attributes_table.safe_push (sa); result->attribute_hash = new hash_table<attribute_hasher> (200); } + else + result->ignored_p |= ignored_p; /* Really add the attributes to their namespace now. */ for (unsigned i = 0; attributes[i].name != NULL; ++i) @@ -224,6 +233,95 @@ check_attribute_tables (void) attribute_tables[j][l].name)); } +/* Used to stash pointers to allocated memory so that we can free them at + the end of parsing of all TUs. */ +static vec<attribute_spec *> ignored_attributes_table; + +/* Parse arguments V of -Wno-attributes=. + Currently we accept: + vendor::attr + vendor:: + This functions also registers the parsed attributes so that we don't + warn that we don't recognize them. */ + +void +handle_ignored_attributes_option (vec<char *> *v) +{ + if (v == nullptr) + return; + + for (auto opt : v) + { + char *cln = strstr (opt, "::"); + /* We don't accept '::attr'. */ + if (cln == nullptr || cln == opt) + { + error ("wrong argument to ignored attributes"); + inform (input_location, "valid format is %<ns::attr%> or %<ns::%>"); + continue; + } + char *vendor_start = opt; + ptrdiff_t vendor_len = cln - opt; + char *attr_start = cln + 2; + /* This could really use rawmemchr :(. */ + ptrdiff_t attr_len = strchr (attr_start, '\0') - attr_start; + /* Verify that they look valid. */ + auto valid_p = [](const char *const s, ptrdiff_t len) { + for (int i = 0; i < len; ++i) + if (!ISALNUM (*s) && *s != '_') + return false; + return true; + }; + if (!valid_p (vendor_start, vendor_len) + || !valid_p (attr_start, attr_len)) + { + error ("wrong argument to ignored attributes"); + continue; + } + /* Turn "__attr__" into "attr" so that we have a canonical form of + attribute names. Likewise for vendor. */ + auto strip = [](char *&s, ptrdiff_t &l) { + if (l > 4 && s[0] == '_' && s[1] == '_' + && s[l - 1] == '_' && s[l - 2] == '_') + { + s += 2; + l -= 4; + } + }; + strip (attr_start, attr_len); + strip (vendor_start, vendor_len); + /* We perform all this hijinks so that we don't have to copy OPT. */ + tree vendor_id = get_identifier_with_length (vendor_start, vendor_len); + tree attr_id = get_identifier_with_length (attr_start, attr_len); + /* If we've already seen this vendor::attr, ignore it. Attempting to + register it twice would lead to a crash. */ + if (lookup_scoped_attribute_spec (vendor_id, attr_id)) + continue; + /* In the "vendor::" case, we should ignore *any* attribute coming + from this attribute namespace. */ + const char *vendor = IDENTIFIER_POINTER (vendor_id); + const char *attr = attr_len == 0 ? nullptr : IDENTIFIER_POINTER (attr_id); + /* Create a table with extra attributes which we will register. + We can't free it here, so squirrel away the pointers. */ + attribute_spec *table = new attribute_spec[2]; + ignored_attributes_table.safe_push (table); + table[0] = { attr, 0, 0, false, false, false, false, nullptr, nullptr }; + table[1] = { nullptr, 0, 0, false, false, false, false, nullptr, + nullptr }; + register_scoped_attributes (table, vendor, !attr); + } +} + +/* Free data we might have allocated when adding extra attributes. */ + +void +free_attr_data () +{ + for (auto x : ignored_attributes_table) + delete[] x; + ignored_attributes_table.release (); +} + /* Initialize attribute tables, and make some sanity checks if checking is enabled. */ @@ -252,6 +350,9 @@ init_attributes (void) /* Put all the GNU attributes into the "gnu" namespace. */ register_scoped_attributes (attribute_tables[i], "gnu"); + vec<char *> *ignored = (vec<char *> *) flag_ignored_attributes; + handle_ignored_attributes_option (ignored); + invoke_plugin_callbacks (PLUGIN_ATTRIBUTES, NULL); attributes_initialized = true; } @@ -456,6 +557,19 @@ diag_attr_exclusions (tree last_decl, tree node, tree attrname, return found; } +/* Return true iff we should not complain about unknown attributes + coming from the attribute namespace NS. This is the case for + the -Wno-attributes=ns:: command-line option. */ + +static bool +attr_namespace_ignored_p (tree ns) +{ + if (ns == NULL_TREE) + return false; + scoped_attributes *r = find_attribute_namespace (IDENTIFIER_POINTER (ns)); + return r && r->ignored_p; +} + /* Process the attributes listed in ATTRIBUTES and install them in *NODE, which is either a DECL (including a TYPE_DECL) or a TYPE. If a DECL, it should be modified in place; if a TYPE, a copy should be created @@ -556,7 +670,8 @@ decl_attributes (tree *node, tree attributes, int flags, if (spec == NULL) { - if (!(flags & (int) ATTR_FLAG_BUILT_IN)) + if (!(flags & (int) ATTR_FLAG_BUILT_IN) + && !attr_namespace_ignored_p (ns)) { if (ns == NULL_TREE || !cxx11_attr_p) warning (OPT_Wattributes, "%qE attribute directive ignored", diff --git a/gcc/attribs.h b/gcc/attribs.h index 138c509bce1..96a527f67a9 100644 --- a/gcc/attribs.h +++ b/gcc/attribs.h @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see #define GCC_ATTRIBS_H extern const struct attribute_spec *lookup_attribute_spec (const_tree); +extern void free_attr_data (); extern void init_attributes (void); /* Process the attributes listed in ATTRIBUTES and install them in *NODE, @@ -40,12 +41,14 @@ extern void apply_tm_attr (tree, tree); extern tree make_attribute (const char *, const char *, tree); extern struct scoped_attributes* register_scoped_attributes (const struct attribute_spec *, - const char *); + const char *, + bool = false); extern char *sorted_attr_string (tree); extern bool common_function_versions (tree, tree); extern tree make_dispatcher_decl (const tree); extern bool is_function_default_version (const tree); +extern void handle_ignored_attributes_option (vec<char *> *); /* Return a type like TTYPE except that its TYPE_ATTRIBUTES is ATTRIBUTE. diff --git a/gcc/c-family/c-pragma.c b/gcc/c-family/c-pragma.c index a9be8df0384..a299ceff6a2 100644 --- a/gcc/c-family/c-pragma.c +++ b/gcc/c-family/c-pragma.c @@ -764,7 +764,7 @@ handle_pragma_diagnostic(cpp_reader *ARG_UNUSED(dummy)) if (token != CPP_NAME) { warning_at (loc, OPT_Wpragmas, - "missing [error|warning|ignored|push|pop]" + "missing [error|warning|ignored|push|pop|ignored_attributes]" " after %<#pragma GCC diagnostic%>"); return; } @@ -787,10 +787,43 @@ handle_pragma_diagnostic(cpp_reader *ARG_UNUSED(dummy)) diagnostic_pop_diagnostics (global_dc, input_location); return; } + else if (strcmp (kind_string, "ignored_attributes") == 0) + { + token = pragma_lex (&x, &loc); + if (token != CPP_STRING) + { + warning_at (loc, OPT_Wpragmas, + "missing attribute name after %<#pragma GCC diagnostic " + "ignored_attributes%>"); + return; + } + char *args = xstrdup (TREE_STRING_POINTER (x)); + const size_t l = strlen (args); + if (l == 0) + { + warning_at (loc, OPT_Wpragmas, "missing argument to %<#pragma GCC " + "diagnostic ignored_attributes%>"); + free (args); + return; + } + else if (args[l - 1] == ',') + { + warning_at (loc, OPT_Wpragmas, "trailing %<,%> in arguments for " + "%<#pragma GCC diagnostic ignored_attributes%>"); + free (args); + return; + } + auto_vec<char *> v; + for (char *p = strtok (args, ","); p; p = strtok (NULL, ",")) + v.safe_push (p); + handle_ignored_attributes_option (&v); + free (args); + return; + } else { warning_at (loc, OPT_Wpragmas, - "expected [error|warning|ignored|push|pop]" + "expected [error|warning|ignored|push|pop|ignored_attributes]" " after %<#pragma GCC diagnostic%>"); return; } diff --git a/gcc/common.opt b/gcc/common.opt index 1a5b9bfcca9..de9b848eda5 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -83,7 +83,7 @@ int flag_gen_aux_info = 0 Variable int flag_shlib -; These two are really VEC(char_p,heap) *. +; These three are really VEC(char_p,heap) *. Variable void *flag_instrument_functions_exclude_functions @@ -91,6 +91,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. @@ -549,6 +552,10 @@ Wattributes Common Var(warn_attributes) Init(1) Warning Warn about inappropriate attribute usage. +Wattributes= +Common Joined +Do not warn about specified attributes. + Wattribute-alias Common Alias(Wattribute_alias=, 1, 0) Warning Warn about type safety and similar errors and mismatches in declarations with alias attributes. diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index eee4c6737bb..6e6c580e329 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -23767,6 +23767,25 @@ restored. foo(d); /* depends on command-line options */ @end smallexample +@item #pragma GCC diagnostic ignored_attributes + +Similarly to @option{-Wno-attributes=}, this pragma allows users to suppress +warnings about unknown scoped attributes (in C++11 and C2X). For example, +@code{#pragma GCC diagnostic ignored_attributes "vendor::attr"} disables +warning about the following declaration: + +@smallexample +[[vendor::attr]] void f(); +@end smallexample + +whereas @code{#pragma GCC diagnostic ignored_attributes "vendor::"} prevents +warning about both of these declarations: + +@smallexample +[[vendor::safe]] void f(); +[[vendor::unsafe]] void f2(); +@end smallexample + @end table GCC also offers a simple mechanism for printing messages during diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index f206cff1221..339b8d7597b 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -8707,6 +8707,26 @@ unrecognized attributes, function attributes applied to variables, etc. This does not stop errors for incorrect use of supported attributes. +Additionally, using @option{-Wno-attributes=}, it is possible to suppress +warnings about unknown scoped attributes (in C++11 and C2X). For example, +@option{-Wno-attributes=vendor::attr} disables warning about the following +declaration: + +@smallexample +[[vendor::attr]] void f(); +@end smallexample + +It is also possible to disable warning about all attributes in a namespace +using @option{-Wno-attributes=vendor::} which prevents warning about both +of these declarations: + +@smallexample +[[vendor::safe]] void f(); +[[vendor::unsafe]] void f2(); +@end smallexample + +Note that @option{-Wno-attributes=} does not imply @option{-Wno-attributes}. + @item -Wno-builtin-declaration-mismatch @opindex Wno-builtin-declaration-mismatch @opindex Wbuiltin-declaration-mismatch diff --git a/gcc/opts.c b/gcc/opts.c index caed6255500..175b4635bb4 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -2611,6 +2611,26 @@ common_handle_option (struct gcc_options *opts, /* Currently handled in a prescan. */ break; + case OPT_Wattributes_: + if (lang_mask == CL_DRIVER) + break; + + if (value) + { + error_at (loc, "arguments ignored for %<-Wattributes=%>; use " + "%<-Wno-attributes=%> instead"); + break; + } + else if (arg[strlen (arg) - 1] == ',') + { + error_at (loc, "trailing %<,%> in arguments for " + "%<-Wno-attributes=%>"); + break; + } + + add_comma_separated_to_vector (&opts->x_flag_ignored_attributes, arg); + break; + case OPT_Werror: dc->warning_as_error_requested = value; break; diff --git a/gcc/plugin.h b/gcc/plugin.h index 1640e253ca5..5556763d1bf 100644 --- a/gcc/plugin.h +++ b/gcc/plugin.h @@ -197,7 +197,9 @@ invoke_plugin_callbacks (int event ATTRIBUTE_UNUSED, /* In attribs.c. */ extern void register_attribute (const struct attribute_spec *attr); +/* The default argument for the third parameter is given in attribs.h. */ extern struct scoped_attributes* register_scoped_attributes (const struct attribute_spec *, - const char *); + const char *, + bool); #endif /* PLUGIN_H */ diff --git a/gcc/testsuite/c-c++-common/Wno-attributes-1.c b/gcc/testsuite/c-c++-common/Wno-attributes-1.c new file mode 100644 index 00000000000..aac1a69fd85 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wno-attributes-1.c @@ -0,0 +1,55 @@ +/* PR c++/101940 */ +/* { dg-do compile } */ +/* { 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=c2::,c2::attr" } */ +/* { dg-additional-options "-Wno-attributes=c3::attr,c3::" } */ +/* { dg-additional-options "-Wno-attributes=x::" } */ +/* { dg-additional-options "-Wno-attributes=yoyodyne::attr_new" } */ +/* { dg-additional-options "-Wno-attributes=c4::__attr__" } */ +/* { dg-additional-options "-Wno-attributes=c5::attr" } */ +/* { dg-additional-options "-Wno-attributes=__c6__::attr" } */ + +[[company::attr]] void f1(); +[[company::attr2]] void f2(); + +[[yoyodyne::attr]] void f3(); +[[yoyodyne::__attr__]] void f3__(); +[[yoyodyne::attrx]] void f4(); /* { dg-warning "ignored" } */ +[[yoyodyne::__attrx__]] void f4__(); /* { dg-warning "ignored" } */ + +[[c1::attr]] void f5(); + +[[c2::attr]] void f6(); +[[c2::attrx]] void f7(); +[[c2::__attr__]] void f6__(); +[[c2::__attrx__]] void f7__(); + +[[c3::attr]] void f8(); +[[c3::attrx]] void f9(); + +[[x::x]] void f10(); + +[[yoyodyne::attr_new]] void f11(); +[[yoyodyne::__attr_new__]] void f11__(); +[[yoyodyne::attr_mew]] void f12(); /* { dg-warning "ignored" } */ +[[yoyodyne::__attr_mew__]] void f12__(); /* { dg-warning "ignored" } */ + +[[c4::attr]] void f13(); +[[c4::__attr__]] void f13__(); +[[c4::attrx]] void f14(); /* { dg-warning "ignored" } */ + +[[c5::attr]] void f15(); +[[c5::__attr__]] void f15__(); +[[__c5__::attr]] void __f15(); +[[__c5__::__attr__]] void __f15__(); +[[c5::attrx]] void f15x(); /* { dg-warning "ignored" } */ +[[__c5__::attrx]] void f15x(); /* { dg-warning "ignored" } */ + +[[c6::attr]] void f16(); +[[c6::__attr__]] void f16__(); +[[__c6__::attr]] void __f16(); +[[__c6__::__attr__]] void __f16__(); +[[c6::attrx]] void f16x(); /* { dg-warning "ignored" } */ +[[__c6__::attrx]] void f16x(); /* { dg-warning "ignored" } */ diff --git a/gcc/testsuite/c-c++-common/Wno-attributes-2.c b/gcc/testsuite/c-c++-common/Wno-attributes-2.c new file mode 100644 index 00000000000..4307c74b048 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wno-attributes-2.c @@ -0,0 +1,56 @@ +/* PR c++/101940 */ +/* { dg-do compile } */ +/* { dg-additional-options "-std=c++11" { target c++ } } */ + +#pragma GCC diagnostic ignored_attributes "company::,yoyodyne::attr" +#pragma GCC diagnostic ignored_attributes "c1::attr,c1::attr,c1::__attr__" +#pragma GCC diagnostic ignored_attributes "c2::,c2::attr" +#pragma GCC diagnostic ignored_attributes "c3::attr,c3::" +#pragma GCC diagnostic ignored_attributes "x::" +#pragma GCC diagnostic ignored_attributes "yoyodyne::attr_new" +#pragma GCC diagnostic ignored_attributes "c4::__attr__" +#pragma GCC diagnostic ignored_attributes "c5::attr" +#pragma GCC diagnostic ignored_attributes "__c6__::attr" + +[[company::attr]] void f1(); +[[company::attr2]] void f2(); + +[[yoyodyne::attr]] void f3(); +[[yoyodyne::__attr__]] void f3__(); +[[yoyodyne::attrx]] void f4(); /* { dg-warning "ignored" } */ +[[yoyodyne::__attrx__]] void f4__(); /* { dg-warning "ignored" } */ + +[[c1::attr]] void f5(); + +[[c2::attr]] void f6(); +[[c2::attrx]] void f7(); +[[c2::__attr__]] void f6__(); +[[c2::__attrx__]] void f7__(); + +[[c3::attr]] void f8(); +[[c3::attrx]] void f9(); + +[[x::x]] void f10(); + +[[yoyodyne::attr_new]] void f11(); +[[yoyodyne::__attr_new__]] void f11__(); +[[yoyodyne::attr_mew]] void f12(); /* { dg-warning "ignored" } */ +[[yoyodyne::__attr_mew__]] void f12__(); /* { dg-warning "ignored" } */ + +[[c4::attr]] void f13(); +[[c4::__attr__]] void f13__(); +[[c4::attrx]] void f14(); /* { dg-warning "ignored" } */ + +[[c5::attr]] void f15(); +[[c5::__attr__]] void f15__(); +[[__c5__::attr]] void __f15(); +[[__c5__::__attr__]] void __f15__(); +[[c5::attrx]] void f15x(); /* { dg-warning "ignored" } */ +[[__c5__::attrx]] void f15x(); /* { dg-warning "ignored" } */ + +[[c6::attr]] void f16(); +[[c6::__attr__]] void f16__(); +[[__c6__::attr]] void __f16(); +[[__c6__::__attr__]] void __f16__(); +[[c6::attrx]] void f16x(); /* { dg-warning "ignored" } */ +[[__c6__::attrx]] void f16x(); /* { dg-warning "ignored" } */ diff --git a/gcc/toplev.c b/gcc/toplev.c index e91f083f8ff..99276bde87d 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -457,6 +457,8 @@ compile_file (void) if (flag_dump_locations) dump_location_info (stderr); + free_attr_data (); + /* Compilation is now finished except for writing what's left of the symbol table output. */