[Mesa-dev] [PATCH 3/9] glsl: ignore all but the rightmost layout qualifier name from the rightmost layout qualifier

Timothy Arceri timothy.arceri at collabora.com
Mon Oct 24 03:09:35 UTC 2016


On Sat, 2016-10-22 at 23:09 +0300, Andres Gomez wrote:
> 

<snip>

> +   | layout_qualifier BUFFER ';'
>     ;
>  
>  layout_in_defaults:
>     layout_qualifier layout_in_defaults
>     {
> -      $$ = NULL;
> +      $$ = $1;
>        if (!state->has_420pack_or_es31()) {
>           _mesa_glsl_error(&@1, state, "duplicate layout(...)
> qualifiers");
>           YYERROR;
> -      } else {
> -         if (!state->in_qualifier->
> -                merge_in_qualifier(& @1, state, $1, $$, false)) {
> -            YYERROR;
> -         }
> -         $$ = $2;
>        }
> -   }
> -   | layout_qualifier IN_TOK ';'
> -   {
> -      $$ = NULL;
> -      if (!state->in_qualifier->
> -             merge_in_qualifier(& @1, state, $1, $$, true)) {
> +      if (!$$.merge_qualifier(& @1, state, $2, false, true)) {
>           YYERROR;
>        }
>     }

After a closer look I take back my r-b. I'm pretty sure this will break
things for non layout-qualifier-name/value pairs.

For example there are some qualifiers that only apply to in/out
globals.

I belive this change will break things like:

layout(...) layout(early_fragment_tests) in;

or maybe it breaks:

layout(early_fragment_tests) layout(...) in;

I forget which way we are going :P but you get the point.


> +   | layout_qualifier IN_TOK ';'
>     ;
>  
>  layout_out_defaults:
>     layout_qualifier layout_out_defaults
>     {
> -      $$ = NULL;
> +      $$ = $1;
>        if (!state->has_420pack_or_es31()) {
>           _mesa_glsl_error(&@1, state, "duplicate layout(...)
> qualifiers");
>           YYERROR;
> -      } else {
> -         if (!state->out_qualifier->
> -                merge_out_qualifier(& @1, state, $1, $$, false)) {
> -            YYERROR;
> -         }
> -         $$ = $2;
>        }
> -   }
> -   | layout_qualifier OUT_TOK ';'
> -   {
> -      $$ = NULL;
> -      if (!state->out_qualifier->
> -             merge_out_qualifier(& @1, state, $1, $$, true))
> +      if (!$$.merge_qualifier(& @1, state, $2, false, true)) {
>           YYERROR;
> +      }
>     }
> +   | layout_qualifier OUT_TOK ';'
>     ;
>  
>  layout_defaults:
>     layout_uniform_defaults
> +   {
> +      $$ = NULL;
> +      if (!state->default_uniform_qualifier->
> +             merge_qualifier(& @1, state, $1, false)) {
> +         YYERROR;
> +      }
> +   }
>     | layout_buffer_defaults
> +   {
> +      $$ = NULL;
> +      if (!state->default_shader_storage_qualifier->
> +             merge_qualifier(& @1, state, $1, false)) {
> +         YYERROR;
> +      }
> +
> +      /* From the GLSL 4.50 spec, section 4.4.5:
> +       *
> +       *     "It is a compile-time error to specify the binding
> identifier for
> +       *     the global scope or for block member declarations."
> +       */
> +      if (state->default_shader_storage_qualifier-
> >flags.q.explicit_binding) {
> +         _mesa_glsl_error(& @1, state,
> +                          "binding qualifier cannot be set for
> default layout");
> +      }
> +   }
>     | layout_in_defaults
> +   {
> +      $$ = NULL;
> +      if (!state->in_qualifier->
> +             merge_in_qualifier(& @1, state, $1, $$, true)) {
> +         YYERROR;
> +      }
> +   }
>     | layout_out_defaults
> +   {
> +      $$ = NULL;
> +      if (!state->out_qualifier->
> +             merge_out_qualifier(& @1, state, $1, $$, true)) {
> +         YYERROR;
> +      }
> +   }
>     ;


More information about the mesa-dev mailing list