From patchwork Thu Jun 24 09:19:33 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Shujing Zhao X-Patchwork-Id: 56772 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 7D19EB6F15 for ; Thu, 24 Jun 2010 19:20:41 +1000 (EST) Received: (qmail 3395 invoked by alias); 24 Jun 2010 09:20:39 -0000 Received: (qmail 3382 invoked by uid 22791); 24 Jun 2010 09:20:38 -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 rcsinet12.oracle.com (HELO rcsinet12.oracle.com) (148.87.113.124) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 24 Jun 2010 09:20:31 +0000 Received: from acsinet15.oracle.com (acsinet15.oracle.com [141.146.126.227]) by rcsinet12.oracle.com (Switch-3.4.2/Switch-3.4.2) with ESMTP id o5O9KQqv018192 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 24 Jun 2010 09:20:28 GMT Received: from acsmt354.oracle.com (acsmt354.oracle.com [141.146.40.154]) by acsinet15.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o5NJuecP020548; Thu, 24 Jun 2010 09:20:24 GMT Received: from abhmt005.oracle.com by acsmt353.oracle.com with ESMTP id 353061471277371180; Thu, 24 Jun 2010 02:19:40 -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 ; Thu, 24 Jun 2010 02:19:38 -0700 Message-ID: <4C232325.2020107@oracle.com> Date: Thu, 24 Jun 2010 17:19:33 +0800 From: Shujing Zhao User-Agent: Thunderbird 2.0.0.24 (X11/20100228) MIME-Version: 1.0 To: "Joseph S. Myers" CC: =?ISO-8859-1?Q?Manuel_L=F3pez-Ib=E1=F1ez?= , GCC Patches , Paolo Carlini Subject: Re: [PATCH C] Fix pr44517 References: <4C2060CC.3040401@oracle.com> <4C21C7EE.4060805@oracle.com> In-Reply-To: X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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 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 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)); }