[C PATCH] remove goto in c_parser_sizeof_expression

[C PATCH] remove goto in c_parser_sizeof_expression

[Prathamesh Kulkarni]

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

Not sure if this a good idea, but it seemed to me that goto sizeof_expr; wasn't
really required in c_parser_sizeof_expression.
Bootstrapped and regression tested on x8_64-unknown-linux-gnu
Ok for trunk ?

* c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr;

Thanks and Regards,
Prathamesh

[-- Attachment #2: rm-goto.patch --]
[-- Type: text/x-patch, Size: 1204 bytes --]

Index: gcc/c/c-parser.c
===================================================================
--- gcc/c/c-parser.c	(revision 207916)
+++ gcc/c/c-parser.c	(working copy)
@@ -6518,26 +6518,27 @@ c_parser_sizeof_expression (c_parser *pa
 	  expr = c_parser_postfix_expression_after_paren_type (parser,
 							       type_name,
 							       expr_loc);
-	  goto sizeof_expr;
 	}
+      else
+  {
       /* sizeof ( type-name ).  */
       c_inhibit_evaluation_warnings--;
       in_sizeof--;
       return c_expr_sizeof_type (expr_loc, type_name);
+  }
     }
   else
     {
       expr_loc = c_parser_peek_token (parser)->location;
       expr = c_parser_unary_expression (parser);
-    sizeof_expr:
-      c_inhibit_evaluation_warnings--;
-      in_sizeof--;
-      mark_exp_read (expr.value);
-      if (TREE_CODE (expr.value) == COMPONENT_REF
+    }
+    c_inhibit_evaluation_warnings--;
+    in_sizeof--;
+    mark_exp_read (expr.value);
+    if (TREE_CODE (expr.value) == COMPONENT_REF
 	  && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
 	error_at (expr_loc, "%<sizeof%> applied to a bit-field");
       return c_expr_sizeof_expr (expr_loc, expr);
-    }
 }
 
 /* Parse an alignof expression.  */

Re: [C PATCH] remove goto in c_parser_sizeof_expression

[Prathamesh Kulkarni]

On Sat, Feb 22, 2014 at 10:34 PM, Prathamesh Kulkarni
<bilbotheelffriend@gmail.com> wrote:
> Not sure if this a good idea, but it seemed to me that goto sizeof_expr; wasn't
> really required in c_parser_sizeof_expression.
> Bootstrapped and regression tested on x8_64-unknown-linux-gnu
> Ok for trunk ?
>
> * c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr;
* c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr.
>
> Thanks and Regards,
> Prathamesh

Re: [C PATCH] remove goto in c_parser_sizeof_expression

[Marek Polacek]

On Sat, Feb 22, 2014 at 10:34:13PM +0530, Prathamesh Kulkarni wrote:
> Not sure if this a good idea, but it seemed to me that goto sizeof_expr; wasn't
> really required in c_parser_sizeof_expression.
> Bootstrapped and regression tested on x8_64-unknown-linux-gnu
> Ok for trunk ?
> 
> * c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr;
> 
> Thanks and Regards,
> Prathamesh

I'm not against it, but...

> Index: gcc/c/c-parser.c
> ===================================================================
> --- gcc/c/c-parser.c	(revision 207916)
> +++ gcc/c/c-parser.c	(working copy)
> @@ -6518,26 +6518,27 @@ c_parser_sizeof_expression (c_parser *pa
>  	  expr = c_parser_postfix_expression_after_paren_type (parser,
>  							       type_name,
>  							       expr_loc);
> -	  goto sizeof_expr;
>  	}

Remove { } around expr = c_parser_...

> +      else
> +  {
>        /* sizeof ( type-name ).  */
>        c_inhibit_evaluation_warnings--;
>        in_sizeof--;
>        return c_expr_sizeof_type (expr_loc, type_name);
> +  }

Tab before { and }.

