diff mbox

[C] remove goto in c_parser_sizeof_expression

Message ID CAJXstsDRb_1Vj7XUtEpfmNkKxAFc0xW7SsFW0oxm7GAm_jX+DQ@mail.gmail.com
State New
Headers show

Commit Message

Prathamesh Kulkarni Feb. 22, 2014, 6:49 p.m. UTC 
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

Comments

Marek Polacek Feb. 22, 2014, 7:48 p.m. UTC | #1 
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
diff mbox

Patch

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.  */