On Fri, 28 Oct 2016, Richard Biener wrote:
> +/* Parse a gimple expression.
> +
> + gimple-expression:
> + gimple-unary-expression
> + gimple-call-statement
> + gimple-binary-expression
> + gimple-assign-expression
> + gimple-cast-expression
I don't see any comments expanding what the syntax is for most of these
constructs.
> + if (c_parser_next_token_is (parser, CPP_EQ))
> + c_parser_consume_token (parser);
That implies you're allowing an optional '=' at this point in the syntax.
That doesn't seem to make sense to me; I'd expect you to do if (=) {
process assignment; } else { other cases; } or similar.
> + /* GIMPLE PHI expression. */
> + if (c_parser_next_token_is_keyword (parser, RID_PHI))
I don't see this mentioned in any of the syntax comments.
> + struct {
> + /* The expression at this stack level. */
> + struct c_expr expr;
> + /* The operation on its left. */
> + enum tree_code op;
> + /* The source location of this operation. */
> + location_t loc;
> + } stack[2];
> + int sp;
> + /* Location of the binary operator. */
> + location_t binary_loc = UNKNOWN_LOCATION; /* Quiet warning. */
> +#define POP \
This all looks like excess complexity. The syntax in the comment
indicates that in GIMPLE, the operands of a binary expression are unary
expressions. So nothing related to precedence is needed at all, and you
shouldn't need this stack construct.
--
Joseph S. Myers
jos...@codesourcery.com