> +    c_inhibit_evaluation_warnings--;
> +    in_sizeof--;
> +    mark_exp_read (expr.value);
> +    if (TREE_CODE (expr.value) == COMPONENT_REF
>  	  && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
>  	error_at (expr_loc, "%<sizeof%> applied to a bit-field");
>        return c_expr_sizeof_expr (expr_loc, expr);

This hunk of code is wrongly indented.

	Marek

Re: [C PATCH] remove goto in c_parser_sizeof_expression

[Prathamesh Kulkarni]

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

On Sat, Feb 22, 2014 at 11:44 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Sat, Feb 22, 2014 at 10:34:13PM +0530, Prathamesh Kulkarni wrote:
>> Not sure if this a good idea, but it seemed to me that goto sizeof_expr; wasn't
>> really required in c_parser_sizeof_expression.
>> Bootstrapped and regression tested on x8_64-unknown-linux-gnu
>> Ok for trunk ?
>>
>> * c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr;
>>
>> Thanks and Regards,
>> Prathamesh
>
> I'm not against it, but...
>
>> Index: gcc/c/c-parser.c
>> ===================================================================
>> --- gcc/c/c-parser.c  (revision 207916)
>> +++ gcc/c/c-parser.c  (working copy)
>> @@ -6518,26 +6518,27 @@ c_parser_sizeof_expression (c_parser *pa
>>         expr = c_parser_postfix_expression_after_paren_type (parser,
>>                                                              type_name,
>>                                                              expr_loc);
>> -       goto sizeof_expr;
>>       }
>
> Remove { } around expr = c_parser_...
>
>> +      else
>> +  {
>>        /* sizeof ( type-name ).  */
>>        c_inhibit_evaluation_warnings--;
>>        in_sizeof--;
>>        return c_expr_sizeof_type (expr_loc, type_name);
>> +  }
>
> Tab before { and }.
>
>> +    c_inhibit_evaluation_warnings--;
>> +    in_sizeof--;
>> +    mark_exp_read (expr.value);
>> +    if (TREE_CODE (expr.value) == COMPONENT_REF
>>         && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
>>       error_at (expr_loc, "%<sizeof%> applied to a bit-field");
>>        return c_expr_sizeof_expr (expr_loc, expr);
>
> This hunk of code is wrongly indented.
>
Is this fine ?
* c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr.

>         Marek

[-- Attachment #2: rm-goto.patch --]
[-- Type: text/x-patch, Size: 1763 bytes --]

Index: gcc/c/c-parser.c
===================================================================
--- gcc/c/c-parser.c	(revision 207916)
+++ gcc/c/c-parser.c	(working copy)
@@ -6514,30 +6514,29 @@ c_parser_sizeof_expression (c_parser *pa
 	  return ret;
 	}
       if (c_parser_next_token_is (parser, CPP_OPEN_BRACE))
-	{
-	  expr = c_parser_postfix_expression_after_paren_type (parser,
-							       type_name,
-							       expr_loc);
-	  goto sizeof_expr;
-	}
-      /* sizeof ( type-name ).  */
-      c_inhibit_evaluation_warnings--;
-      in_sizeof--;
-      return c_expr_sizeof_type (expr_loc, type_name);
+	        expr = c_parser_postfix_expression_after_paren_type (parser,
+							            type_name,
+							            expr_loc);
+      else
+        {
+          /* sizeof ( type-name ).  */
+          c_inhibit_evaluation_warnings--;
+          in_sizeof--;
+          return c_expr_sizeof_type (expr_loc, type_name);
+        }
     }
   else
     {
       expr_loc = c_parser_peek_token (parser)->location;
       expr = c_parser_unary_expression (parser);
-    sizeof_expr:
-      c_inhibit_evaluation_warnings--;
-      in_sizeof--;
-      mark_exp_read (expr.value);
-      if (TREE_CODE (expr.value) == COMPONENT_REF
-	  && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
-	error_at (expr_loc, "%<sizeof%> applied to a bit-field");
-      return c_expr_sizeof_expr (expr_loc, expr);
     }
+    c_inhibit_evaluation_warnings--;
+    in_sizeof--;
+    mark_exp_read (expr.value);
+    if (TREE_CODE (expr.value) == COMPONENT_REF
+	  && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
+	    error_at (expr_loc, "%<sizeof%> applied to a bit-field");
+    return c_expr_sizeof_expr (expr_loc, expr);
 }
 
 /* Parse an alignof expression.  */

Re: [C PATCH] remove goto in c_parser_sizeof_expression

[Marek Polacek]

On Sun, Feb 23, 2014 at 12:19:49AM +0530, Prathamesh Kulkarni wrote:
> Is this fine ?

No, there still are some formatting issues.

> Index: gcc/c/c-parser.c
> ===================================================================
> --- gcc/c/c-parser.c	(revision 207916)
> +++ gcc/c/c-parser.c	(working copy)
> @@ -6514,30 +6514,29 @@ c_parser_sizeof_expression (c_parser *pa
>  	  return ret;
>  	}
>        if (c_parser_next_token_is (parser, CPP_OPEN_BRACE))
> -	{
> -	  expr = c_parser_postfix_expression_after_paren_type (parser,
> -							       type_name,
> -							       expr_loc);
> -	  goto sizeof_expr;
> -	}
> -      /* sizeof ( type-name ).  */
> -      c_inhibit_evaluation_warnings--;
> -      in_sizeof--;
> -      return c_expr_sizeof_type (expr_loc, type_name);
> +	        expr = c_parser_postfix_expression_after_paren_type (parser,
> +							            type_name,
> +							            expr_loc);

This should be
      if (c_parser_next_token_is (parser, CPP_OPEN_BRACE))
	expr = c_parser_postfix_expression_after_paren_type (parser,
							     type_name,
							     expr_loc);

> +      else
> +        {
> +          /* sizeof ( type-name ).  */
> +          c_inhibit_evaluation_warnings--;
> +          in_sizeof--;
> +          return c_expr_sizeof_type (expr_loc, type_name);
> +        }

Replace 8 spaces in indentation with a tab.

>    else
>      {
>        expr_loc = c_parser_peek_token (parser)->location;
>        expr = c_parser_unary_expression (parser);
> -    sizeof_expr:
> -      c_inhibit_evaluation_warnings--;
> -      in_sizeof--;
> -      mark_exp_read (expr.value);
> -      if (TREE_CODE (expr.value) == COMPONENT_REF
> -	  && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
> -	error_at (expr_loc, "%<sizeof%> applied to a bit-field");
> -      return c_expr_sizeof_expr (expr_loc, expr);
>      }
> +    c_inhibit_evaluation_warnings--;
> +    in_sizeof--;
> +    mark_exp_read (expr.value);

Two spaces here, not four.

> +    if (TREE_CODE (expr.value) == COMPONENT_REF
> +	  && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
> +	    error_at (expr_loc, "%<sizeof%> applied to a bit-field");
> +    return c_expr_sizeof_expr (expr_loc, expr);

And this should be
  if (TREE_CODE (expr.value) == COMPONENT_REF
      && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
    error_at (expr_loc, "%<sizeof%> applied to a bit-field");
  return c_expr_sizeof_expr (expr_loc, expr);

	Marek

Re: [C PATCH] remove goto in c_parser_sizeof_expression

[Prathamesh Kulkarni]

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

On Sun, Feb 23, 2014 at 1:18 AM, Marek Polacek <polacek@redhat.com> wrote:
> On Sun, Feb 23, 2014 at 12:19:49AM +0530, Prathamesh Kulkarni wrote:
>> Is this fine ?
>
> No, there still are some formatting issues.
>
>> Index: gcc/c/c-parser.c
>> ===================================================================
>> --- gcc/c/c-parser.c  (revision 207916)
>> +++ gcc/c/c-parser.c  (working copy)
>> @@ -6514,30 +6514,29 @@ c_parser_sizeof_expression (c_parser *pa
>>         return ret;
>>       }
>>        if (c_parser_next_token_is (parser, CPP_OPEN_BRACE))
>> -     {
>> -       expr = c_parser_postfix_expression_after_paren_type (parser,
>> -                                                            type_name,
>> -                                                            expr_loc);
>> -       goto sizeof_expr;
>> -     }
>> -      /* sizeof ( type-name ).  */
>> -      c_inhibit_evaluation_warnings--;
>> -      in_sizeof--;
>> -      return c_expr_sizeof_type (expr_loc, type_name);
>> +             expr = c_parser_postfix_expression_after_paren_type (parser,
>> +                                                                 type_name,
>> +                                                                 expr_loc);
>
> This should be
>       if (c_parser_next_token_is (parser, CPP_OPEN_BRACE))
>         expr = c_parser_postfix_expression_after_paren_type (parser,
>                                                              type_name,
>                                                              expr_loc);
>
>> +      else
>> +        {
>> +          /* sizeof ( type-name ).  */
>> +          c_inhibit_evaluation_warnings--;
>> +          in_sizeof--;
>> +          return c_expr_sizeof_type (expr_loc, type_name);
>> +        }
>
> Replace 8 spaces in indentation with a tab.
>
>>    else
>>      {
>>        expr_loc = c_parser_peek_token (parser)->location;
>>        expr = c_parser_unary_expression (parser);
>> -    sizeof_expr:
>> -      c_inhibit_evaluation_warnings--;
>> -      in_sizeof--;
>> -      mark_exp_read (expr.value);
>> -      if (TREE_CODE (expr.value) == COMPONENT_REF
>> -       && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
>> -     error_at (expr_loc, "%<sizeof%> applied to a bit-field");
>> -      return c_expr_sizeof_expr (expr_loc, expr);
>>      }
>> +    c_inhibit_evaluation_warnings--;
>> +    in_sizeof--;
>> +    mark_exp_read (expr.value);
>
> Two spaces here, not four.
>
>> +    if (TREE_CODE (expr.value) == COMPONENT_REF
>> +       && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
>> +         error_at (expr_loc, "%<sizeof%> applied to a bit-field");
>> +    return c_expr_sizeof_expr (expr_loc, expr);
>
> And this should be
>   if (TREE_CODE (expr.value) == COMPONENT_REF
>       && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
>     error_at (expr_loc, "%<sizeof%> applied to a bit-field");
>   return c_expr_sizeof_expr (expr_loc, expr);
>
I hope this is fine.
I apologize for bothering with stupid mistakes.

* c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr.

>         Marek

[-- Attachment #2: rm-goto.patch --]
[-- Type: text/x-patch, Size: 1680 bytes --]

Index: gcc/c/c-parser.c
===================================================================
--- gcc/c/c-parser.c	(revision 207916)
+++ gcc/c/c-parser.c	(working copy)
@@ -6514,30 +6514,29 @@ c_parser_sizeof_expression (c_parser *pa
 	  return ret;
 	}
       if (c_parser_next_token_is (parser, CPP_OPEN_BRACE))
+	expr = c_parser_postfix_expression_after_paren_type (parser,
+							     type_name,
+							     expr_loc);
+      else
 	{
-	  expr = c_parser_postfix_expression_after_paren_type (parser,
-							       type_name,
-							       expr_loc);
-	  goto sizeof_expr;
+	  /* sizeof ( type-name ).  */
+	  c_inhibit_evaluation_warnings--;
+	  in_sizeof--;
+	  return c_expr_sizeof_type (expr_loc, type_name);
 	}
-      /* sizeof ( type-name ).  */
-      c_inhibit_evaluation_warnings--;
-      in_sizeof--;
-      return c_expr_sizeof_type (expr_loc, type_name);
     }
   else
     {
       expr_loc = c_parser_peek_token (parser)->location;
       expr = c_parser_unary_expression (parser);
-    sizeof_expr:
-      c_inhibit_evaluation_warnings--;
-      in_sizeof--;
-      mark_exp_read (expr.value);
-      if (TREE_CODE (expr.value) == COMPONENT_REF
-	  && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
-	error_at (expr_loc, "%<sizeof%> applied to a bit-field");
-      return c_expr_sizeof_expr (expr_loc, expr);
     }
+  c_inhibit_evaluation_warnings--;
+  in_sizeof--;
+  mark_exp_read (expr.value);
+  if (TREE_CODE (expr.value) == COMPONENT_REF
+      && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
+	error_at (expr_loc, "%<sizeof%> applied to a bit-field");
+  return c_expr_sizeof_expr (expr_loc, expr);
 }
 
 /* Parse an alignof expression.  */

Re: [C PATCH] remove goto in c_parser_sizeof_expression

[Marek Polacek]

On Mon, Feb 24, 2014 at 11:08:27PM +0530, Prathamesh Kulkarni wrote:
> I apologize for bothering with stupid mistakes.

No problem, it takes some time to get up to speed.
 
> +  mark_exp_read (expr.value);
> +  if (TREE_CODE (expr.value) == COMPONENT_REF
> +      && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
> +	error_at (expr_loc, "%<sizeof%> applied to a bit-field");
> +  return c_expr_sizeof_expr (expr_loc, expr);
>  }

This still doesn't seem quite right.  As I said:

> > And this should be
> >   if (TREE_CODE (expr.value) == COMPONENT_REF
> >       && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
> >     error_at (expr_loc, "%<sizeof%> applied to a bit-field");
> >   return c_expr_sizeof_expr (expr_loc, expr);
> >

That is, no tab before error_at (...), but four spaces.
(Of course there's no need to retest the patch when making such
trivial adjustments.)

	Marek

Re: [C PATCH] remove goto in c_parser_sizeof_expression

[Prathamesh Kulkarni]

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

On Mon, Feb 24, 2014 at 11:28 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Mon, Feb 24, 2014 at 11:08:27PM +0530, Prathamesh Kulkarni wrote:
>> I apologize for bothering with stupid mistakes.
>
> No problem, it takes some time to get up to speed.
>
>> +  mark_exp_read (expr.value);
>> +  if (TREE_CODE (expr.value) == COMPONENT_REF
>> +      && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
>> +     error_at (expr_loc, "%<sizeof%> applied to a bit-field");
>> +  return c_expr_sizeof_expr (expr_loc, expr);
>>  }
>
> This still doesn't seem quite right.  As I said:
>
>> > And this should be
>> >   if (TREE_CODE (expr.value) == COMPONENT_REF
>> >       && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
>> >     error_at (expr_loc, "%<sizeof%> applied to a bit-field");
>> >   return c_expr_sizeof_expr (expr_loc, expr);
>> >
>
> That is, no tab before error_at (...), but four spaces.
> (Of course there's no need to retest the patch when making such
> trivial adjustments.)
>
Replaced tab by 4 spaces before error_at.
* c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr.
>         Marek

[-- Attachment #2: rm-goto.patch --]
[-- Type: text/x-patch, Size: 1683 bytes --]

Index: gcc/c/c-parser.c
===================================================================
--- gcc/c/c-parser.c	(revision 207916)
+++ gcc/c/c-parser.c	(working copy)
@@ -6514,30 +6514,29 @@ c_parser_sizeof_expression (c_parser *pa
 	  return ret;
 	}
       if (c_parser_next_token_is (parser, CPP_OPEN_BRACE))
+	expr = c_parser_postfix_expression_after_paren_type (parser,
+							     type_name,
+							     expr_loc);
+      else
 	{
-	  expr = c_parser_postfix_expression_after_paren_type (parser,
-							       type_name,
-							       expr_loc);
-	  goto sizeof_expr;
+	  /* sizeof ( type-name ).  */
+	  c_inhibit_evaluation_warnings--;
+	  in_sizeof--;
+	  return c_expr_sizeof_type (expr_loc, type_name);
 	}
-      /* sizeof ( type-name ).  */
-      c_inhibit_evaluation_warnings--;
-      in_sizeof--;
-      return c_expr_sizeof_type (expr_loc, type_name);
     }
   else
     {
       expr_loc = c_parser_peek_token (parser)->location;
       expr = c_parser_unary_expression (parser);
-    sizeof_expr:
-      c_inhibit_evaluation_warnings--;
-      in_sizeof--;
-      mark_exp_read (expr.value);
-      if (TREE_CODE (expr.value) == COMPONENT_REF
-	  && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
-	error_at (expr_loc, "%<sizeof%> applied to a bit-field");
-      return c_expr_sizeof_expr (expr_loc, expr);
     }
+  c_inhibit_evaluation_warnings--;
+  in_sizeof--;
+  mark_exp_read (expr.value);
+  if (TREE_CODE (expr.value) == COMPONENT_REF
+      && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
+    error_at (expr_loc, "%<sizeof%> applied to a bit-field");
+  return c_expr_sizeof_expr (expr_loc, expr);
 }
 
 /* Parse an alignof expression.  */

Re: [C PATCH] remove goto in c_parser_sizeof_expression

[Marek Polacek]

On Tue, Feb 25, 2014 at 12:01:25AM +0530, Prathamesh Kulkarni wrote:
> Replaced tab by 4 spaces before error_at.
> * c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr.

Looks good now, but I can't approve it.  Thanks,

	Marek

Re: [C PATCH] remove goto in c_parser_sizeof_expression

[Joseph S. Myers]

On Mon, 24 Feb 2014, Marek Polacek wrote:

> On Tue, Feb 25, 2014 at 12:01:25AM +0530, Prathamesh Kulkarni wrote:
> > Replaced tab by 4 spaces before error_at.
> > * c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr.
> 
> Looks good now, but I can't approve it.  Thanks,

I think all the discussion here has been missing a key point: what is the 
rationale for the patch?  I think the explicit control flow is clearer 
here than making code appear unconditional at the end of the function for 
what is in fact just one of the two equal cases (expressions and types) 
for this function (the handling of types having returned earlier).

Effectively, this really is conceptually "at first this looks like sizeof 
(type), but later it turns out to be sizeof expression, so we need to 
recover from having already parsed (type)", and the goto reflects that 
concept of how this parse works.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [C PATCH] remove goto in c_parser_sizeof_expression

[Jeff Law]

On 02/24/14 12:28, Marek Polacek wrote:
> On Tue, Feb 25, 2014 at 12:01:25AM +0530, Prathamesh Kulkarni wrote:
>> Replaced tab by 4 spaces before error_at.
>> * c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr.
>
> Looks good now, but I can't approve it.  Thanks,
Unless this fixes a regression, I think we should queue it for after 4.9 
branches.

jeff

Re: [C PATCH] remove goto in c_parser_sizeof_expression

[Jeff Law]

On 02/24/14 12:28, Marek Polacek wrote:
> On Tue, Feb 25, 2014 at 12:01:25AM +0530, Prathamesh Kulkarni wrote:
>> Replaced tab by 4 spaces before error_at.
>> * c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr.
>
> Looks good now, but I can't approve it.  Thanks,
Applied to the trunk.  Thanks and sorry for the delays.

jeff

Re: [C PATCH] remove goto in c_parser_sizeof_expression

[Marek Polacek]

On Thu, Apr 24, 2014 at 03:27:42PM -0600, Jeff Law wrote:
> On 02/24/14 12:28, Marek Polacek wrote:
> >On Tue, Feb 25, 2014 at 12:01:25AM +0530, Prathamesh Kulkarni wrote:
> >>Replaced tab by 4 spaces before error_at.
> >>* c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr.
> >
> >Looks good now, but I can't approve it.  Thanks,
> Applied to the trunk.  Thanks and sorry for the delays.

Hold on, I thought that in the end we don't want this.  I wanted to
point out formatting issues, and it seemed good to go, but after
Joseph's mail I have changed my mind; goto seems clearer here (to me).

	Marek

Re: [C PATCH] remove goto in c_parser_sizeof_expression

[Joseph S. Myers]

On Fri, 25 Apr 2014, Marek Polacek wrote:

> On Thu, Apr 24, 2014 at 03:27:42PM -0600, Jeff Law wrote:
> > On 02/24/14 12:28, Marek Polacek wrote:
> > >On Tue, Feb 25, 2014 at 12:01:25AM +0530, Prathamesh Kulkarni wrote:
> > >>Replaced tab by 4 spaces before error_at.
> > >>* c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr.
> > >
> > >Looks good now, but I can't approve it.  Thanks,
> > Applied to the trunk.  Thanks and sorry for the delays.
> 
> Hold on, I thought that in the end we don't want this.  I wanted to
> point out formatting issues, and it seemed good to go, but after
> Joseph's mail I have changed my mind; goto seems clearer here (to me).

Indeed.  Please revert.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [C PATCH] remove goto in c_parser_sizeof_expression

[Jeff Law]

On 04/25/14 01:18, Marek Polacek wrote:
> On Thu, Apr 24, 2014 at 03:27:42PM -0600, Jeff Law wrote:
>> On 02/24/14 12:28, Marek Polacek wrote:
>>> On Tue, Feb 25, 2014 at 12:01:25AM +0530, Prathamesh Kulkarni wrote:
>>>> Replaced tab by 4 spaces before error_at.
>>>> * c-parser.c (c_parser_sizeof_expression): Remove goto sizeof_expr.
>>>
>>> Looks good now, but I can't approve it.  Thanks,
>> Applied to the trunk.  Thanks and sorry for the delays.
>
> Hold on, I thought that in the end we don't want this.  I wanted to
> point out formatting issues, and it seemed good to go, but after
> Joseph's mail I have changed my mind; goto seems clearer here (to me).
Sorry, I didn't see Joseph's mail.  I'll revert.

jeff