| Message ID | C6DEA1BA-5FF7-4417-AE4E-0D294BDF158E@sandoe.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | Objective-C++ : Allow prefix attrs on linkage specs. | expand |
On 11/7/20 8:11 AM, Iain Sandoe wrote: > Hi, > > For Objective-C++/C, we cater for the possibility that a class interface > (@interface) might be preceded by prefix attributes. In the case of > Objective-C++, the reference implementation (a.k.a. clang) also allows > (and combines) prefix attributes that precede a linkage specification > (but only on a single decl). > > Some discussion with Nathan here: > https://gcc.gnu.org/pipermail/gcc/2020-October/234057.html > > The upshot is that clang’s behaviour is inconsistent (I can file a bug, > I guess) - but since what is “well-formed” for Objective-C is defined in > reality by what clang accepts - there is a body of code out there that > depends on the behaviour (some variant of Hyrum’s law, or corollary > to it, perhaps?). > > Inability to parse code including these patterns is blocking progress > in modernising GNU Objective-C.. so I need to find a way forward. > > The compromise made here is to accept the sequence when parsing > for Objective-C++, and to warn** that the attributes are discarded otherwise. > > This seems to me to be an improvement in diagnostics for regular C++ > (since it now says something pertinent to the actual problem and does > the 'same as usual' when encountering an unhandled attribute). > > Tested across the Darwin patch, and on x86_64-linux-gnu, > OK for master? > thanks > Iain > > ** trivially, that could be an error instead - but it seems we usually warn > for unrecognised attributes. > > —— commit message > > For Objective-C++, this combines prefix attributes from before and > after top level linkage specs. The "reference implementation" for > Objective-C++ allows this, and system headers depend on it. > > e.g. > > __attribute__((__deprecated__)) > extern "C" __attribute__((__visibility__("default"))) > @interface MyClass > ... > @end > > Would consider the list of prefix attributes to the interface for > MyClass to include both the visibility and deprecated ones. > > When we are compiling regular C++, this emits a warning and discards > any prefix attributes before a linkage spec. > > gcc/cp/ChangeLog: > > * parser.c (cp_parser_declaration): Unless we are compiling for > Ojective-C++, warn about and discard any attributes that prefix > a linkage specification. Absolutely willing to trust your judgment on ObjC++. So, OK jeff
On 11/7/20 10:11 AM, Iain Sandoe wrote: > Hi, > > For Objective-C++/C, we cater for the possibility that a class interface > (@interface) might be preceded by prefix attributes. In the case of > Objective-C++, the reference implementation (a.k.a. clang) also allows > (and combines) prefix attributes that precede a linkage specification > (but only on a single decl). > > Some discussion with Nathan here: > https://gcc.gnu.org/pipermail/gcc/2020-October/234057.html > > The upshot is that clang’s behaviour is inconsistent (I can file a bug, > I guess) - but since what is “well-formed” for Objective-C is defined in > reality by what clang accepts - there is a body of code out there that > depends on the behaviour (some variant of Hyrum’s law, or corollary > to it, perhaps?). > > Inability to parse code including these patterns is blocking progress > in modernising GNU Objective-C.. so I need to find a way forward. > > The compromise made here is to accept the sequence when parsing > for Objective-C++, and to warn** that the attributes are discarded otherwise. > > This seems to me to be an improvement in diagnostics for regular C++ > (since it now says something pertinent to the actual problem and does > the 'same as usual' when encountering an unhandled attribute). > > Tested across the Darwin patch, and on x86_64-linux-gnu, > OK for master? > thanks > Iain > > ** trivially, that could be an error instead - but it seems we usually warn > for unrecognised attributes. > > —— commit message > > For Objective-C++, this combines prefix attributes from before and > after top level linkage specs. The "reference implementation" for > Objective-C++ allows this, and system headers depend on it. > > e.g. > > __attribute__((__deprecated__)) > extern "C" __attribute__((__visibility__("default"))) > @interface MyClass > ... > @end > > Would consider the list of prefix attributes to the interface for > MyClass to include both the visibility and deprecated ones. > > When we are compiling regular C++, this emits a warning and discards > any prefix attributes before a linkage spec. > > gcc/cp/ChangeLog: > > * parser.c (cp_parser_declaration): Unless we are compiling for > Ojective-C++, warn about and discard any attributes that prefix > a linkage specification. > --- > gcc/cp/parser.c | 71 +++++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 57 insertions(+), 14 deletions(-) > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index c4c672efa09..320d151c060 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -2187,7 +2187,7 @@ static void cp_parser_already_scoped_statement > static void cp_parser_declaration_seq_opt > (cp_parser *); > static void cp_parser_declaration > - (cp_parser *); > + (cp_parser *, tree); > static void cp_parser_toplevel_declaration > (cp_parser *); > static void cp_parser_block_declaration > @@ -2238,7 +2238,7 @@ static tree cp_parser_alias_declaration > static void cp_parser_asm_definition > (cp_parser *); > static void cp_parser_linkage_specification > - (cp_parser *); > + (cp_parser *, tree); > static void cp_parser_static_assert > (cp_parser *, bool); > static tree cp_parser_decltype > @@ -13496,7 +13496,7 @@ cp_parser_declaration_seq_opt (cp_parser* parser) > __extension__ declaration */ > > static void > -cp_parser_declaration (cp_parser* parser) > +cp_parser_declaration (cp_parser* parser, tree prefix_attrs) > { > int saved_pedantic; > > @@ -13504,7 +13504,7 @@ cp_parser_declaration (cp_parser* parser) > if (cp_parser_extension_opt (parser, &saved_pedantic)) > { > /* Parse the qualified declaration. */ > - cp_parser_declaration (parser); > + cp_parser_declaration (parser, prefix_attrs); > /* Restore the PEDANTIC flag. */ > pedantic = saved_pedantic; > > @@ -13521,11 +13521,50 @@ cp_parser_declaration (cp_parser* parser) > > tree attributes = NULL_TREE; > > + /* Conditionally, allow attributes to precede a linkage specification. */ > + if (token1->keyword == RID_ATTRIBUTE) > + { > + cp_lexer_save_tokens (parser->lexer); > + attributes = cp_parser_attributes_opt (parser); > + gcc_checking_assert (attributes); > + cp_token *t1 = cp_lexer_peek_token (parser->lexer); > + cp_token *t2 = (t1->type == CPP_EOF > + ? t1 : cp_lexer_peek_nth_token (parser->lexer, 2)); > + if (t1->keyword == RID_EXTERN > + && cp_parser_is_pure_string_literal (t2)) > + { > + cp_lexer_commit_tokens (parser->lexer); > + /* We might have already been here. */ > + if (!c_dialect_objc ()) > + { > + warning_at (token1->location, OPT_Wattributes, "attributes are" > + " only permitted in this position for Objective-C++," > + " ignored"); It would be nice for the warning to suggest where to move the attribute, rather than just say that this location is bad. > + attributes = NULL_TREE; > + } > + token1 = t1; > + token2 = t2; > + } > + else > + { > + cp_lexer_rollback_tokens (parser->lexer); > + attributes = NULL_TREE; > + } > + } > + /* If we already had some attributes, and we've added more, then prepend. > + Otherwise attributes just contains any that we just read. */ > + if (prefix_attrs) > + { > + if (attributes) > + TREE_CHAIN (prefix_attrs) = attributes; > + attributes = prefix_attrs; > + } > + > /* If the next token is `extern' and the following token is a string > literal, then we have a linkage specification. */ > if (token1->keyword == RID_EXTERN > && cp_parser_is_pure_string_literal (token2)) > - cp_parser_linkage_specification (parser); > + cp_parser_linkage_specification (parser, attributes); > /* If the next token is `template', then we have either a template > declaration, an explicit instantiation, or an explicit > specialization. */ > @@ -13575,7 +13614,7 @@ cp_parser_declaration (cp_parser* parser) > cp_parser_namespace_definition (parser); > /* Objective-C++ declaration/definition. */ > else if (c_dialect_objc () && OBJC_IS_AT_KEYWORD (token1->keyword)) > - cp_parser_objc_declaration (parser, NULL_TREE); > + cp_parser_objc_declaration (parser, attributes); > else if (c_dialect_objc () > && token1->keyword == RID_ATTRIBUTE > && cp_parser_objc_valid_prefix_attributes (parser, &attributes)) > @@ -13617,7 +13656,7 @@ cp_parser_toplevel_declaration (cp_parser* parser) > } > else > /* Parse the declaration itself. */ > - cp_parser_declaration (parser); > + cp_parser_declaration (parser, NULL_TREE); > } > > /* Parse a block-declaration. > @@ -14726,7 +14765,7 @@ cp_parser_function_specifier_opt (cp_parser* parser, > extern string-literal declaration */ > > static void > -cp_parser_linkage_specification (cp_parser* parser) > +cp_parser_linkage_specification (cp_parser* parser, tree prefix_attr) > { > tree linkage; > > @@ -14791,7 +14830,7 @@ cp_parser_linkage_specification (cp_parser* parser) > saved_in_unbraced_linkage_specification_p > = parser->in_unbraced_linkage_specification_p; > parser->in_unbraced_linkage_specification_p = true; > - cp_parser_declaration (parser); > + cp_parser_declaration (parser, prefix_attr); > parser->in_unbraced_linkage_specification_p > = saved_in_unbraced_linkage_specification_p; > } > @@ -33089,7 +33128,7 @@ cp_parser_objc_interstitial_code (cp_parser* parser) > if (token->keyword == RID_EXTERN > && cp_parser_is_pure_string_literal > (cp_lexer_peek_nth_token (parser->lexer, 2))) > - cp_parser_linkage_specification (parser); > + cp_parser_linkage_specification (parser, NULL_TREE); > /* Handle #pragma, if any. */ > else if (token->type == CPP_PRAGMA) > cp_parser_pragma (parser, pragma_objc_icode, NULL); > @@ -33854,11 +33893,15 @@ static bool > cp_parser_objc_valid_prefix_attributes (cp_parser* parser, tree *attrib) > { > cp_lexer_save_tokens (parser->lexer); > - *attrib = cp_parser_attributes_opt (parser); > - gcc_assert (*attrib); > + tree addon = cp_parser_attributes_opt (parser); > + gcc_checking_assert (addon); > if (OBJC_IS_AT_KEYWORD (cp_lexer_peek_token (parser->lexer)->keyword)) > { > cp_lexer_commit_tokens (parser->lexer); > + if (*attrib) > + TREE_CHAIN (*attrib) = addon; > + else > + *attrib = addon; > return true; > } > cp_lexer_rollback_tokens (parser->lexer); > @@ -41869,7 +41912,7 @@ cp_parser_omp_declare_simd (cp_parser *parser, cp_token *pragma_tok, > switch (context) > { > case pragma_external: > - cp_parser_declaration (parser); > + cp_parser_declaration (parser, NULL_TREE); > break; > case pragma_member: > cp_parser_member_declaration (parser); > @@ -43426,7 +43469,7 @@ cp_parser_oacc_routine (cp_parser *parser, cp_token *pragma_tok, > } > > /* We only have to consider the pragma_external case here. */ > - cp_parser_declaration (parser); > + cp_parser_declaration (parser, NULL_TREE); > if (parser->oacc_routine > && !parser->oacc_routine->fndecl_seen) > cp_ensure_no_oacc_routine (parser); >
Jason Merrill <jason@redhat.com> wrote: > On 11/7/20 10:11 AM, Iain Sandoe wrote: >> + warning_at (token1->location, OPT_Wattributes, "attributes are" >> + " only permitted in this position for Objective-C++," >> + " ignored"); > > It would be nice for the warning to suggest where to move the attribute, > rather than just say that this location is bad. f.C: __attribute__(()) extern "C" int foo (void); === So, I looked at two possibles. 1/ trying to use a fixit hint - but this seems a little lacking in information to me. f.C:2:1: warning: attributes are only permitted in this position for Objective-C++ (ignored for C++) [-Wattributes] 2 | __attribute__(()) | ^~~~~~~~~~~~~ 3 | extern "C" int foo (void); | __attribute__ (NOTE: ‘__attribute__' starting indent is below ‘int’ for a fixed spacing font) 2/ adding an inform: f.C:2:1: warning: attributes are only permitted in this position for Objective-C++ (ignored for C++) [-Wattributes] 2 | __attribute__(()) | ^~~~~~~~~~~~~ f.C:3:12: note: for C++, attributes may be added here 3 | extern "C" int foo (void); | ^~~ (NOTE: likewise, ^~~ starting indent is below ‘int’ for a fixed spacing font) === I’m inclined to think that the second is more useful, but have patches for both, which (or something else) do you prefer? thanks Iain
On Mon, Nov 23, 2020 at 8:52 AM Iain Sandoe <iain@sandoe.co.uk> wrote: > Jason Merrill <jason@redhat.com> wrote: > > > On 11/7/20 10:11 AM, Iain Sandoe wrote: > > >> + warning_at (token1->location, OPT_Wattributes, "attributes > are" > >> + " only permitted in this position for > Objective-C++," > >> + " ignored"); > > > > It would be nice for the warning to suggest where to move the > attribute, > > rather than just say that this location is bad. > > f.C: > > __attribute__(()) > extern "C" int foo (void); > > === > > So, I looked at two possibles. > > 1/ trying to use a fixit hint - but this seems a little lacking in > information to me. > > f.C:2:1: warning: attributes are only permitted in this position for > Objective-C++ (ignored for C++) [-Wattributes] > 2 | __attribute__(()) > | ^~~~~~~~~~~~~ > 3 | extern "C" int foo (void); > | __attribute__ > > (NOTE: ‘__attribute__' starting indent is below ‘int’ for a fixed spacing > font) > > 2/ adding an inform: > > f.C:2:1: warning: attributes are only permitted in this position for > Objective-C++ (ignored for C++) [-Wattributes] > 2 | __attribute__(()) > | ^~~~~~~~~~~~~ > f.C:3:12: note: for C++, attributes may be added here > 3 | extern "C" int foo (void); > | ^~~ > > (NOTE: likewise, ^~~ starting indent is below ‘int’ for a fixed spacing > font) > > === > > I’m inclined to think that the second is more useful, > but have patches for both, > which (or something else) do you prefer? > I agree that the second is preferable, thanks. But let's not underline all of "int" there, just the caret is sufficient. I'd also drop the mention of Obj-C++. Jason
(resending - this didn’t seem to reach gcc-patches@) Jason Merrill <jason@redhat.com> wrote: > On Mon, Nov 23, 2020 at 8:52 AM Iain Sandoe <iain@sandoe.co.uk> wrote: > Jason Merrill <jason@redhat.com> wrote: > (NOTE: likewise, ^~~ starting indent is below ‘int’ for a fixed spacing > font) > > === > > I’m inclined to think that the second is more useful, > but have patches for both, > which (or something else) do you prefer? > > I agree that the second is preferable, thanks. But let's not underline > all of "int" there, just the caret is sufficient. I'd also drop the > mention of Obj-C++. t.C:2:1: warning: attributes are not permitted in this position [-Wattributes] 2 | __attribute__(()) | ^~~~~~~~~~~~~ t.C:3:11: note: attributes may be inserted here 3 | extern "C" int foo (void); | ^ (the caret _is_ below the space) (cool, I got to find out how to make a diagnostic point to the space between two tokens) OK? thanks Iain [PATCH] C++ : Adjust warning for misplaced attributes. This removes the reference to Objective-C++ for the warning that attributes may not be placed before linkage specifications. It also adds a note that they may be placed after that. gcc/cp/ChangeLog: * parser.c (cp_parser_declaration): Add a not about where attributes may be placed. --- gcc/cp/parser.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 7a83bf4a2a7..fe1dffc391f 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -13567,9 +13567,12 @@ cp_parser_declaration (cp_parser* parser, tree prefix_attrs) /* We might have already been here. */ if (!c_dialect_objc ()) { + location_t where = get_finish (t2->location); warning_at (token1->location, OPT_Wattributes, "attributes are" - " only permitted in this position for Objective-C++," - " ignored"); + " not permitted in this position"); + where = linemap_position_for_loc_and_offset (line_table, + where, 1); + inform (where, "attributes may be inserted here"); attributes = NULL_TREE; } token1 = t1;
On 11/27/20 6:08 PM, Iain Sandoe wrote: > Jason Merrill <jason@redhat.com> wrote: > >> On Mon, Nov 23, 2020 at 8:52 AM Iain Sandoe <iain@sandoe.co.uk> wrote: >> Jason Merrill <jason@redhat.com> wrote: > >> (NOTE: likewise, ^~~ starting indent is below ‘int’ for a fixed spacing >> font) >> >> === >> >> I’m inclined to think that the second is more useful, >> but have patches for both, >> which (or something else) do you prefer? >> >> I agree that the second is preferable, thanks. But let's not >> underline all of "int" there, just the caret is sufficient. I'd also >> drop the mention of Obj-C++. > > t.C:2:1: warning: attributes are not permitted in this position > [-Wattributes] > 2 | __attribute__(()) > | ^~~~~~~~~~~~~ > t.C:3:11: note: attributes may be inserted here > 3 | extern "C" int foo (void); > | ^ > > (the caret _is_ below the space) > > (cool, I got to find out how to make a diagnostic point to the space > between two tokens) > > OK? > thanks > Iain OK, thanks. > [PATCH] C++ : Adjust warning for misplaced attributes. > > This removes the reference to Objective-C++ for the warning that > attributes may not be placed before linkage specifications. It also > adds a note that they may be placed after that. > gcc/cp/ChangeLog: > > * parser.c (cp_parser_declaration): Add a not about where > attributes may be placed. > --- > gcc/cp/parser.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index 7a83bf4a2a7..fe1dffc391f 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -13567,9 +13567,12 @@ cp_parser_declaration (cp_parser* parser, tree > prefix_attrs) > /* We might have already been here. */ > if (!c_dialect_objc ()) > { > + location_t where = get_finish (t2->location); > warning_at (token1->location, OPT_Wattributes, "attributes are" > - " only permitted in this position for Objective-C++," > - " ignored"); > + " not permitted in this position"); > + where = linemap_position_for_loc_and_offset (line_table, > + where, 1); > + inform (where, "attributes may be inserted here"); > attributes = NULL_TREE; > } > token1 = t1; > -- > 2.24.1 > >
On 11/28/20 4:53 AM, Iain Sandoe wrote: > (resending - this didn’t seem to reach gcc-patches@) > > Jason Merrill <jason@redhat.com> wrote: > >> On Mon, Nov 23, 2020 at 8:52 AM Iain Sandoe <iain@sandoe.co.uk> wrote: >> Jason Merrill <jason@redhat.com> wrote: > >> (NOTE: likewise, ^~~ starting indent is below ‘int’ for a fixed spacing >> font) >> >> === >> >> I’m inclined to think that the second is more useful, >> but have patches for both, >> which (or something else) do you prefer? >> >> I agree that the second is preferable, thanks. But let's not >> underline all of "int" there, just the caret is sufficient. I'd also >> drop the mention of Obj-C++. > > t.C:2:1: warning: attributes are not permitted in this position > [-Wattributes] > 2 | __attribute__(()) > | ^~~~~~~~~~~~~ > t.C:3:11: note: attributes may be inserted here > 3 | extern "C" int foo (void); > | ^ > > (the caret _is_ below the space) > > (cool, I got to find out how to make a diagnostic point to the space > between two tokens) > > OK? > thanks > Iain OK, thanks. > [PATCH] C++ : Adjust warning for misplaced attributes. > > This removes the reference to Objective-C++ for the warning that > attributes may not be placed before linkage specifications. It also > adds a note that they may be placed after that. > > gcc/cp/ChangeLog: > > * parser.c (cp_parser_declaration): Add a not about where > attributes may be placed. > --- > gcc/cp/parser.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index 7a83bf4a2a7..fe1dffc391f 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -13567,9 +13567,12 @@ cp_parser_declaration (cp_parser* parser, tree > prefix_attrs) > /* We might have already been here. */ > if (!c_dialect_objc ()) > { > + location_t where = get_finish (t2->location); > warning_at (token1->location, OPT_Wattributes, "attributes are" > - " only permitted in this position for Objective-C++," > - " ignored"); > + " not permitted in this position"); > + where = linemap_position_for_loc_and_offset (line_table, > + where, 1); > + inform (where, "attributes may be inserted here"); > attributes = NULL_TREE; > } > token1 = t1;
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index c4c672efa09..320d151c060 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -2187,7 +2187,7 @@ static void cp_parser_already_scoped_statement static void cp_parser_declaration_seq_opt (cp_parser *); static void cp_parser_declaration - (cp_parser *); + (cp_parser *, tree); static void cp_parser_toplevel_declaration (cp_parser *); static void cp_parser_block_declaration @@ -2238,7 +2238,7 @@ static tree cp_parser_alias_declaration static void cp_parser_asm_definition (cp_parser *); static void cp_parser_linkage_specification - (cp_parser *); + (cp_parser *, tree); static void cp_parser_static_assert (cp_parser *, bool); static tree cp_parser_decltype @@ -13496,7 +13496,7 @@ cp_parser_declaration_seq_opt (cp_parser* parser) __extension__ declaration */ static void -cp_parser_declaration (cp_parser* parser) +cp_parser_declaration (cp_parser* parser, tree prefix_attrs) { int saved_pedantic; @@ -13504,7 +13504,7 @@ cp_parser_declaration (cp_parser* parser) if (cp_parser_extension_opt (parser, &saved_pedantic)) { /* Parse the qualified declaration. */ - cp_parser_declaration (parser); + cp_parser_declaration (parser, prefix_attrs); /* Restore the PEDANTIC flag. */ pedantic = saved_pedantic; @@ -13521,11 +13521,50 @@ cp_parser_declaration (cp_parser* parser) tree attributes = NULL_TREE; + /* Conditionally, allow attributes to precede a linkage specification. */ + if (token1->keyword == RID_ATTRIBUTE) + { + cp_lexer_save_tokens (parser->lexer); + attributes = cp_parser_attributes_opt (parser); + gcc_checking_assert (attributes); + cp_token *t1 = cp_lexer_peek_token (parser->lexer); + cp_token *t2 = (t1->type == CPP_EOF + ? t1 : cp_lexer_peek_nth_token (parser->lexer, 2)); + if (t1->keyword == RID_EXTERN + && cp_parser_is_pure_string_literal (t2)) + { + cp_lexer_commit_tokens (parser->lexer); + /* We might have already been here. */ + if (!c_dialect_objc ()) + { + warning_at (token1->location, OPT_Wattributes, "attributes are" + " only permitted in this position for Objective-C++," + " ignored"); + attributes = NULL_TREE; + } + token1 = t1; + token2 = t2; + } + else + { + cp_lexer_rollback_tokens (parser->lexer); + attributes = NULL_TREE; + } + } + /* If we already had some attributes, and we've added more, then prepend. + Otherwise attributes just contains any that we just read. */ + if (prefix_attrs) + { + if (attributes) + TREE_CHAIN (prefix_attrs) = attributes; + attributes = prefix_attrs; + } + /* If the next token is `extern' and the following token is a string literal, then we have a linkage specification. */ if (token1->keyword == RID_EXTERN && cp_parser_is_pure_string_literal (token2)) - cp_parser_linkage_specification (parser); + cp_parser_linkage_specification (parser, attributes); /* If the next token is `template', then we have either a template declaration, an explicit instantiation, or an explicit specialization. */ @@ -13575,7 +13614,7 @@ cp_parser_declaration (cp_parser* parser) cp_parser_namespace_definition (parser); /* Objective-C++ declaration/definition. */ else if (c_dialect_objc () && OBJC_IS_AT_KEYWORD (token1->keyword)) - cp_parser_objc_declaration (parser, NULL_TREE); + cp_parser_objc_declaration (parser, attributes); else if (c_dialect_objc () && token1->keyword == RID_ATTRIBUTE && cp_parser_objc_valid_prefix_attributes (parser, &attributes)) @@ -13617,7 +13656,7 @@ cp_parser_toplevel_declaration (cp_parser* parser) } else /* Parse the declaration itself. */ - cp_parser_declaration (parser); + cp_parser_declaration (parser, NULL_TREE); } /* Parse a block-declaration. @@ -14726,7 +14765,7 @@ cp_parser_function_specifier_opt (cp_parser* parser, extern string-literal declaration */ static void -cp_parser_linkage_specification (cp_parser* parser) +cp_parser_linkage_specification (cp_parser* parser, tree prefix_attr) { tree linkage; @@ -14791,7 +14830,7 @@ cp_parser_linkage_specification (cp_parser* parser) saved_in_unbraced_linkage_specification_p = parser->in_unbraced_linkage_specification_p; parser->in_unbraced_linkage_specification_p = true; - cp_parser_declaration (parser); + cp_parser_declaration (parser, prefix_attr); parser->in_unbraced_linkage_specification_p = saved_in_unbraced_linkage_specification_p; } @@ -33089,7 +33128,7 @@ cp_parser_objc_interstitial_code (cp_parser* parser) if (token->keyword == RID_EXTERN && cp_parser_is_pure_string_literal (cp_lexer_peek_nth_token (parser->lexer, 2))) - cp_parser_linkage_specification (parser); + cp_parser_linkage_specification (parser, NULL_TREE); /* Handle #pragma, if any. */ else if (token->type == CPP_PRAGMA) cp_parser_pragma (parser, pragma_objc_icode, NULL); @@ -33854,11 +33893,15 @@ static bool cp_parser_objc_valid_prefix_attributes (cp_parser* parser, tree *attrib) { cp_lexer_save_tokens (parser->lexer); - *attrib = cp_parser_attributes_opt (parser); - gcc_assert (*attrib); + tree addon = cp_parser_attributes_opt (parser); + gcc_checking_assert (addon); if (OBJC_IS_AT_KEYWORD (cp_lexer_peek_token (parser->lexer)->keyword)) { cp_lexer_commit_tokens (parser->lexer); + if (*attrib) + TREE_CHAIN (*attrib) = addon; + else + *attrib = addon; return true; } cp_lexer_rollback_tokens (parser->lexer); @@ -41869,7 +41912,7 @@ cp_parser_omp_declare_simd (cp_parser *parser, cp_token *pragma_tok, switch (context) { case pragma_external: - cp_parser_declaration (parser); + cp_parser_declaration (parser, NULL_TREE); break; case pragma_member: cp_parser_member_declaration (parser); @@ -43426,7 +43469,7 @@ cp_parser_oacc_routine (cp_parser *parser, cp_token *pragma_tok, } /* We only have to consider the pragma_external case here. */ - cp_parser_declaration (parser); + cp_parser_declaration (parser, NULL_TREE); if (parser->oacc_routine && !parser->oacc_routine->fndecl_seen) cp_ensure_no_oacc_routine (parser);
Hi, For Objective-C++/C, we cater for the possibility that a class interface (@interface) might be preceded by prefix attributes. In the case of Objective-C++, the reference implementation (a.k.a. clang) also allows (and combines) prefix attributes that precede a linkage specification (but only on a single decl). Some discussion with Nathan here: https://gcc.gnu.org/pipermail/gcc/2020-October/234057.html The upshot is that clang’s behaviour is inconsistent (I can file a bug, I guess) - but since what is “well-formed” for Objective-C is defined in reality by what clang accepts - there is a body of code out there that depends on the behaviour (some variant of Hyrum’s law, or corollary to it, perhaps?). Inability to parse code including these patterns is blocking progress in modernising GNU Objective-C.. so I need to find a way forward. The compromise made here is to accept the sequence when parsing for Objective-C++, and to warn** that the attributes are discarded otherwise. This seems to me to be an improvement in diagnostics for regular C++ (since it now says something pertinent to the actual problem and does the 'same as usual' when encountering an unhandled attribute). Tested across the Darwin patch, and on x86_64-linux-gnu, OK for master? thanks Iain ** trivially, that could be an error instead - but it seems we usually warn for unrecognised attributes. —— commit message For Objective-C++, this combines prefix attributes from before and after top level linkage specs. The "reference implementation" for Objective-C++ allows this, and system headers depend on it. e.g. __attribute__((__deprecated__)) extern "C" __attribute__((__visibility__("default"))) @interface MyClass ... @end Would consider the list of prefix attributes to the interface for MyClass to include both the visibility and deprecated ones. When we are compiling regular C++, this emits a warning and discards any prefix attributes before a linkage spec. gcc/cp/ChangeLog: * parser.c (cp_parser_declaration): Unless we are compiling for Ojective-C++, warn about and discard any attributes that prefix a linkage specification. --- gcc/cp/parser.c | 71 +++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 57 insertions(+), 14 deletions(-)