Other than the code formatting comment below, this series is
Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
On 12/19/2013 04:25 PM, Carl Worth wrote:
> Two things make this code confusing:
>
> 1. The uncharacteristic manipulation of lexer start state outside of
> flex rules.
>
> 2. The confusing semantics of the skip_stack (including the
> "lexing_if" override and the SKIP_NO_SKIP state).
>
> This new comment is intended to bring a bit more clarity for any readers.
>
> There is no intended beahvioral change to the code here. The actual code
> changes include better indentation to avoid an excessively-long line, and
> using the more descriptive INITIAL rather than 0.
> ---
> src/glsl/glcpp/glcpp-lex.l | 45 +++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 39 insertions(+), 6 deletions(-)
>
> diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
> index a029f62..5543edc 100644
> --- a/src/glsl/glcpp/glcpp-lex.l
> +++ b/src/glsl/glcpp/glcpp-lex.l
> @@ -92,14 +92,47 @@ OCTAL_INTEGER 0[0-7]*[uU]?
> HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]?
>
> %%
> - /* Implicitly switch between SKIP and INITIAL (non-skipping);
> - * don't switch if some other state was explicitly set.
> +
> + /* The handling of the SKIP vs INITIAL start states requires
> + * some special handling. Typically, a lexer would change
> + * start states with statements like "BEGIN SKIP" within the
> + * lexer rules. We can't get away with that here, since we
> + * need the parser to actually evaluate expressions for
> + * directives like "#if".
> + *
> + * So, here, in code that will be executed on every call to
> + * the lexer,and before any rules, we examine the skip_stack
> + * as set by the parser to know whether to change from INITIAL
> + * to SKIP or from SKIP back to INITIAL.
> + *
> + * Three cases cause us to switch out of the SKIP state and
> + * back to the INITIAL state:
> + *
> + * 1. The top of the skip_stack is of type SKIP_NO_SKIP
> + * This means we're still evaluating some #if
> + * hierarchy, but we're on a branch of it where
> + * content should not be skipped (such as "#if 1" or
> + * "#else" or so).
> + *
> + * 2. The skip_stack is NULL meaning that we've reached
> + * the last #endif.
> + *
> + * 3. The lexing_if bit is set. This indicates that we
> + * are lexing the expression following an "#if" of
> + * "#elif". Even inside an "#if 0" we need to lex this
> + * expression so the parser can correctly update the
> + * skip_stack state.
> */
> glcpp_parser_t *parser = yyextra;
> - if (YY_START == 0 || YY_START == SKIP) {
> - if (parser->lexing_if || parser->skip_stack == NULL ||
> parser->skip_stack->type == SKIP_NO_SKIP) {
> - BEGIN 0;
> - } else {
> + if (YY_START == INITIAL || YY_START == SKIP) {
> + if (parser->lexing_if ||
> + parser->skip_stack == NULL ||
> + parser->skip_stack->type == SKIP_NO_SKIP)
> + {
> + BEGIN INITIAL;
> + }
> + else
> + {
This should used the same formatting as the rest of Mesa (the { or } on
the same line as the if or else).
> BEGIN SKIP;
> }
> }
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
