Objective-C++ : Allow prefix attrs on linkage specs.

Message ID C6DEA1BA-5FF7-4417-AE4E-0D294BDF158E@sandoe.co.uk
State New
Headers show
Series
  • Objective-C++ : Allow prefix attrs on linkage specs.
Related show

Commit Message

Iain Sandoe Nov. 7, 2020, 3:11 p.m.
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(-)

-- 
2.24.1

Comments

Xionghu Luo via Gcc-patches Nov. 11, 2020, 4:16 a.m. | #1
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
Xionghu Luo via Gcc-patches Nov. 20, 2020, 10:35 p.m. | #2
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);

>
Iain Sandoe Nov. 23, 2020, 1:52 p.m. | #3
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
Xionghu Luo via Gcc-patches Nov. 23, 2020, 10:17 p.m. | #4
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
Iain Sandoe Nov. 28, 2020, 9:53 a.m. | #5
(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;
-- 
2.24.1
Xionghu Luo via Gcc-patches Nov. 30, 2020, 8:56 p.m. | #6
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

> 

>
Xionghu Luo via Gcc-patches Dec. 1, 2020, 8:33 p.m. | #7
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;

Patch

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);