[Mesa-dev] [PATCH 04/14] glsl: Handle most qualifier ordering in C code rather than the grammar.

[Matt Turner]

On Mon, Jul 15, 2013 at 7:32 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> The GL_ARB_shading_language_420pack extension/GLSL 4.20 allow qualifiers
> to be specified in (basically) any order.  In order to support this, we
> can't hardcode the ordering restrictions in the grammar.
>
> This patch alters the grammar to accept invariant, storage, layout, and
> interpolation qualifiers in any order, but adds C code to enforce the
> ordering requirements.  In the 420pack case, we should be able to simply
> skip the error checks.
>
> As a bonus, this also lets us generate decent error messages, rather
> than Bison's awful "unexpected TOKEN" errors.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/glsl/glsl_parser.yy | 110 ++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 92 insertions(+), 18 deletions(-)
>
> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> index b8f3df9..72bf560 100644
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> @@ -1314,34 +1314,108 @@ parameter_type_qualifier:
>     ;
>
>  type_qualifier:
> -   storage_qualifier
> -   | layout_qualifier
> -   | layout_qualifier storage_qualifier
> +   /* Single qualifiers */
> +   INVARIANT
>     {
> -      $$ = $1;
> -      $$.flags.i |= $2.flags.i;
> +      memset(& $$, 0, sizeof($$));
> +      $$.flags.q.invariant = 1;
>     }
> +   | storage_qualifier
>     | interpolation_qualifier
> -   | interpolation_qualifier storage_qualifier
> -   {
> -      $$ = $1;
> -      $$.flags.i |= $2.flags.i;
> -   }
> -   | INVARIANT storage_qualifier
> +   | layout_qualifier
> +
> +   /* Multiple qualifiers:
> +    * In GLSL 4.20, these can be specified in any order.  In earlier versions,
> +    * they appear in this order (see GLSL 1.50 section 4.7 & comments below):
> +    *
> +    *    invariant interpolation storage precision  ...or...
> +    *    layout storage precision
> +    *
> +    * Each qualifier's rule ensures that the accumulated qualifiers on the right
> +    * side don't contain any that must appear on the left hand side.
> +    * For example, when processing a storage qualifier, we check that there are
> +    * no interpolation, layout, or invariant qualifiers to the right.
> +    */
> +   | INVARIANT type_qualifier
>     {
> +      if ($2.flags.q.invariant)
> +         _mesa_glsl_error(&@1, state, "Duplicate \"invariant\" qualifier.\n");
> +
> +      if ($2.has_layout()) {
> +         _mesa_glsl_error(&@1, state,
> +                          "\"invariant\" cannot be used with layout(...).\n");
> +      }
> +
>        $$ = $2;
>        $$.flags.q.invariant = 1;
>     }
> -   | INVARIANT interpolation_qualifier storage_qualifier
> +   | interpolation_qualifier type_qualifier
> +   {
> +      /* Section 4.3 of the GLSL 1.40 specification states:
> +       * "...qualified with one of these interpolation qualifiers"
> +       *
> +       * GLSL 1.30 claims to allow "one or more", but insists that:
> +       * "These interpolation qualifiers may only precede the qualifiers in,
> +       *  centroid in, out, or centroid out in a declaration."
> +       *
> +       * ...which means that e.g. smooth can't precede smooth, so there can be
> +       * only one after all, and the 1.40 text is a clarification, not a change.
> +       */
> +      if ($2.has_interpolation())
> +         _mesa_glsl_error(&@1, state, "Duplicate interpolation qualifier.\n");
> +
> +      if ($2.has_layout()) {
> +         _mesa_glsl_error(&@1, state, "Interpolation qualifiers cannot be used "
> +                          "with layout(...).\n");
> +      }
> +
> +      if ($2.flags.q.invariant) {
> +         _mesa_glsl_error(&@1, state, "Interpolation qualifiers must come "
> +                          "after \"invariant\".\n");
> +      }
> +
> +      $$ = $1;
> +      $$.flags.i |= $2.flags.i;
> +   }
> +   | layout_qualifier type_qualifier
>     {
> -      $$ = $2;
> -      $$.flags.i |= $3.flags.i;
> -      $$.flags.q.invariant = 1;
> +      /* The GLSL 1.50 grammar indicates that a layout(...) declaration can be
> +       * used standalone or immediately before a storage qualifier.  It cannot
> +       * be used with interpolation qualifiers or invariant.  There does not
> +       * appear to be any text indicating that it must come before the storage
> +       * qualifier, but always seems to in examples.
> +       */
> +      if ($2.has_layout())

Should be

if (!state->ARB_shading_language_420pack_enable && $2.has_layout())

Since duplicate layout qualifiers are explicitly allowed.