From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18441 invoked by alias); 23 Jun 2010 08:38:41 -0000 Received: (qmail 18395 invoked by uid 22791); 23 Jun 2010 08:38:32 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,TW_CP,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY X-Spam-Check-By: sourceware.org Received: from rcsinet10.oracle.com (HELO rcsinet10.oracle.com) (148.87.113.121) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 23 Jun 2010 08:38:26 +0000 Received: from rcsinet15.oracle.com (rcsinet15.oracle.com [148.87.113.117]) by rcsinet10.oracle.com (Switch-3.4.2/Switch-3.4.2) with ESMTP id o5N8cHPR011888 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 23 Jun 2010 08:38:19 GMT Received: from acsmt353.oracle.com (acsmt353.oracle.com [141.146.40.153]) by rcsinet15.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o5N6UDOK018760; Wed, 23 Jun 2010 08:38:17 GMT Received: from abhmt017.oracle.com by acsmt355.oracle.com with ESMTP id 349519681277282296; Wed, 23 Jun 2010 01:38:16 -0700 Received: from dhcp-beijing-cdc-10-182-121-28.cn.oracle.com (/10.182.121.28) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 23 Jun 2010 01:38:14 -0700 Message-ID: <4C21C7EE.4060805@oracle.com> Date: Wed, 23 Jun 2010 09:17:00 -0000 From: Shujing Zhao User-Agent: Thunderbird 2.0.0.24 (X11/20100228) MIME-Version: 1.0 To: "Joseph S. Myers" CC: GCC Patches , =?ISO-8859-1?Q?Manuel_L=F3pe?= =?ISO-8859-1?Q?z-Ib=E1=F1ez?= , Paolo Carlini Subject: Re: [PATCH C] Fix pr44517 References: <4C2060CC.3040401@oracle.com> In-Reply-To: Content-Type: multipart/mixed; boundary="------------020509030202070904020708" X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2010-06/txt/msg02291.txt.bz2 This is a multi-part message in MIME format. --------------020509030202070904020708 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2465 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 --------------020509030202070904020708 Content-Type: text/plain; name="ChangeLog" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="ChangeLog" Content-length: 452 gcc/ 2010-06-23 Shujing Zhao 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 PR c/44517 * gcc.dg/noncompile/pr44517.c: New. * gcc.dg/noncompile/990416-1.c: Adjust expected error. --------------020509030202070904020708 Content-Type: text/x-patch; name="pr44517.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="pr44517.patch" Content-length: 4674 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)); } --------------020509030202070904020708--