From patchwork Wed Jun 23 09:30:09 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Shujing Zhao X-Patchwork-Id: 56620 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 985DEB6F0E for ; Wed, 23 Jun 2010 19:31:47 +1000 (EST) Received: (qmail 28467 invoked by alias); 23 Jun 2010 09:31:45 -0000 Received: (qmail 28456 invoked by uid 22791); 23 Jun 2010 09:31:44 -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 09:31:37 +0000 Received: from acsinet15.oracle.com (acsinet15.oracle.com [141.146.126.227]) by rcsinet10.oracle.com (Switch-3.4.2/Switch-3.4.2) with ESMTP id o5N9VUds025949 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 23 Jun 2010 09:31:32 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 o5N9VRsM010166; Wed, 23 Jun 2010 09:31:27 GMT Received: from abhmt013.oracle.com by acsmt354.oracle.com with ESMTP id 349670211277285409; Wed, 23 Jun 2010 02:30:09 -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 02:30:08 -0700 Message-ID: <4C21D421.8070004@oracle.com> Date: Wed, 23 Jun 2010 17:30:09 +0800 From: Shujing Zhao User-Agent: Thunderbird 2.0.0.24 (X11/20100228) MIME-Version: 1.0 To: =?ISO-8859-1?Q?Manuel_L=F3pez-Ib=E1=F1ez?= CC: "Joseph S. Myers" , 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 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 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)); }