Bug 60915 - confusing diagnostic from attribute on function definition
Summary: confusing diagnostic from attribute on function definition
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: unknown
: P3 normal
Target Milestone: 5.0
Assignee: Marek Polacek
URL: http://gcc.gnu.org/ml/gcc-patches/201...
Keywords: diagnostic, patch
Depends on:
Blocks:
 
Reported: 2014-04-21 14:40 UTC by Tom Tromey
Modified: 2014-05-01 07:41 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2014-04-30 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Tromey 2014-04-21 14:40:28 UTC
Consider this program:

int something(void) __attribute__((__visibility__("default")))
{
  return 23;
}


Compiling gives:

barimba. gcc --syntax-only t.c
t.c:2:1: error: expected ‘,’ or ‘;’ before ‘{’ token
 {
 ^


A few notes here:

First, while this is not the correct syntax, it seems natural
enough that I've written it several times by mistake now.
Is there any chance it could be blessed?

Second, I think the diagnostic could be improved.  For example
it could mention that an attribute is not valid in this position
on a definition, and suggest an alternative.

Third, the manual could use an example of how to do this.
Comment 1 Marek Polacek 2014-04-23 14:13:10 UTC
(In reply to Tom Tromey from comment #0)
> First, while this is not the correct syntax, it seems natural
> enough that I've written it several times by mistake now.
> Is there any chance it could be blessed?

It looks like it.  Quoting from http://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html#Attribute-Syntax: "An attribute specifier list may, in future, be permitted to appear after the declarator in a function definition (before any old-style parameter declarations or the function body)."

> Second, I think the diagnostic could be improved.  For example
> it could mention that an attribute is not valid in this position
> on a definition, and suggest an alternative.

Yep.  The following (untested) patch should bring hopefully better error message; do you agree with the wording?

--- a/gcc/c/c-parser.c
+++ b/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;

This would of course go away if we decide to support attribute specifier after the declarator in a function definition.

> Third, the manual could use an example of how to do this.

Yeah, an example never hurts.
Comment 2 Marek Polacek 2014-04-24 14:31:51 UTC
Patch posted:
http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01556.html
Comment 3 Marek Polacek 2014-04-30 06:17:46 UTC
So mine.
Comment 4 Marek Polacek 2014-05-01 07:40:57 UTC
Author: mpolacek
Date: Thu May  1 07:40:26 2014
New Revision: 209975

URL: http://gcc.gnu.org/viewcvs?rev=209975&root=gcc&view=rev
Log:
	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.

Added:
    trunk/gcc/testsuite/gcc.dg/pr60915.c
Modified:
    trunk/gcc/c/ChangeLog
    trunk/gcc/c/c-parser.c
    trunk/gcc/testsuite/ChangeLog
Comment 5 Marek Polacek 2014-05-01 07:41:34 UTC
Fixed.