On Fri, Oct 23, 2015 at 07:31:51PM -0700, Cesar Philippidis wrote:
> +static tree
> +c_parser_oacc_shape_clause (c_parser *parser, omp_clause_code kind,
> + const char *str, tree list)
> +{
> + const char *id = "num";
> + tree op0 = NULL_TREE, op1 = NULL_TREE, c;
> + location_t loc = c_parser_peek_token (parser)->location;
> +
> + if (kind == OMP_CLAUSE_VECTOR)
> + id = "length";
> +
> + if (c_parser_next_token_is (parser, CPP_OPEN_PAREN))
> + {
> + tree *op_to_parse = &op0;
> + c_token *next;
> +
> + c_parser_consume_token (parser);
> +
> + do
> + {
> + op_to_parse = &op0;
> +
> + /* Consume a comma if present. */
> + if (c_parser_next_token_is (parser, CPP_COMMA))
> + {
> + if (op0 == NULL && op1 == NULL)
> + {
> + c_parser_error (parser, "unexpected argument");
> + goto cleanup_error;
> + }
> +
> + c_parser_consume_token (parser);
> + }
This means you parse
gang (, static: *)
vector (, 5)
etc., even when you error on it afterwards with unexpected argument,
it is still different diagnostics from other invalid tokens immediately
after the opening (.
Also, loc and next are wrong if there is a valid comma.
So I'm really wondering why
gang (static: *, num: 5)
works, because next is the CPP_COMMA token, so while
c_parser_next_token_is (parser, CPP_NAME) matches the actual name,
what exactly next->value contains is unclear.
I think it would be better to:
tree ops[2] = { NULL_TREE, NULL_TREE };
do
{
// Note, declare these here
c_token *next = c_parser_peek_token (parser);
location_t loc = next->location;
// Just use ops[idx] instead of *op_to_parse etc., though if you strongly
// prefer *op_to_parse, I won't object.
int idx = 0;
// Note it seems generally the C parser doesn't check for CPP_KEYWORD
// before calling c_parser_next_token_is_keyword. And I'd just do it
// for OMP_CLAUSE_GANG, which has it in the grammar.
if (kind == OMP_CLAUSE_GANG
&& c_parser_next_token_is_keyword (parser, RID_STATIC))
{
// ...
// Your current code, except that for
if (c_parser_next_token_is (parser, CPP_MULT))
{
c_parser_consume_token (parser);
if (c_parser_next_token_is (parser, CPP_COMMA))
{
c_parser_consume_token (parser);
continue;
}
break;
}
}
else if (... num: / length: )
{
// ...
}
// ...
mark_exp_read (expr);
ops[idx] = expr;
if (kind == OMP_CLAUSE_GANG
&& c_parser_next_token_is (parser, CPP_COMMA))
{
c_parser_consume_token (parser);
continue;
}
break;
}
while (1);
if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>"))
goto cleanup_error;
That way you don't parse something that is not in the grammar.
Jakub
