[C PATCH] Improve error on attrs after declarator in a fndef (PR c/60915)

[C PATCH] Improve error on attrs after declarator in a fndef (PR c/60915)

[Marek Polacek]

This PR is about not very clear error message when one tries to 
add attributes *after* the declarator in a function definition.
cc1plus already handles this well, so I used the same message.

Regtested/bootstrapped on x86_64-linux, ok for trunk?

2014-04-24  Marek Polacek  <polacek@redhat.com>

	PR c/60915
	* c-parser.c (c_parser_declaration_or_fndef): Give better error if
	function-definition has an attribute after the declarator.

	* gcc.dg/pr60915.c: New test.

diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 0deab84..a76a7cc 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -1688,7 +1688,19 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
 	  if (c_parser_next_token_is_keyword (parser, RID_ASM))
 	    asm_name = c_parser_simple_asm_expr (parser);
 	  if (c_parser_next_token_is_keyword (parser, RID_ATTRIBUTE))
-	    postfix_attrs = c_parser_attributes (parser);
+	    {
+	      postfix_attrs = c_parser_attributes (parser);
+	      if (c_parser_next_token_is (parser, CPP_OPEN_BRACE))
+		{
+		  /* This means there is an attribute specifier after
+		     the declarator in a function definition.  Provide
+		     some more information for the user.  */
+		  error_at (here, "attributes are not allowed on a "
+			    "function-definition");
+		  c_parser_skip_to_end_of_block_or_statement (parser);
+		  return;
+		}
+	    }
 	  if (c_parser_next_token_is (parser, CPP_EQ))
 	    {
 	      tree d;
diff --git gcc/testsuite/gcc.dg/pr60915.c gcc/testsuite/gcc.dg/pr60915.c
index e69de29..73a6717 100644
--- gcc/testsuite/gcc.dg/pr60915.c
+++ gcc/testsuite/gcc.dg/pr60915.c
@@ -0,0 +1,7 @@
+/* PR c/60915 */
+/* { dg-do compile } */
+
+void /* { dg-error "attributes are not allowed on a function-definition" } */
+foo (void) __attribute__((__visibility__("default")))
+{
+}

	Marek

Re: [C PATCH] Improve error on attrs after declarator in a fndef (PR c/60915)

[Marc Glisse]

On Thu, 24 Apr 2014, Marek Polacek wrote:

> This PR is about not very clear error message when one tries to
> add attributes *after* the declarator in a function definition.
> cc1plus already handles this well, so I used the same message.

I thought you had an earlier version of the patch where, instead of 
"forbidden", you said "move the attribute" so users would know that 
writing the attribute *before* was fine? That sounded much better to me.

-- 
Marc Glisse

Re: [C PATCH] Improve error on attrs after declarator in a fndef (PR c/60915)

[Marek Polacek]

On Thu, Apr 24, 2014 at 08:25:52PM +0200, Marc Glisse wrote:
> On Thu, 24 Apr 2014, Marek Polacek wrote:
> 
> >This PR is about not very clear error message when one tries to
> >add attributes *after* the declarator in a function definition.
> >cc1plus already handles this well, so I used the same message.
> 
> I thought you had an earlier version of the patch where, instead of
> "forbidden", you said "move the attribute" so users would know that
> writing the attribute *before* was fine? That sounded much better to
> me.

That's right, I posted it here:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60915#c1
but in the end I thought it'll be better to stay in sync with C++.
Here's the version from the BZ with a testcase.  
So whatever people like more...

2014-04-24  Marek Polacek  <polacek@redhat.com>

	PR c/60915
	* c-parser.c (c_parser_declaration_or_fndef): Give better error if
	function-definition has an attribute after the declarator.

	* gcc.dg/pr60915.c: New test.

diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 0deab84..2e01464 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -1688,7 +1688,19 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
 	  if (c_parser_next_token_is_keyword (parser, RID_ASM))
 	    asm_name = c_parser_simple_asm_expr (parser);
 	  if (c_parser_next_token_is_keyword (parser, RID_ATTRIBUTE))
-	    postfix_attrs = c_parser_attributes (parser);
+	    {
+	      postfix_attrs = c_parser_attributes (parser);
+	      if (c_parser_next_token_is (parser, CPP_OPEN_BRACE))
+		{
+		  /* This means there is an attribute specifier after
+		     the declarator in a function definition.  Provide
+		     some more information for the user.  */
+		  error_at (here, "attributes should be specified before the "
+			    "declarator in a function definition");
+		  c_parser_skip_to_end_of_block_or_statement (parser);
+		  return;
+		}
+	    }
 	  if (c_parser_next_token_is (parser, CPP_EQ))
 	    {
 	      tree d;
diff --git gcc/testsuite/gcc.dg/pr60915.c gcc/testsuite/gcc.dg/pr60915.c
index e69de29..2ed0a5f 100644
--- gcc/testsuite/gcc.dg/pr60915.c
+++ gcc/testsuite/gcc.dg/pr60915.c
@@ -0,0 +1,7 @@
+/* PR c/60915 */
+/* { dg-do compile } */
+
+void /* { dg-error "attributes should be specified before the declarator in a function definition" } */
+foo (void) __attribute__((__visibility__("default")))
+{
+}

	Marek

Re: [C PATCH] Improve error on attrs after declarator in a fndef (PR c/60915)

[Jeff Law]

On 04/24/14 08:31, Marek Polacek wrote:
> This PR is about not very clear error message when one tries to
> add attributes *after* the declarator in a function definition.
> cc1plus already handles this well, so I used the same message.
>
> Regtested/bootstrapped on x86_64-linux, ok for trunk?
>
> 2014-04-24  Marek Polacek  <polacek@redhat.com>
>
> 	PR c/60915
> 	* c-parser.c (c_parser_declaration_or_fndef): Give better error if
> 	function-definition has an attribute after the declarator.
>
> 	* gcc.dg/pr60915.c: New test.
No strong opinions on which of the two is better.  Slight preference to 
sync with g++.  You choose.  Either is OK for the trunk.
jeff