* [PATCH C] Fix pr44517 @ 2010-06-22 8:14 Shujing Zhao 2010-06-22 8:49 ` Manuel López-Ibáñez 2010-06-22 13:36 ` Joseph S. Myers 0 siblings, 2 replies; 14+ messages in thread From: Shujing Zhao @ 2010-06-22 8:14 UTC (permalink / raw) To: GCC Patches Cc: Joseph S. Myers, Manuel López-Ibáñez, Paolo Carlini [-- Attachment #1: Type: text/plain, Size: 445 bytes --] Hi, This patch tries to improve diagnostic for wrong type name in function declaration. It also changes the algorithm of function c_parser_parms_list_declarator. If one of parameter declaration is wrong, c_parser_parms_list_declarator would return NULL, not an empty parameter information struct. A test case is added to test this change. Bootstrapped and tested on i686-pc-linux-gnu with enabled c, objc and c++. Is it ok? Thanks Pearly [-- Attachment #2: ChangeLog --] [-- Type: text/plain, Size: 438 bytes --] gcc/ 2010-06-21 Shujing Zhao <pearly.zhao@oracle.com> PR c/44517 * c-parser.c (c_parser_parms_list_declarator): Return NULL if the parameters are not good. (c_parser_parameter_declaration): Error unknown type name if the parameter can't start declaration specifiers. gcc/testsuite/ 2010-06-22 Shujing Zhao <pearly.zhao@oracle.com> PR c/44517 * gcc.dg/pr44517.c: New. * gcc.dg/noncompile/990416-1.c: Adjust expected error. [-- Attachment #3: pr44517.patch --] [-- Type: text/x-patch, Size: 4090 bytes --] Index: c-parser.c =================================================================== --- c-parser.c (revision 160889) +++ c-parser.c (working copy) @@ -2706,7 +2706,7 @@ c_parser_parms_declarator (c_parser *par static struct c_arg_info * c_parser_parms_list_declarator (c_parser *parser, tree attrs) { - bool good_parm = false; + bool good_parm = true; /* ??? Following the old parser, forward parameter declarations may use abstract declarators, and if no real parameter declarations follow the forward declarations then this is not diagnosed. Also @@ -2758,11 +2758,10 @@ c_parser_parms_list_declarator (c_parser /* Parse a parameter. */ struct c_parm *parm = c_parser_parameter_declaration (parser, attrs); attrs = NULL_TREE; - if (parm != NULL) - { - good_parm = true; - push_parm_decl (parm); - } + if (parm == NULL) + good_parm = false; + else + push_parm_decl (parm); if (c_parser_next_token_is (parser, CPP_SEMICOLON)) { tree new_attrs; @@ -2777,17 +2776,7 @@ c_parser_parms_list_declarator (c_parser if (good_parm) return get_parm_info (false); else - { - struct c_arg_info *ret - = XOBNEW (&parser_obstack, struct c_arg_info); - ret->parms = 0; - ret->tags = 0; - ret->types = 0; - ret->others = 0; - ret->pending_sizes = 0; - ret->had_vla_unspec = 0; - return ret; - } + return NULL; } if (!c_parser_require (parser, CPP_COMMA, "expected %<;%>, %<,%> or %<)%>")) @@ -2805,17 +2794,7 @@ c_parser_parms_list_declarator (c_parser if (good_parm) return get_parm_info (true); else - { - struct c_arg_info *ret - = XOBNEW (&parser_obstack, struct c_arg_info); - ret->parms = 0; - ret->tags = 0; - ret->types = 0; - ret->others = 0; - ret->pending_sizes = 0; - ret->had_vla_unspec = 0; - return ret; - } + return false; } else { @@ -2843,8 +2822,12 @@ c_parser_parameter_declaration (c_parser { /* ??? In some Objective-C cases '...' isn't applicable so there should be a different message. */ - c_parser_error (parser, - "expected declaration specifiers or %<...%>"); + c_token *token = c_parser_peek_token (parser); + if (parser->error) + return NULL; + parser->error = true; + c_parser_set_source_position_from_token (token); + error ("unknown type name %qE", token->value); c_parser_skip_to_end_of_parameter (parser); return NULL; } Index: testsuite/gcc.dg/pr44517.c =================================================================== --- testsuite/gcc.dg/pr44517.c (revision 0) +++ testsuite/gcc.dg/pr44517.c (revision 0) @@ -0,0 +1,10 @@ +/* PR c/44517 */ +/* { dg-do compile } */ +/* { dg-options "" } */ +int foo(int x, pid_t y, long z, in t) { /* { dg-error "unknown type name.*pid_t|unknown type name.*in" } */ + return x + y + z + t; +} + +int bar(int x, lon y, long z, ...){ /* { dg-error "unknown type name" } */ + return; +} Index: testsuite/gcc.dg/noncompile/990416-1.c =================================================================== --- testsuite/gcc.dg/noncompile/990416-1.c (revision 160889) +++ testsuite/gcc.dg/noncompile/990416-1.c (working copy) @@ -2,11 +2,11 @@ extern void *memcpy (void *, const void typedef int word_type; static void -copy_reg (unsigned int reg, frame_state *udata, /* { dg-error "parse|syntax|expected" } */ - frame_state *target_udata) /* { dg-error "expected" } */ +copy_reg (unsigned int reg, frame_state *udata, /* { dg-error "unknown type name" } */ + frame_state *target_udata) /* { dg-error "unknown type name" } */ { - word_type *preg = get_reg_addr (reg, udata, 0); /* { dg-error "undeclared|function|without a cast" } */ - word_type *ptreg = get_reg_addr (reg, target_udata, 0); /* { dg-error "undeclared|without a cast" } */ + word_type *preg = ge_reg_addr (reg, udata, 0); + word_type *ptreg = ge_reg_addr (reg, target_udata, 0); memcpy (ptreg, preg, __builtin_dwarf_reg_size (reg)); } ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH C] Fix pr44517 2010-06-22 8:14 [PATCH C] Fix pr44517 Shujing Zhao @ 2010-06-22 8:49 ` Manuel López-Ibáñez 2010-06-22 9:57 ` Shujing Zhao 2010-06-22 13:36 ` Joseph S. Myers 1 sibling, 1 reply; 14+ messages in thread From: Manuel López-Ibáñez @ 2010-06-22 8:49 UTC (permalink / raw) To: Shujing Zhao; +Cc: GCC Patches, Joseph S. Myers, Paolo Carlini On 22 June 2010 09:05, Shujing Zhao <pearly.zhao@oracle.com> wrote: > Hi, > > This patch tries to improve diagnostic for wrong type name in function > declaration. It also changes the algorithm of function > c_parser_parms_list_declarator. If one of parameter declaration is wrong, > c_parser_parms_list_declarator would return NULL, not an empty parameter > information struct. A test case is added to test this change. > Great! One minor nit: - } + return false; } This should be return NULL. Don't need to send a new patch for this. Just wait for approval. Manuel. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH C] Fix pr44517 2010-06-22 8:49 ` Manuel López-Ibáñez @ 2010-06-22 9:57 ` Shujing Zhao 2010-06-22 10:00 ` Manuel López-Ibáñez 0 siblings, 1 reply; 14+ messages in thread From: Shujing Zhao @ 2010-06-22 9:57 UTC (permalink / raw) To: Manuel López-Ibáñez Cc: GCC Patches, Joseph S. Myers, Paolo Carlini On 06/22/2010 03:40 PM, Manuel López-Ibáñez wrote: > On 22 June 2010 09:05, Shujing Zhao <pearly.zhao@oracle.com> wrote: >> Hi, >> >> This patch tries to improve diagnostic for wrong type name in function >> declaration. It also changes the algorithm of function >> c_parser_parms_list_declarator. If one of parameter declaration is wrong, >> c_parser_parms_list_declarator would return NULL, not an empty parameter >> information struct. A test case is added to test this change. >> > > Great! One minor nit: > > - } > + return false; > } > > This should be return NULL. > > Don't need to send a new patch for this. Just wait for approval. Oops, sorry. Gcc didn't detect it, is it because both 'false' and 'NULL' are 0? is it a bug? Thanks Pearly ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH C] Fix pr44517 2010-06-22 9:57 ` Shujing Zhao @ 2010-06-22 10:00 ` Manuel López-Ibáñez 0 siblings, 0 replies; 14+ messages in thread From: Manuel López-Ibáñez @ 2010-06-22 10:00 UTC (permalink / raw) To: Shujing Zhao; +Cc: GCC Patches, Joseph S. Myers, Paolo Carlini On 22 June 2010 11:04, Shujing Zhao <pearly.zhao@oracle.com> wrote: > On 06/22/2010 03:40 PM, Manuel López-Ibáńez wrote: >> >> On 22 June 2010 09:05, Shujing Zhao <pearly.zhao@oracle.com> wrote: >>> >>> Hi, >>> >>> This patch tries to improve diagnostic for wrong type name in function >>> declaration. It also changes the algorithm of function >>> c_parser_parms_list_declarator. If one of parameter declaration is wrong, >>> c_parser_parms_list_declarator would return NULL, not an empty parameter >>> information struct. A test case is added to test this change. >>> >> >> Great! One minor nit: >> >> - } >> + return false; >> } >> >> This should be return NULL. >> >> Don't need to send a new patch for this. Just wait for approval. > > Oops, sorry. Gcc didn't detect it, is it because both 'false' and 'NULL' are > 0? is it a bug? It isn't a bug. The behaviour should be the same. But for the sake of clarity, we should return NULL for pointers and false only for bool. Cheers, Manuel. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH C] Fix pr44517 2010-06-22 8:14 [PATCH C] Fix pr44517 Shujing Zhao 2010-06-22 8:49 ` Manuel López-Ibáñez @ 2010-06-22 13:36 ` Joseph S. Myers 2010-06-23 9:17 ` Shujing Zhao 1 sibling, 1 reply; 14+ messages in thread From: Joseph S. Myers @ 2010-06-22 13:36 UTC (permalink / raw) To: Shujing Zhao Cc: GCC Patches, Manuel López-Ibáñez, Paolo Carlini On Tue, 22 Jun 2010, Shujing Zhao wrote: > Hi, > > This patch tries to improve diagnostic for wrong type name in function > declaration. It also changes the algorithm of function > c_parser_parms_list_declarator. If one of parameter declaration is wrong, > c_parser_parms_list_declarator would return NULL, not an empty parameter > information struct. A test case is added to test this change. You are changing the semantics of the variable good_parm from meaning "there was at least one good parameter" to "there were no bad parameters". Now, such a change should be accompanied by a change of name (e.g. to bad_parm, also reversing the sense of the variable). There was a previous invariant that get_pending_sizes would be called after any parameters were parsed, either directly or via get_parm_info because good_parm would be set, and with this patch, this invariant is no longer maintained. This is unsafe; you need to run this cleanup, whatever you then return from c_parser_parms_list_declarator. Consider for example the testcase: void foo(int n, int a[n], pid_t x); void bar() {} (related to gcc.dg/noncompile/pr35444-*.c). With your patch, this testcase (which should be added to the next revision of the patch) gets an ICE, because you lost the call to get_pending_sizes via get_parm_info. > + error ("unknown type name %qE", token->value); I don't think %qE is appropriate here if the token is not an identifier. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH C] Fix pr44517 2010-06-22 13:36 ` Joseph S. Myers @ 2010-06-23 9:17 ` Shujing Zhao 2010-06-23 9:31 ` Manuel López-Ibáñez 2010-06-23 12:24 ` Joseph S. Myers 0 siblings, 2 replies; 14+ messages in thread From: Shujing Zhao @ 2010-06-23 9:17 UTC (permalink / raw) To: Joseph S. Myers Cc: GCC Patches, Manuel López-Ibáñez, Paolo Carlini [-- Attachment #1: Type: text/plain, Size: 2465 bytes --] On 06/22/2010 08:55 PM, Joseph S. Myers wrote: > On Tue, 22 Jun 2010, Shujing Zhao wrote: > >> Hi, >> >> This patch tries to improve diagnostic for wrong type name in function >> declaration. It also changes the algorithm of function >> c_parser_parms_list_declarator. If one of parameter declaration is wrong, >> c_parser_parms_list_declarator would return NULL, not an empty parameter >> information struct. A test case is added to test this change. > > You are changing the semantics of the variable good_parm from meaning > "there was at least one good parameter" to "there were no bad parameters". > Now, such a change should be accompanied by a change of name (e.g. to > bad_parm, also reversing the sense of the variable). > > There was a previous invariant that get_pending_sizes would be called > after any parameters were parsed, either directly or via get_parm_info > because good_parm would be set, and with this patch, this invariant is no > longer maintained. This is unsafe; you need to run this cleanup, whatever > you then return from c_parser_parms_list_declarator. Consider for example > the testcase: > > void foo(int n, int a[n], pid_t x); > void bar() {} > > (related to gcc.dg/noncompile/pr35444-*.c). With your patch, this > testcase (which should be added to the next revision of the patch) gets an > ICE, because you lost the call to get_pending_sizes via get_parm_info. > You are right! Yes, If append the above test case to the end of pr44517.c, it gets an ICE. The above problem is fixed at the updated patch and the test case is moved to the directory gcc.dg/noncompile/. >> + error ("unknown type name %qE", token->value); > > I don't think %qE is appropriate here if the token is not an identifier. > Yes. But the problem is that token->id_kind is C_ID_ID and token->value is IDENTIFIER_NODE. At function c_lex_one_token, the token->id_kind is always be set to C_ID_ID if it is not the other identifier. Look at enum c_id_kind, C_ID_ID is "an ordinary identifier" and there is an "not an identifier" C_ID_NONE, but it never be really set. If token is CPP_NAME, and it was not declared as some type name, the token->id_kind should be set C_ID_NONE. But where should C_ID_ID be set? I think that is the problem of c_lex_one_token, not the message format. I can't give a solution for that issue now. Does this patch can be committed firstly? Retested on i686-pc-linux-gnu. Is it ok? Thanks Pearly [-- Attachment #2: ChangeLog --] [-- Type: text/plain, Size: 452 bytes --] gcc/ 2010-06-23 Shujing Zhao <pearly.zhao@oracle.com> PR c/44517 * c-parser.c (c_parser_parms_list_declarator): Return NULL if one of parameters are not good. (c_parser_parameter_declaration): Error unknown type name if the parameter can't start declaration specifiers. gcc/testsuite/ 2010-06-23 Shujing Zhao <pearly.zhao@oracle.com> PR c/44517 * gcc.dg/noncompile/pr44517.c: New. * gcc.dg/noncompile/990416-1.c: Adjust expected error. [-- Attachment #3: pr44517.patch --] [-- Type: text/x-patch, Size: 4674 bytes --] Index: c-parser.c =================================================================== --- c-parser.c (revision 160889) +++ c-parser.c (working copy) @@ -2706,7 +2706,7 @@ c_parser_parms_declarator (c_parser *par static struct c_arg_info * c_parser_parms_list_declarator (c_parser *parser, tree attrs) { - bool good_parm = false; + bool bad_parm = false; /* ??? Following the old parser, forward parameter declarations may use abstract declarators, and if no real parameter declarations follow the forward declarations then this is not diagnosed. Also @@ -2758,11 +2758,10 @@ c_parser_parms_list_declarator (c_parser /* Parse a parameter. */ struct c_parm *parm = c_parser_parameter_declaration (parser, attrs); attrs = NULL_TREE; - if (parm != NULL) - { - good_parm = true; - push_parm_decl (parm); - } + if (parm == NULL) + bad_parm = true; + else + push_parm_decl (parm); if (c_parser_next_token_is (parser, CPP_SEMICOLON)) { tree new_attrs; @@ -2774,20 +2773,13 @@ c_parser_parms_list_declarator (c_parser if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN)) { c_parser_consume_token (parser); - if (good_parm) - return get_parm_info (false); - else + if (bad_parm) { - struct c_arg_info *ret - = XOBNEW (&parser_obstack, struct c_arg_info); - ret->parms = 0; - ret->tags = 0; - ret->types = 0; - ret->others = 0; - ret->pending_sizes = 0; - ret->had_vla_unspec = 0; - return ret; + get_pending_sizes (); + return NULL; } + else + return get_parm_info (false); } if (!c_parser_require (parser, CPP_COMMA, "expected %<;%>, %<,%> or %<)%>")) @@ -2802,20 +2794,13 @@ c_parser_parms_list_declarator (c_parser if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN)) { c_parser_consume_token (parser); - if (good_parm) - return get_parm_info (true); - else + if (bad_parm) { - struct c_arg_info *ret - = XOBNEW (&parser_obstack, struct c_arg_info); - ret->parms = 0; - ret->tags = 0; - ret->types = 0; - ret->others = 0; - ret->pending_sizes = 0; - ret->had_vla_unspec = 0; - return ret; + get_pending_sizes (); + return NULL; } + else + return get_parm_info (true); } else { @@ -2843,8 +2828,12 @@ c_parser_parameter_declaration (c_parser { /* ??? In some Objective-C cases '...' isn't applicable so there should be a different message. */ - c_parser_error (parser, - "expected declaration specifiers or %<...%>"); + c_token *token = c_parser_peek_token (parser); + if (parser->error) + return NULL; + parser->error = true; + c_parser_set_source_position_from_token (token); + error ("unknown type name %qE", token->value); c_parser_skip_to_end_of_parameter (parser); return NULL; } Index: testsuite/gcc.dg/noncompile/pr44517.c =================================================================== --- testsuite/gcc.dg/noncompile/pr44517.c (revision 0) +++ testsuite/gcc.dg/noncompile/pr44517.c (revision 0) @@ -0,0 +1,11 @@ +/* PR c/44517: Improve diagnostic for misspelled typename in function declaration. */ +int foo(int x, pid_t y, long z, in t) { /* { dg-error "unknown type name.*pid_t|unknown type name.*in" } */ + return x + y + z + t; +} + +int bar(int x, lon y, long z, ...){ /* { dg-error "unknown type name" { target *-*-* } 8 } */ + return; +} + +void foo(int n, int a[n], pid_t x); /* { dg-error "unknown type name" { target *-*-* } 12 } */ +void bar() {}; Index: testsuite/gcc.dg/noncompile/990416-1.c =================================================================== --- testsuite/gcc.dg/noncompile/990416-1.c (revision 160889) +++ testsuite/gcc.dg/noncompile/990416-1.c (working copy) @@ -2,11 +2,11 @@ extern void *memcpy (void *, const void typedef int word_type; static void -copy_reg (unsigned int reg, frame_state *udata, /* { dg-error "parse|syntax|expected" } */ - frame_state *target_udata) /* { dg-error "expected" } */ +copy_reg (unsigned int reg, frame_state *udata, /* { dg-error "unknown type name" } */ + frame_state *target_udata) /* { dg-error "unknown type name" } */ { - word_type *preg = get_reg_addr (reg, udata, 0); /* { dg-error "undeclared|function|without a cast" } */ - word_type *ptreg = get_reg_addr (reg, target_udata, 0); /* { dg-error "undeclared|without a cast" } */ + word_type *preg = ge_reg_addr (reg, udata, 0); + word_type *ptreg = ge_reg_addr (reg, target_udata, 0); memcpy (ptreg, preg, __builtin_dwarf_reg_size (reg)); } ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH C] Fix pr44517 2010-06-23 9:17 ` Shujing Zhao @ 2010-06-23 9:31 ` Manuel López-Ibáñez 2010-06-23 9:55 ` Shujing Zhao 2010-06-23 12:24 ` Joseph S. Myers 1 sibling, 1 reply; 14+ messages in thread From: Manuel López-Ibáñez @ 2010-06-23 9:31 UTC (permalink / raw) To: Shujing Zhao; +Cc: Joseph S. Myers, GCC Patches, Paolo Carlini Nice improvement! Some unimportant comments: @@ -2843,8 +2828,12 @@ c_parser_parameter_declaration (c_parser { /* ??? In some Objective-C cases '...' isn't applicable so there should be a different message. */ - c_parser_error (parser, - "expected declaration specifiers or %<...%>"); + c_token *token = c_parser_peek_token (parser); + if (parser->error) + return NULL; + parser->error = true; + c_parser_set_source_position_from_token (token); + error ("unknown type name %qE", token->value); c_parser_skip_to_end_of_parameter (parser); return NULL; } The ??? comment does not make sense anymore (and without any example wasn't very useful to start with), I would propose to remove it. BTW, do you know that there is a error_at (LOCATION) function? I am not sure whether here using that is more correct (or efficient) than c_parser_set_source_position_from_token but just to let you know for future patches. +/* PR c/44517: Improve diagnostic for misspelled typename in function declaration. */ +int foo(int x, pid_t y, long z, in t) { /* { dg-error "unknown type name.*pid_t|unknown type name.*in" } */ + return x + y + z + t; +} + +int bar(int x, lon y, long z, ...){ /* { dg-error "unknown type name" { target *-*-* } 8 } */ + return; +} + +void foo(int n, int a[n], pid_t x); /* { dg-error "unknown type name" { target *-*-* } 12 } */ +void bar() {}; What is the default dg action for tests without dg-do directive? Also, I am not sure how these directives are working correctly, because the complete format is: { dg-error PATTERN COMMENT { TARGET_SPEC } LINE } so the comment is missing. As far as I know, you can drop arguments from right to left but not, for example, put a LINE and not a TARGET or COMMENT. (Unfortunate, because LINE is probably the second most used after PATTERN). BTW, you do not need LINE if the error is given in the same line as the directive dg-error. Cheers, Manuel. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH C] Fix pr44517 2010-06-23 9:31 ` Manuel López-Ibáñez @ 2010-06-23 9:55 ` Shujing Zhao 0 siblings, 0 replies; 14+ messages in thread From: Shujing Zhao @ 2010-06-23 9:55 UTC (permalink / raw) To: Manuel López-Ibáñez Cc: Joseph S. Myers, GCC Patches, Paolo Carlini [-- Attachment #1: Type: text/plain, Size: 2789 bytes --] On 06/23/2010 04:56 PM, Manuel López-Ibáñez wrote: > Nice improvement! Some unimportant comments: > > @@ -2843,8 +2828,12 @@ c_parser_parameter_declaration (c_parser > { > /* ??? In some Objective-C cases '...' isn't applicable so there > should be a different message. */ > - c_parser_error (parser, > - "expected declaration specifiers or %<...%>"); > + c_token *token = c_parser_peek_token (parser); > + if (parser->error) > + return NULL; > + parser->error = true; > + c_parser_set_source_position_from_token (token); > + error ("unknown type name %qE", token->value); > c_parser_skip_to_end_of_parameter (parser); > return NULL; > } > > The ??? comment does not make sense anymore (and without any example > wasn't very useful to start with), I would propose to remove it. Yes, you are right. Remove it from the updated patch. BTW, > do you know that there is a error_at (LOCATION) function? I am not > sure whether here using that is more correct (or efficient) than > c_parser_set_source_position_from_token but just to let you know for > future patches. > If it uses error_at, the code would be changed from + c_parser_set_source_position_from_token (token); + error ("unknown type name %qE", token->value); to + if (token->type != CPP_EOF) + { + error_at (token->location, "unknown type name %qE", token->value); + } + else + error ("unknown type name %qE", token->value); I think using c_parser_set_source_position_from_token is better. > > +/* PR c/44517: Improve diagnostic for misspelled typename in function > declaration. */ > +int foo(int x, pid_t y, long z, in t) { /* { dg-error "unknown type > name.*pid_t|unknown type name.*in" } */ > + return x + y + z + t; > +} > + > +int bar(int x, lon y, long z, ...){ /* { dg-error "unknown type name" > { target *-*-* } 8 } */ > + return; > +} > + > +void foo(int n, int a[n], pid_t x); /* { dg-error "unknown type name" > { target *-*-* } 12 } */ > +void bar() {}; > > What is the default dg action for tests without dg-do directive? Also, > I am not sure how these directives are working correctly, because the > complete format is: > > { dg-error PATTERN COMMENT { TARGET_SPEC } LINE } > > so the comment is missing. As far as I know, you can drop arguments > from right to left but not, for example, put a LINE and not a TARGET > or COMMENT. (Unfortunate, because LINE is probably the second most > used after PATTERN). BTW, you do not need LINE if the error is given > in the same line as the directive dg-error. Thanks. I also wonder how they are passed? Maybe it is caused the ".*" is used at the first dg-error directive. Using "'" directly to quote the unknown type name solves this issue. Thanks Pearly [-- Attachment #2: pr44517.patch --] [-- Type: text/x-patch, Size: 4730 bytes --] Index: c-parser.c =================================================================== --- c-parser.c (revision 160889) +++ c-parser.c (working copy) @@ -2706,7 +2706,7 @@ c_parser_parms_declarator (c_parser *par static struct c_arg_info * c_parser_parms_list_declarator (c_parser *parser, tree attrs) { - bool good_parm = false; + bool bad_parm = false; /* ??? Following the old parser, forward parameter declarations may use abstract declarators, and if no real parameter declarations follow the forward declarations then this is not diagnosed. Also @@ -2758,11 +2758,10 @@ c_parser_parms_list_declarator (c_parser /* Parse a parameter. */ struct c_parm *parm = c_parser_parameter_declaration (parser, attrs); attrs = NULL_TREE; - if (parm != NULL) - { - good_parm = true; - push_parm_decl (parm); - } + if (parm == NULL) + bad_parm = true; + else + push_parm_decl (parm); if (c_parser_next_token_is (parser, CPP_SEMICOLON)) { tree new_attrs; @@ -2774,20 +2773,13 @@ c_parser_parms_list_declarator (c_parser if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN)) { c_parser_consume_token (parser); - if (good_parm) - return get_parm_info (false); - else + if (bad_parm) { - struct c_arg_info *ret - = XOBNEW (&parser_obstack, struct c_arg_info); - ret->parms = 0; - ret->tags = 0; - ret->types = 0; - ret->others = 0; - ret->pending_sizes = 0; - ret->had_vla_unspec = 0; - return ret; + get_pending_sizes (); + return NULL; } + else + return get_parm_info (false); } if (!c_parser_require (parser, CPP_COMMA, "expected %<;%>, %<,%> or %<)%>")) @@ -2802,20 +2794,13 @@ c_parser_parms_list_declarator (c_parser if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN)) { c_parser_consume_token (parser); - if (good_parm) - return get_parm_info (true); - else + if (bad_parm) { - struct c_arg_info *ret - = XOBNEW (&parser_obstack, struct c_arg_info); - ret->parms = 0; - ret->tags = 0; - ret->types = 0; - ret->others = 0; - ret->pending_sizes = 0; - ret->had_vla_unspec = 0; - return ret; + get_pending_sizes (); + return NULL; } + else + return get_parm_info (true); } else { @@ -2841,10 +2826,12 @@ c_parser_parameter_declaration (c_parser bool dummy = false; if (!c_parser_next_token_starts_declspecs (parser)) { - /* ??? In some Objective-C cases '...' isn't applicable so there - should be a different message. */ - c_parser_error (parser, - "expected declaration specifiers or %<...%>"); + c_token *token = c_parser_peek_token (parser); + if (parser->error) + return NULL; + parser->error = true; + c_parser_set_source_position_from_token (token); + error ("unknown type name %qE", token->value); c_parser_skip_to_end_of_parameter (parser); return NULL; } Index: testsuite/gcc.dg/noncompile/pr44517.c =================================================================== --- testsuite/gcc.dg/noncompile/pr44517.c (revision 0) +++ testsuite/gcc.dg/noncompile/pr44517.c (revision 0) @@ -0,0 +1,11 @@ +/* PR c/44517: Improve diagnostic for misspelled typename in function declaration. */ +int foo(int x, pid_t y, long z, in t) { /* { dg-error "unknown type name 'pid_t'|unknown type name 'in'" } */ + return x + y + z + t; +} + +int bar(int x, lon y, long z, ...){ /* { dg-error "unknown type name 'lon'" } */ + return; +} + +void foo(int n, int a[n], pid_t x); /* { dg-error "unknown type name 'pid_t'" } */ +void bar() {}; Index: testsuite/gcc.dg/noncompile/990416-1.c =================================================================== --- testsuite/gcc.dg/noncompile/990416-1.c (revision 160889) +++ testsuite/gcc.dg/noncompile/990416-1.c (working copy) @@ -2,11 +2,11 @@ extern void *memcpy (void *, const void typedef int word_type; static void -copy_reg (unsigned int reg, frame_state *udata, /* { dg-error "parse|syntax|expected" } */ - frame_state *target_udata) /* { dg-error "expected" } */ +copy_reg (unsigned int reg, frame_state *udata, /* { dg-error "unknown type name" } */ + frame_state *target_udata) /* { dg-error "unknown type name" } */ { - word_type *preg = get_reg_addr (reg, udata, 0); /* { dg-error "undeclared|function|without a cast" } */ - word_type *ptreg = get_reg_addr (reg, target_udata, 0); /* { dg-error "undeclared|without a cast" } */ + word_type *preg = ge_reg_addr (reg, udata, 0); + word_type *ptreg = ge_reg_addr (reg, target_udata, 0); memcpy (ptreg, preg, __builtin_dwarf_reg_size (reg)); } ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH C] Fix pr44517 2010-06-23 9:17 ` Shujing Zhao 2010-06-23 9:31 ` Manuel López-Ibáñez @ 2010-06-23 12:24 ` Joseph S. Myers 2010-06-23 12:28 ` Manuel López-Ibáñez 1 sibling, 1 reply; 14+ messages in thread From: Joseph S. Myers @ 2010-06-23 12:24 UTC (permalink / raw) To: Shujing Zhao Cc: GCC Patches, Manuel López-Ibáñez, Paolo Carlini On Wed, 23 Jun 2010, Shujing Zhao wrote: > > > + error ("unknown type name %qE", token->value); > > > > I don't think %qE is appropriate here if the token is not an identifier. > > > Yes. But the problem is that token->id_kind is C_ID_ID and token->value is > IDENTIFIER_NODE. At function c_lex_one_token, the token->id_kind is always be > set to C_ID_ID if it is not the other identifier. Look at enum c_id_kind, > C_ID_ID is "an ordinary identifier" and there is an "not an identifier" > C_ID_NONE, but it never be really set. If token is CPP_NAME, and it was not > declared as some type name, the token->id_kind should be set C_ID_NONE. But > where should C_ID_ID be set? > I think that is the problem of c_lex_one_token, not the message format. I > can't give a solution for that issue now. Does this patch can be committed > firstly? I don't understand what you are saying. C_ID_NONE is for tokens that are not CPP_NAME at all. C_ID_ID is for a subset of CPP_NAME tokens. Tokens that are not CPP_NAME at all can have many different sorts of trees for token->value, which may not be appropriate for %qE. They may use NULL_TREE if no token value is needed at all. For example, the following test, which should be added to the next patch revision, segfaults with your latest patch applied because you are inappropriately using token->value when it is NULL. As I said, use it *only* for tokens that are some kind of identifier (CPP_NAME or CPP_KEYWORD); include testcases for both CPP_NAME and CPP_KEYWORD and for other kinds of token. void f(int a, *b); > +/* PR c/44517: Improve diagnostic for misspelled typename in function declaration. */ > +int foo(int x, pid_t y, long z, in t) { /* { dg-error "unknown type name 'pid_t'|unknown type name 'in'" } */ > + return x + y + z + t; > +} > + > +int bar(int x, lon y, long z, ...){ /* { dg-error "unknown type name 'lon'" } */ > + return; > +} > + > +void foo(int n, int a[n], pid_t x); /* { dg-error "unknown type name 'pid_t'" } */ > +void bar() {}; Use four different names for the four different functions. Remove the stray semicolon after your last function body. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH C] Fix pr44517 2010-06-23 12:24 ` Joseph S. Myers @ 2010-06-23 12:28 ` Manuel López-Ibáñez 2010-06-23 12:41 ` Joseph S. Myers 0 siblings, 1 reply; 14+ messages in thread From: Manuel López-Ibáñez @ 2010-06-23 12:28 UTC (permalink / raw) To: Joseph S. Myers; +Cc: Shujing Zhao, GCC Patches, Paolo Carlini On 23 June 2010 13:45, Joseph S. Myers <joseph@codesourcery.com> wrote: > token->value when it is NULL. As I said, use it *only* for tokens that > are some kind of identifier (CPP_NAME or CPP_KEYWORD); include testcases > for both CPP_NAME and CPP_KEYWORD and for other kinds of token. > > void f(int a, *b); What other types of tokens can be found at this point? How to print them properly? I guess for other types of tokens the "unknown type" message may not be appropriate at all. Perhaps for those cases, the old message: - c_parser_error (parser, - "expected declaration specifiers or %<...%>"); would still be appropriate. >> +/* PR c/44517: Improve diagnostic for misspelled typename in function declaration. */ >> +int foo(int x, pid_t y, long z, in t) { /* { dg-error "unknown type name 'pid_t'|unknown type name 'in'" } */ >> + return x + y + z + t; >> +} >> + >> +int bar(int x, lon y, long z, ...){ /* { dg-error "unknown type name 'lon'" } */ >> + return; >> +} >> + >> +void foo(int n, int a[n], pid_t x); /* { dg-error "unknown type name 'pid_t'" } */ >> +void bar() {}; > > Use four different names for the four different functions. Remove the > stray semicolon after your last function body. How can this test pass then? Perhaps it needs an explicit /* { dg-do compile } */ ? Manuel. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH C] Fix pr44517 2010-06-23 12:28 ` Manuel López-Ibáñez @ 2010-06-23 12:41 ` Joseph S. Myers 2010-06-24 10:50 ` Shujing Zhao 0 siblings, 1 reply; 14+ messages in thread From: Joseph S. Myers @ 2010-06-23 12:41 UTC (permalink / raw) To: Manuel López-Ibáñez Cc: Shujing Zhao, GCC Patches, Paolo Carlini [-- Attachment #1: Type: TEXT/PLAIN, Size: 2536 bytes --] On Wed, 23 Jun 2010, Manuel López-Ibáñez wrote: > On 23 June 2010 13:45, Joseph S. Myers <joseph@codesourcery.com> wrote: > > token->value when it is NULL. As I said, use it *only* for tokens that > > are some kind of identifier (CPP_NAME or CPP_KEYWORD); include testcases > > for both CPP_NAME and CPP_KEYWORD and for other kinds of token. > > > > void f(int a, *b); > > What other types of tokens can be found at this point? That is a matter for the patch submitter to assess and explain as part of justifying the patch; understanding the circumstances in which particular code is reached is an important part of patching the parser. > How to print them properly? c_parse_error deals with printing different kinds of tokens, although not necessarily optimally. > I guess for other types of tokens the "unknown type" message may not > be appropriate at all. Perhaps for those cases, the old message: > > - c_parser_error (parser, > - "expected declaration specifiers or %<...%>"); > > would still be appropriate. That might also be the case for keywords; saying that e.g. "goto" is an unknown type may not be appropriate. > >> +/* PR c/44517: Improve diagnostic for misspelled typename in function declaration. */ > >> +int foo(int x, pid_t y, long z, in t) { /* { dg-error "unknown type name 'pid_t'|unknown type name 'in'" } */ > >> + return x + y + z + t; > >> +} > >> + > >> +int bar(int x, lon y, long z, ...){ /* { dg-error "unknown type name 'lon'" } */ > >> + return; > >> +} > >> + > >> +void foo(int n, int a[n], pid_t x); /* { dg-error "unknown type name 'pid_t'" } */ > >> +void bar() {}; > > > > Use four different names for the four different functions. Remove the > > stray semicolon after your last function body. > > How can this test pass then? Perhaps it needs an explicit /* { dg-do > compile } */ ? No, that's the default. Probably the details of how syntactically erroneous function declarations are handled mean that conflicts do not end up being reported for conflicting types or multiple definitions (if one declaration cannot be parsed, it may not be possible to get a useful idea of whether the types conflict or not). But since the point of this test is not the details of whether duplicate declarations are diagnosed when those declarations are erroneous in other ways, different names should be used so the test doesn't depend on those details. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH C] Fix pr44517 2010-06-23 12:41 ` Joseph S. Myers @ 2010-06-24 10:50 ` Shujing Zhao 2010-06-24 11:56 ` Joseph S. Myers 0 siblings, 1 reply; 14+ messages in thread From: Shujing Zhao @ 2010-06-24 10:50 UTC (permalink / raw) To: Joseph S. Myers Cc: Manuel López-Ibáñez, GCC Patches, Paolo Carlini [-- Attachment #1: Type: text/plain, Size: 3130 bytes --] On 06/23/2010 08:23 PM, Joseph S. Myers wrote: > On Wed, 23 Jun 2010, Manuel López-Ibáñez wrote: > >> On 23 June 2010 13:45, Joseph S. Myers <joseph@codesourcery.com> wrote: >>> token->value when it is NULL. As I said, use it *only* for tokens that >>> are some kind of identifier (CPP_NAME or CPP_KEYWORD); include testcases >>> for both CPP_NAME and CPP_KEYWORD and for other kinds of token. >>> >>> void f(int a, *b); >> What other types of tokens can be found at this point? > > That is a matter for the patch submitter to assess and explain as part of > justifying the patch; understanding the circumstances in which particular > code is reached is an important part of patching the parser. > >> How to print them properly? > > c_parse_error deals with printing different kinds of tokens, although not > necessarily optimally. > >> I guess for other types of tokens the "unknown type" message may not >> be appropriate at all. Perhaps for those cases, the old message: >> >> - c_parser_error (parser, >> - "expected declaration specifiers or %<...%>"); >> >> would still be appropriate. > > That might also be the case for keywords; saying that e.g. "goto" is an > unknown type may not be appropriate. > >>>> +/* PR c/44517: Improve diagnostic for misspelled typename in function declaration. */ >>>> +int foo(int x, pid_t y, long z, in t) { /* { dg-error "unknown type name 'pid_t'|unknown type name 'in'" } */ >>>> + return x + y + z + t; >>>> +} >>>> + >>>> +int bar(int x, lon y, long z, ...){ /* { dg-error "unknown type name 'lon'" } */ >>>> + return; >>>> +} >>>> + >>>> +void foo(int n, int a[n], pid_t x); /* { dg-error "unknown type name 'pid_t'" } */ >>>> +void bar() {}; >>> Use four different names for the four different functions. Remove the >>> stray semicolon after your last function body. >> How can this test pass then? Perhaps it needs an explicit /* { dg-do >> compile } */ ? > > No, that's the default. Probably the details of how syntactically > erroneous function declarations are handled mean that conflicts do not end > up being reported for conflicting types or multiple definitions (if one > declaration cannot be parsed, it may not be possible to get a useful idea > of whether the types conflict or not). But since the point of this test > is not the details of whether duplicate declarations are diagnosed when > those declarations are erroneous in other ways, different names should be > used so the test doesn't depend on those details. > Thanks. According to the above discussion, if the token type is CPP_NAME and the followed token is not ')' or ',', the error message would be "unknown type name %qE". If the token type is CPP_NAME, but followed by ',' or ')', it looks like only parameter name is declared and declaration specifier is missed. The old message is more appropriate. If the token type is CPP_KEYWORD, such as 'goto', the old error message is appropriate. If the token type is the others, it would still use the old message. More cases are added to the test case to test this changes. Is it right? Thanks Pearly [-- Attachment #2: pr44517.patch --] [-- Type: text/x-patch, Size: 5348 bytes --] Index: c-parser.c =================================================================== --- c-parser.c (revision 160889) +++ c-parser.c (working copy) @@ -2706,7 +2706,7 @@ c_parser_parms_declarator (c_parser *par static struct c_arg_info * c_parser_parms_list_declarator (c_parser *parser, tree attrs) { - bool good_parm = false; + bool bad_parm = false; /* ??? Following the old parser, forward parameter declarations may use abstract declarators, and if no real parameter declarations follow the forward declarations then this is not diagnosed. Also @@ -2758,11 +2758,10 @@ c_parser_parms_list_declarator (c_parser /* Parse a parameter. */ struct c_parm *parm = c_parser_parameter_declaration (parser, attrs); attrs = NULL_TREE; - if (parm != NULL) - { - good_parm = true; - push_parm_decl (parm); - } + if (parm == NULL) + bad_parm = true; + else + push_parm_decl (parm); if (c_parser_next_token_is (parser, CPP_SEMICOLON)) { tree new_attrs; @@ -2774,20 +2773,13 @@ c_parser_parms_list_declarator (c_parser if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN)) { c_parser_consume_token (parser); - if (good_parm) - return get_parm_info (false); - else + if (bad_parm) { - struct c_arg_info *ret - = XOBNEW (&parser_obstack, struct c_arg_info); - ret->parms = 0; - ret->tags = 0; - ret->types = 0; - ret->others = 0; - ret->pending_sizes = 0; - ret->had_vla_unspec = 0; - return ret; + get_pending_sizes (); + return NULL; } + else + return get_parm_info (false); } if (!c_parser_require (parser, CPP_COMMA, "expected %<;%>, %<,%> or %<)%>")) @@ -2802,20 +2794,13 @@ c_parser_parms_list_declarator (c_parser if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN)) { c_parser_consume_token (parser); - if (good_parm) - return get_parm_info (true); - else + if (bad_parm) { - struct c_arg_info *ret - = XOBNEW (&parser_obstack, struct c_arg_info); - ret->parms = 0; - ret->tags = 0; - ret->types = 0; - ret->others = 0; - ret->pending_sizes = 0; - ret->had_vla_unspec = 0; - return ret; + get_pending_sizes (); + return NULL; } + else + return get_parm_info (true); } else { @@ -2841,10 +2826,22 @@ c_parser_parameter_declaration (c_parser bool dummy = false; if (!c_parser_next_token_starts_declspecs (parser)) { + c_token *token = c_parser_peek_token (parser); + if (parser->error) + return NULL; + c_parser_set_source_position_from_token (token); + if (token->type == CPP_NAME + && c_parser_peek_2nd_token (parser)->type != CPP_COMMA + && c_parser_peek_2nd_token (parser)->type != CPP_CLOSE_PAREN) + { + error ("unknown type name %qE", token->value); + parser->error = true; + } /* ??? In some Objective-C cases '...' isn't applicable so there should be a different message. */ - c_parser_error (parser, - "expected declaration specifiers or %<...%>"); + else + c_parser_error (parser, + "expected declaration specifiers or %<...%>"); c_parser_skip_to_end_of_parameter (parser); return NULL; } Index: testsuite/gcc.dg/noncompile/pr44517.c =================================================================== --- testsuite/gcc.dg/noncompile/pr44517.c (revision 0) +++ testsuite/gcc.dg/noncompile/pr44517.c (revision 0) @@ -0,0 +1,16 @@ +/* PR c/44517: Improve diagnostic for misspelled typename in function declaration. */ +int f1(int x, pid_t y, long z, in t) { /* { dg-error "unknown type name 'pid_t'|unknown type name 'in'" } */ + return x + y + z + t; +} + +int f2(int x, lon y, long z, ...){ /* { dg-error "unknown type name 'lon'" } */ + return; +} + +void f3(int n, int a[n], pid_t x); /* { dg-error "unknown type name 'pid_t'" } */ +void f4() {} +void f5(int a, *b); /* { dg-error "expected declaration specifiers or" } */ +void f6(int a, b); /* { dg-error "expected declaration specifiers or" } */ +void f7(int a, goto b); /* { dg-error "expected declaration specifiers or" } */ +void f8(int a, in goto); /* { dg-error "unknown type name 'in'" } */ +void f9(int a, in 1); /* { dg-error "unknown type name 'in'" } */ Index: testsuite/gcc.dg/noncompile/990416-1.c =================================================================== --- testsuite/gcc.dg/noncompile/990416-1.c (revision 160889) +++ testsuite/gcc.dg/noncompile/990416-1.c (working copy) @@ -2,11 +2,11 @@ extern void *memcpy (void *, const void typedef int word_type; static void -copy_reg (unsigned int reg, frame_state *udata, /* { dg-error "parse|syntax|expected" } */ - frame_state *target_udata) /* { dg-error "expected" } */ +copy_reg (unsigned int reg, frame_state *udata, /* { dg-error "unknown type name" } */ + frame_state *target_udata) /* { dg-error "unknown type name" } */ { - word_type *preg = get_reg_addr (reg, udata, 0); /* { dg-error "undeclared|function|without a cast" } */ - word_type *ptreg = get_reg_addr (reg, target_udata, 0); /* { dg-error "undeclared|without a cast" } */ + word_type *preg = ge_reg_addr (reg, udata, 0); + word_type *ptreg = ge_reg_addr (reg, target_udata, 0); memcpy (ptreg, preg, __builtin_dwarf_reg_size (reg)); } ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH C] Fix pr44517 2010-06-24 10:50 ` Shujing Zhao @ 2010-06-24 11:56 ` Joseph S. Myers 2010-06-25 10:02 ` Shujing Zhao 0 siblings, 1 reply; 14+ messages in thread From: Joseph S. Myers @ 2010-06-24 11:56 UTC (permalink / raw) To: Shujing Zhao Cc: Manuel López-Ibáñez, GCC Patches, Paolo Carlini On Thu, 24 Jun 2010, Shujing Zhao wrote: > Thanks. According to the above discussion, if the token type is CPP_NAME and > the followed token is not ')' or ',', the error message would be "unknown type > name %qE". If the token type is CPP_NAME, but followed by ',' or ')', it looks > like only parameter name is declared and declaration specifier is missed. The > old message is more appropriate. If the token type is CPP_KEYWORD, such as Rather than saying the old message is more appropriate, I'd say that you can't really tell the difference between (int, pid_t) where a type was undeclared and (int a, b) where a type name is needed before "b". So perhaps the messages should say that either a type is undeclared or a type is missing before a parameter name. (Actually, I'd guess that almost all function prototypes either name all parameters or no parameters, so in the first case it's a very good guess that pid_t is a type and in the second it's a very good guess that b is a parameter name. But that's definitely an enhancement for the future.) The patch is OK without further code changes needed but with one testcase fix: > +int f1(int x, pid_t y, long z, in t) { /* { dg-error "unknown type name 'pid_t'|unknown type name 'in'" } */ Please use two separate dg-error directives to match the two separate error messages. The second one would use the form with a line number, { dg-error "message" "test name" { target *-*-* } line-number }. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH C] Fix pr44517 2010-06-24 11:56 ` Joseph S. Myers @ 2010-06-25 10:02 ` Shujing Zhao 0 siblings, 0 replies; 14+ messages in thread From: Shujing Zhao @ 2010-06-25 10:02 UTC (permalink / raw) To: Joseph S. Myers Cc: Manuel López-Ibáñez, GCC Patches, Paolo Carlini [-- Attachment #1: Type: text/plain, Size: 1152 bytes --] On 06/24/2010 07:20 PM, Joseph S. Myers wrote: > Rather than saying the old message is more appropriate, I'd say that you > can't really tell the difference between (int, pid_t) where a type was > undeclared and (int a, b) where a type name is needed before "b". So > perhaps the messages should say that either a type is undeclared or a type > is missing before a parameter name. (Actually, I'd guess that almost all > function prototypes either name all parameters or no parameters, so in the > first case it's a very good guess that pid_t is a type and in the second > it's a very good guess that b is a parameter name. But that's definitely > an enhancement for the future.) > > The patch is OK without further code changes needed but with one testcase > fix: > >> +int f1(int x, pid_t y, long z, in t) { /* { dg-error "unknown type name 'pid_t'|unknown type name 'in'" } */ > > Please use two separate dg-error directives to match the two separate > error messages. The second one would use the form with a line number, { > dg-error "message" "test name" { target *-*-* } line-number }. > Thanks. Committed to trunk. Pearly [-- Attachment #2: a.diff --] [-- Type: text/x-patch, Size: 6414 bytes --] Index: testsuite/gcc.dg/noncompile/pr44517.c =================================================================== --- testsuite/gcc.dg/noncompile/pr44517.c (revision 0) +++ testsuite/gcc.dg/noncompile/pr44517.c (revision 161363) @@ -0,0 +1,18 @@ +/* PR c/44517: Improve diagnostic for misspelled typename in function declaration. */ +int f1(int x, pid_t y, long z, in t) { +/* { dg-error "unknown type name 'pid_t'" "" { target *-*-* } 2 } */ +/* { dg-error "unknown type name 'in'" "" { target *-*-* } 2 } */ + return x + y + z + t; +} + +int f2(int x, lon y, long z, ...){ /* { dg-error "unknown type name 'lon'" } */ + return; +} + +void f3(int n, int a[n], pid_t x); /* { dg-error "unknown type name 'pid_t'" } */ +void f4() {} +void f5(int a, *b); /* { dg-error "expected declaration specifiers or" } */ +void f6(int a, b); /* { dg-error "expected declaration specifiers or" } */ +void f7(int a, goto b); /* { dg-error "expected declaration specifiers or" } */ +void f8(int a, in goto); /* { dg-error "unknown type name 'in'" } */ +void f9(int a, in 1); /* { dg-error "unknown type name 'in'" } */ Index: testsuite/gcc.dg/noncompile/990416-1.c =================================================================== --- testsuite/gcc.dg/noncompile/990416-1.c (revision 161362) +++ testsuite/gcc.dg/noncompile/990416-1.c (revision 161363) @@ -2,11 +2,11 @@ extern void *memcpy (void *, const void typedef int word_type; static void -copy_reg (unsigned int reg, frame_state *udata, /* { dg-error "parse|syntax|expected" } */ - frame_state *target_udata) /* { dg-error "expected" } */ +copy_reg (unsigned int reg, frame_state *udata, /* { dg-error "unknown type name" } */ + frame_state *target_udata) /* { dg-error "unknown type name" } */ { - word_type *preg = get_reg_addr (reg, udata, 0); /* { dg-error "undeclared|function|without a cast" } */ - word_type *ptreg = get_reg_addr (reg, target_udata, 0); /* { dg-error "undeclared|without a cast" } */ + word_type *preg = ge_reg_addr (reg, udata, 0); + word_type *ptreg = ge_reg_addr (reg, target_udata, 0); memcpy (ptreg, preg, __builtin_dwarf_reg_size (reg)); } Index: testsuite/ChangeLog =================================================================== --- testsuite/ChangeLog (revision 161362) +++ testsuite/ChangeLog (revision 161363) @@ -1,3 +1,9 @@ +2010-06-25 Shujing Zhao <pearly.zhao@oracle.com> + + PR c/44517 + * gcc.dg/noncompile/pr44517.c: New. + * gcc.dg/noncompile/990416-1.c: Adjust expected error. + 2010-06-24 Steve Ellcey <sje@cup.hp.com> PR testsuite/43283 Index: ChangeLog =================================================================== --- ChangeLog (revision 161362) +++ ChangeLog (revision 161363) @@ -1,3 +1,11 @@ +2010-06-25 Shujing Zhao <pearly.zhao@oracle.com> + + PR c/44517 + * c-parser.c (c_parser_parms_list_declarator): Return NULL if one of + parameters are not good. + (c_parser_parameter_declaration): Error unknown type name if the type + name can't start declaration specifiers. + 2010-06-25 Joseph Myers <joseph@codesourcery.com> * gcc.c (translate_options): Don't mention +e in comment. Index: c-parser.c =================================================================== --- c-parser.c (revision 161362) +++ c-parser.c (revision 161363) @@ -2706,7 +2706,7 @@ c_parser_parms_declarator (c_parser *par static struct c_arg_info * c_parser_parms_list_declarator (c_parser *parser, tree attrs) { - bool good_parm = false; + bool bad_parm = false; /* ??? Following the old parser, forward parameter declarations may use abstract declarators, and if no real parameter declarations follow the forward declarations then this is not diagnosed. Also @@ -2758,11 +2758,10 @@ c_parser_parms_list_declarator (c_parser /* Parse a parameter. */ struct c_parm *parm = c_parser_parameter_declaration (parser, attrs); attrs = NULL_TREE; - if (parm != NULL) - { - good_parm = true; - push_parm_decl (parm); - } + if (parm == NULL) + bad_parm = true; + else + push_parm_decl (parm); if (c_parser_next_token_is (parser, CPP_SEMICOLON)) { tree new_attrs; @@ -2774,20 +2773,13 @@ c_parser_parms_list_declarator (c_parser if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN)) { c_parser_consume_token (parser); - if (good_parm) - return get_parm_info (false); - else + if (bad_parm) { - struct c_arg_info *ret - = XOBNEW (&parser_obstack, struct c_arg_info); - ret->parms = 0; - ret->tags = 0; - ret->types = 0; - ret->others = 0; - ret->pending_sizes = 0; - ret->had_vla_unspec = 0; - return ret; + get_pending_sizes (); + return NULL; } + else + return get_parm_info (false); } if (!c_parser_require (parser, CPP_COMMA, "expected %<;%>, %<,%> or %<)%>")) @@ -2802,20 +2794,13 @@ c_parser_parms_list_declarator (c_parser if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN)) { c_parser_consume_token (parser); - if (good_parm) - return get_parm_info (true); - else + if (bad_parm) { - struct c_arg_info *ret - = XOBNEW (&parser_obstack, struct c_arg_info); - ret->parms = 0; - ret->tags = 0; - ret->types = 0; - ret->others = 0; - ret->pending_sizes = 0; - ret->had_vla_unspec = 0; - return ret; + get_pending_sizes (); + return NULL; } + else + return get_parm_info (true); } else { @@ -2841,10 +2826,22 @@ c_parser_parameter_declaration (c_parser bool dummy = false; if (!c_parser_next_token_starts_declspecs (parser)) { + c_token *token = c_parser_peek_token (parser); + if (parser->error) + return NULL; + c_parser_set_source_position_from_token (token); + if (token->type == CPP_NAME + && c_parser_peek_2nd_token (parser)->type != CPP_COMMA + && c_parser_peek_2nd_token (parser)->type != CPP_CLOSE_PAREN) + { + error ("unknown type name %qE", token->value); + parser->error = true; + } /* ??? In some Objective-C cases '...' isn't applicable so there should be a different message. */ - c_parser_error (parser, - "expected declaration specifiers or %<...%>"); + else + c_parser_error (parser, + "expected declaration specifiers or %<...%>"); c_parser_skip_to_end_of_parameter (parser); return NULL; } ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2010-06-25 8:01 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-06-22 8:14 [PATCH C] Fix pr44517 Shujing Zhao 2010-06-22 8:49 ` Manuel López-Ibáñez 2010-06-22 9:57 ` Shujing Zhao 2010-06-22 10:00 ` Manuel López-Ibáñez 2010-06-22 13:36 ` Joseph S. Myers 2010-06-23 9:17 ` Shujing Zhao 2010-06-23 9:31 ` Manuel López-Ibáñez 2010-06-23 9:55 ` Shujing Zhao 2010-06-23 12:24 ` Joseph S. Myers 2010-06-23 12:28 ` Manuel López-Ibáñez 2010-06-23 12:41 ` Joseph S. Myers 2010-06-24 10:50 ` Shujing Zhao 2010-06-24 11:56 ` Joseph S. Myers 2010-06-25 10:02 ` Shujing Zhao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).