Message ID | 20220604000511.108367-1-polacek@redhat.com |
---|---|
State | New |
Headers | show |
Series | c++: Allow mixing GNU/std-style attributes [PR69585] | expand |
On 6/3/22 20:05, Marek Polacek wrote: > cp_parser_attributes_opt doesn't accept GNU attributes followed by > [[]] attributes and vice versa; only a sequence of attributes of the > same kind. That causes grief for code like: > > struct __attribute__ ((may_alias)) alignas (2) struct S { }; > > or > > #define EXPORT __attribute__((visibility("default"))) > struct [[nodiscard]] EXPORT F { }; > > It doesn't seem to a documented restriction, so this patch fixes the > problem. > > However, the patch does not touch the C FE. The C FE doesn't have > a counterpart to C++'s cp_parser_attributes_opt -- it only has > c_parser_transaction_attributes (which parses both GNU and [[]] > attributes), but that's TM-specific. The C FE seems to use either > c_parser_gnu_attributes or c_parser_std_attribute_specifier_sequence. > As a consequence, this works: > > [[maybe_unused]] __attribute__((deprecated)) void f2 (); > > but this doesn't: > > __attribute__((deprecated)) [[maybe_unused]] void f1 (); > > I'm not sure what, if anything, should be done about this. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? OK. > PR c++/102399 > PR c++/69585 > > gcc/cp/ChangeLog: > > * parser.cc (cp_parser_attributes_opt): Accept GNU attributes > followed by [[]] attributes and vice versa. > > gcc/testsuite/ChangeLog: > > * g++.dg/ext/attrib65.C: New test. > * g++.dg/ext/attrib66.C: New test. > * g++.dg/ext/attrib67.C: New test. > --- > gcc/cp/parser.cc | 14 +++++++++++--- > gcc/testsuite/g++.dg/ext/attrib65.C | 7 +++++++ > gcc/testsuite/g++.dg/ext/attrib66.C | 27 +++++++++++++++++++++++++++ > gcc/testsuite/g++.dg/ext/attrib67.C | 27 +++++++++++++++++++++++++++ > 4 files changed, 72 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/ext/attrib65.C > create mode 100644 gcc/testsuite/g++.dg/ext/attrib66.C > create mode 100644 gcc/testsuite/g++.dg/ext/attrib67.C > > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > index 3fc73442da5..535bf7eedbb 100644 > --- a/gcc/cp/parser.cc > +++ b/gcc/cp/parser.cc > @@ -28727,9 +28727,17 @@ cp_nth_tokens_can_be_attribute_p (cp_parser *parser, size_t n) > static tree > cp_parser_attributes_opt (cp_parser *parser) > { > - if (cp_next_tokens_can_be_gnu_attribute_p (parser)) > - return cp_parser_gnu_attributes_opt (parser); > - return cp_parser_std_attribute_spec_seq (parser); > + tree attrs = NULL_TREE; > + while (true) > + { > + if (cp_next_tokens_can_be_gnu_attribute_p (parser)) > + attrs = attr_chainon (attrs, cp_parser_gnu_attributes_opt (parser)); > + else if (cp_next_tokens_can_be_std_attribute_p (parser)) > + attrs = attr_chainon (attrs, cp_parser_std_attribute_spec_seq (parser)); > + else > + break; > + } > + return attrs; > } > > /* Parse an (optional) series of attributes. > diff --git a/gcc/testsuite/g++.dg/ext/attrib65.C b/gcc/testsuite/g++.dg/ext/attrib65.C > new file mode 100644 > index 00000000000..0af138700f6 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/ext/attrib65.C > @@ -0,0 +1,7 @@ > +// PR c++/102399 > +// { dg-do compile { target c++11 } } > +// Test mixing the GNU and standard forms of attributes. > + > +#define EXPORT __attribute__((visibility("default"))) > + > +struct [[nodiscard]] EXPORT Foo { Foo(); }; > diff --git a/gcc/testsuite/g++.dg/ext/attrib66.C b/gcc/testsuite/g++.dg/ext/attrib66.C > new file mode 100644 > index 00000000000..102ed709b1d > --- /dev/null > +++ b/gcc/testsuite/g++.dg/ext/attrib66.C > @@ -0,0 +1,27 @@ > +// PR c++/69585 > +// { dg-do compile { target c++11 } } > + > +struct __attribute__ ((aligned (2))) __attribute__ ((may_alias)) > +S1 { }; > + > +struct __attribute__ ((aligned (2))) [[gnu::may_alias]] > +S2 { }; > + > +struct alignas (2) __attribute__ ((may_alias)) > +S3 { }; > + > +struct alignas (2) [[gnu::may_alias]] > +S4 { }; > + > + > +struct __attribute__ ((may_alias)) __attribute__ ((aligned (2))) > +S1_2 { }; > + > +struct [[gnu::may_alias]] __attribute__ ((aligned (2))) > +S2_2 { }; > + > +struct __attribute__ ((may_alias)) alignas (2) > +S3_2 { }; > + > +struct [[gnu::may_alias]] alignas (2) > +S4_2 { }; > diff --git a/gcc/testsuite/g++.dg/ext/attrib67.C b/gcc/testsuite/g++.dg/ext/attrib67.C > new file mode 100644 > index 00000000000..a5107665077 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/ext/attrib67.C > @@ -0,0 +1,27 @@ > +// PR c++/69585 > +// { dg-do compile { target c++11 } } > +// Test mixing the GNU and standard forms of attributes. > + > +__attribute__((deprecated)) [[maybe_unused]] void f1 (); > +[[maybe_unused]] __attribute__((deprecated)) void f2 (); > +[[maybe_unused]] __attribute__((deprecated)) [[nodiscard]] int f3 (); > +__attribute__((unused)) [[nodiscard]] __attribute__((deprecated)) int f4 (); > + > +struct [[maybe_unused]] __attribute__((aligned)) S1 { double d; }; > +struct __attribute__((aligned)) [[maybe_unused]] S2 { double d; }; > + > +enum E { > + X [[maybe_unused]] __attribute__((unavailable)), > + Y __attribute__((unavailable)) [[maybe_unused]], > +}; > + > +void > +g ([[maybe_unused]] __attribute__((unavailable)) int i1, > + __attribute__((unavailable)) [[maybe_unused]] int i2) > +{ > + [[maybe_unused]] __attribute__((aligned)) int i3; > + __attribute__((aligned)) [[maybe_unused]] int i4; > + > +[[maybe_unused]] > +lab: __attribute__((cold)); > +} > > base-commit: 891d64721626f45fb95fa47a57a3f396b80f31e9
On Fri, 3 Jun 2022, Marek Polacek via Gcc-patches wrote: > However, the patch does not touch the C FE. The C FE doesn't have > a counterpart to C++'s cp_parser_attributes_opt -- it only has > c_parser_transaction_attributes (which parses both GNU and [[]] > attributes), but that's TM-specific. The C FE seems to use either > c_parser_gnu_attributes or c_parser_std_attribute_specifier_sequence. > As a consequence, this works: > > [[maybe_unused]] __attribute__((deprecated)) void f2 (); > > but this doesn't: > > __attribute__((deprecated)) [[maybe_unused]] void f1 (); > > I'm not sure what, if anything, should be done about this. In many places, both GNU and [[]] attributes are valid, but appertain to different entities, so it would be hard to define semantics for mixing them. At the start of a declaration like that, for example, GNU attributes can be mixed arbitrarily among the declaration specifiers (and appertain to the declaration) - but [[]] attributes can appear only before the declaration specifiers (appertaining to the declaration) or after them (appertaining to the type determined by those declaration specifiers), not in the middle.
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index 3fc73442da5..535bf7eedbb 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -28727,9 +28727,17 @@ cp_nth_tokens_can_be_attribute_p (cp_parser *parser, size_t n) static tree cp_parser_attributes_opt (cp_parser *parser) { - if (cp_next_tokens_can_be_gnu_attribute_p (parser)) - return cp_parser_gnu_attributes_opt (parser); - return cp_parser_std_attribute_spec_seq (parser); + tree attrs = NULL_TREE; + while (true) + { + if (cp_next_tokens_can_be_gnu_attribute_p (parser)) + attrs = attr_chainon (attrs, cp_parser_gnu_attributes_opt (parser)); + else if (cp_next_tokens_can_be_std_attribute_p (parser)) + attrs = attr_chainon (attrs, cp_parser_std_attribute_spec_seq (parser)); + else + break; + } + return attrs; } /* Parse an (optional) series of attributes. diff --git a/gcc/testsuite/g++.dg/ext/attrib65.C b/gcc/testsuite/g++.dg/ext/attrib65.C new file mode 100644 index 00000000000..0af138700f6 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attrib65.C @@ -0,0 +1,7 @@ +// PR c++/102399 +// { dg-do compile { target c++11 } } +// Test mixing the GNU and standard forms of attributes. + +#define EXPORT __attribute__((visibility("default"))) + +struct [[nodiscard]] EXPORT Foo { Foo(); }; diff --git a/gcc/testsuite/g++.dg/ext/attrib66.C b/gcc/testsuite/g++.dg/ext/attrib66.C new file mode 100644 index 00000000000..102ed709b1d --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attrib66.C @@ -0,0 +1,27 @@ +// PR c++/69585 +// { dg-do compile { target c++11 } } + +struct __attribute__ ((aligned (2))) __attribute__ ((may_alias)) +S1 { }; + +struct __attribute__ ((aligned (2))) [[gnu::may_alias]] +S2 { }; + +struct alignas (2) __attribute__ ((may_alias)) +S3 { }; + +struct alignas (2) [[gnu::may_alias]] +S4 { }; + + +struct __attribute__ ((may_alias)) __attribute__ ((aligned (2))) +S1_2 { }; + +struct [[gnu::may_alias]] __attribute__ ((aligned (2))) +S2_2 { }; + +struct __attribute__ ((may_alias)) alignas (2) +S3_2 { }; + +struct [[gnu::may_alias]] alignas (2) +S4_2 { }; diff --git a/gcc/testsuite/g++.dg/ext/attrib67.C b/gcc/testsuite/g++.dg/ext/attrib67.C new file mode 100644 index 00000000000..a5107665077 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attrib67.C @@ -0,0 +1,27 @@ +// PR c++/69585 +// { dg-do compile { target c++11 } } +// Test mixing the GNU and standard forms of attributes. + +__attribute__((deprecated)) [[maybe_unused]] void f1 (); +[[maybe_unused]] __attribute__((deprecated)) void f2 (); +[[maybe_unused]] __attribute__((deprecated)) [[nodiscard]] int f3 (); +__attribute__((unused)) [[nodiscard]] __attribute__((deprecated)) int f4 (); + +struct [[maybe_unused]] __attribute__((aligned)) S1 { double d; }; +struct __attribute__((aligned)) [[maybe_unused]] S2 { double d; }; + +enum E { + X [[maybe_unused]] __attribute__((unavailable)), + Y __attribute__((unavailable)) [[maybe_unused]], +}; + +void +g ([[maybe_unused]] __attribute__((unavailable)) int i1, + __attribute__((unavailable)) [[maybe_unused]] int i2) +{ + [[maybe_unused]] __attribute__((aligned)) int i3; + __attribute__((aligned)) [[maybe_unused]] int i4; + +[[maybe_unused]] +lab: __attribute__((cold)); +}