[PATCH] cleanup in c-parser

[PATCH] cleanup in c-parser

[Anthony Brandon]

[-- Attachment #1: Type: text/plain, Size: 403 bytes --]

Hi,

I'm a new contributor and I don't yet have a copyright assignment or
commit access.

This is a cleanup of code duplication in c-parser.
I bootstrapped and tested on x86_64-linux.


gcc/c/ChangeLog:

2014-10-12  Anthony Brandon  <anthony.brandon@gmail.com>

        * c-parser.c (c_parser_all_labels): New function to replace
the duplicate code.
        (c_parser_statement): Call the new function.

[-- Attachment #2: cleanup.diff --]
[-- Type: text/plain, Size: 2186 bytes --]

Index: gcc/c/c-parser.c
===================================================================
--- gcc/c/c-parser.c	(revision 215973)
+++ gcc/c/c-parser.c	(working copy)
@@ -4654,6 +4654,16 @@
   mark_valid_location_for_stdc_pragma (save_valid_for_pragma);
 }
 
+static void
+c_parser_all_labels (c_parser *parser)
+{
+  while (c_parser_next_token_is_keyword (parser, RID_CASE)
+	 || c_parser_next_token_is_keyword (parser, RID_DEFAULT)
+	 || (c_parser_next_token_is (parser, CPP_NAME)
+	     && c_parser_peek_2nd_token (parser)->type == CPP_COLON))
+    c_parser_label (parser);
+}
+
 /* Parse a label (C90 6.6.1, C99 6.8.1).
 
    label:
@@ -4854,11 +4864,7 @@
 static void
 c_parser_statement (c_parser *parser)
 {
-  while (c_parser_next_token_is_keyword (parser, RID_CASE)
-	 || c_parser_next_token_is_keyword (parser, RID_DEFAULT)
-	 || (c_parser_next_token_is (parser, CPP_NAME)
-	     && c_parser_peek_2nd_token (parser)->type == CPP_COLON))
-    c_parser_label (parser);
+  c_parser_all_labels (parser);
   c_parser_statement_after_labels (parser);
 }
 
@@ -5090,11 +5096,7 @@
 {
   tree block = c_begin_compound_stmt (flag_isoc99);
   location_t body_loc = c_parser_peek_token (parser)->location;
-  while (c_parser_next_token_is_keyword (parser, RID_CASE)
-	 || c_parser_next_token_is_keyword (parser, RID_DEFAULT)
-	 || (c_parser_next_token_is (parser, CPP_NAME)
-	     && c_parser_peek_2nd_token (parser)->type == CPP_COLON))
-    c_parser_label (parser);
+  c_parser_all_labels (parser);
   *if_p = c_parser_next_token_is_keyword (parser, RID_IF);
   if (c_parser_next_token_is (parser, CPP_SEMICOLON))
     {
@@ -5121,11 +5123,7 @@
 {
   location_t else_loc = c_parser_peek_token (parser)->location;
   tree block = c_begin_compound_stmt (flag_isoc99);
-  while (c_parser_next_token_is_keyword (parser, RID_CASE)
-	 || c_parser_next_token_is_keyword (parser, RID_DEFAULT)
-	 || (c_parser_next_token_is (parser, CPP_NAME)
-	     && c_parser_peek_2nd_token (parser)->type == CPP_COLON))
-    c_parser_label (parser);
+  c_parser_all_labels (parser);
   if (c_parser_next_token_is (parser, CPP_SEMICOLON))
     {
       location_t loc = c_parser_peek_token (parser)->location;

Re: [PATCH] cleanup in c-parser

[pinskia]

> On Oct 12, 2014, at 12:37 PM, Anthony Brandon <anthony.brandon@gmail.com> wrote:
> 
> Hi,
> 
> I'm a new contributor and I don't yet have a copyright assignment or
> commit access.


Thanks for you contribution.  Your new function is missing a comment before it saying what it does. Yes it might be obvious what the function does but the coding style requires it. 

Thanks,
Andrew

> 
> This is a cleanup of code duplication in c-parser.
> I bootstrapped and tested on x86_64-linux.
> 
> 
> gcc/c/ChangeLog:
> 
> 2014-10-12  Anthony Brandon  <anthony.brandon@gmail.com>
> 
>        * c-parser.c (c_parser_all_labels): New function to replace
> the duplicate code.
>        (c_parser_statement): Call the new function.
> <cleanup.diff>

Re: [PATCH] cleanup in c-parser

[Anthony Brandon]

[-- Attachment #1: Type: text/plain, Size: 1102 bytes --]

I updated the patch with a comment. Actually, Manuel handed me this
patch just to help me get familiar with the process of submitting and
testing.
Generating this one with git diff looks different so I'm not sure if
that's a problem or not.

Thanks,
Anthony

On Sun, Oct 12, 2014 at 10:09 PM,  <pinskia@gmail.com> wrote:
>
>
>
>
>> On Oct 12, 2014, at 12:37 PM, Anthony Brandon <anthony.brandon@gmail.com> wrote:
>>
>> Hi,
>>
>> I'm a new contributor and I don't yet have a copyright assignment or
>> commit access.
>
>
> Thanks for you contribution.  Your new function is missing a comment before it saying what it does. Yes it might be obvious what the function does but the coding style requires it.
>
> Thanks,
> Andrew
>
>>
>> This is a cleanup of code duplication in c-parser.
>> I bootstrapped and tested on x86_64-linux.
>>
>>
>> gcc/c/ChangeLog:
>>
>> 2014-10-12  Anthony Brandon  <anthony.brandon@gmail.com>
>>
>>        * c-parser.c (c_parser_all_labels): New function to replace
>> the duplicate code.
>>        (c_parser_statement): Call the new function.
>> <cleanup.diff>



-- 
Anthony

[-- Attachment #2: cleanup-2.diff --]
[-- Type: text/plain, Size: 2358 bytes --]

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 0d159fd..346448a 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -4654,6 +4654,18 @@ c_parser_compound_statement_nostart (c_parser *parser)
   mark_valid_location_for_stdc_pragma (save_valid_for_pragma);
 }
 
+/* Parse all consecutive labels. */
+
+static void
+c_parser_all_labels (c_parser *parser)
+{
+  while (c_parser_next_token_is_keyword (parser, RID_CASE)
+	 || c_parser_next_token_is_keyword (parser, RID_DEFAULT)
+	 || (c_parser_next_token_is (parser, CPP_NAME)
+	     && c_parser_peek_2nd_token (parser)->type == CPP_COLON))
+    c_parser_label (parser);
+}
+
 /* Parse a label (C90 6.6.1, C99 6.8.1).
 
    label:
@@ -4854,11 +4866,7 @@ c_parser_label (c_parser *parser)
 static void
 c_parser_statement (c_parser *parser)
 {
-  while (c_parser_next_token_is_keyword (parser, RID_CASE)
-	 || c_parser_next_token_is_keyword (parser, RID_DEFAULT)
-	 || (c_parser_next_token_is (parser, CPP_NAME)
-	     && c_parser_peek_2nd_token (parser)->type == CPP_COLON))
-    c_parser_label (parser);
+  c_parser_all_labels (parser);
   c_parser_statement_after_labels (parser);
 }
 
@@ -5090,11 +5098,7 @@ c_parser_if_body (c_parser *parser, bool *if_p)
 {
   tree block = c_begin_compound_stmt (flag_isoc99);
   location_t body_loc = c_parser_peek_token (parser)->location;
-  while (c_parser_next_token_is_keyword (parser, RID_CASE)
-	 || c_parser_next_token_is_keyword (parser, RID_DEFAULT)
-	 || (c_parser_next_token_is (parser, CPP_NAME)
-	     && c_parser_peek_2nd_token (parser)->type == CPP_COLON))
-    c_parser_label (parser);
+  c_parser_all_labels (parser);
   *if_p = c_parser_next_token_is_keyword (parser, RID_IF);
   if (c_parser_next_token_is (parser, CPP_SEMICOLON))
     {
@@ -5121,11 +5125,7 @@ c_parser_else_body (c_parser *parser)
 {
   location_t else_loc = c_parser_peek_token (parser)->location;
   tree block = c_begin_compound_stmt (flag_isoc99);
-  while (c_parser_next_token_is_keyword (parser, RID_CASE)
-	 || c_parser_next_token_is_keyword (parser, RID_DEFAULT)
-	 || (c_parser_next_token_is (parser, CPP_NAME)
-	     && c_parser_peek_2nd_token (parser)->type == CPP_COLON))
-    c_parser_label (parser);
+  c_parser_all_labels (parser);
   if (c_parser_next_token_is (parser, CPP_SEMICOLON))
     {
       location_t loc = c_parser_peek_token (parser)->location;

Re: [PATCH] cleanup in c-parser

[Jeff Law]

On 10/13/14 05:11, Anthony Brandon wrote:
> I updated the patch with a comment. Actually, Manuel handed me this
> patch just to help me get familiar with the process of submitting and
> testing.
> Generating this one with git diff looks different so I'm not sure if
> that's a problem or not.
This version is fine.  Please install on the trunk.

Thanks,
jeff

Re: [PATCH] cleanup in c-parser

[Manuel López-Ibáñez]

On 13 October 2014 20:52, Jeff Law <law@redhat.com> wrote:
> On 10/13/14 05:11, Anthony Brandon wrote:
>>
>> I updated the patch with a comment. Actually, Manuel handed me this
>> patch just to help me get familiar with the process of submitting and
>> testing.
>> Generating this one with git diff looks different so I'm not sure if
>> that's a problem or not.
>
> This version is fine.  Please install on the trunk.

Committed as revision https://gcc.gnu.org/r216165

Congratulations Anthony, you cleared the tutorial level!

Have you decided what are you going to fix next?

Cheers,

Manuel.