Re: PING: Fwd: Re: [patch] implement Cilk Plus simd loops on trunk

[Jason Merrill <jason at redhat dot com>]

On 08/27/2013 04:03 PM, Aldy Hernandez wrote:
> +  /* First, try to parse as an initialized declaration.  See
> +     cp_parser_condition, from whence the bulk of this is copied.  */

You didn't do this copy, but I'd appreciate it if you could reintegrate 
this with cp_parser_condition.  I notice that there's already been 
significant drift: this copied code doesn't recognize {} initialization.

> +	      else if (!TREE_TYPE (e) || !TREE_CONSTANT (e)
> +		       || !INTEGRAL_TYPE_P (TREE_TYPE (e)))
> +		cp_parser_error (parser,
> +				 "step size must be an integer constant");

Can't the step size be a value-dependent expression like a template 
non-type parameter?

> +  if (cp_lexer_next_token_is_keyword (parser->lexer, RID_STATIC)
> +      || cp_lexer_next_token_is_keyword (parser->lexer, RID_REGISTER)
> +      || cp_lexer_next_token_is_keyword (parser->lexer, RID_EXTERN)
> +      || cp_lexer_next_token_is_keyword (parser->lexer, RID_MUTABLE)
> +      || cp_lexer_next_token_is_keyword (parser->lexer, RID_THREAD))
> +    {
> +      error_at (loc, "storage class is not allowed");
> +      cp_lexer_consume_token (parser->lexer);
> +    }

How is this different from any other for-loop?

> +  gcc_assert (for_keyword == RID_FOR);
> +
> +  if (!cp_lexer_next_token_is_keyword (parser->lexer, for_keyword))
> +    {
> +      if (for_keyword == RID_FOR)

A lot of code in this function tests for_keyword after the assert.  A 
comment lower down says

> +      /* Handle _Cilk_for here when implemented.  */
> +      gcc_unreachable ();

So do we need the earlier assert?  At least add a comment to it.

Jason