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

[Kenneth Graunke]

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())
+         _mesa_glsl_error(&@1, state, "Duplicate layout(...) qualifiers.\n");
+
+      if ($2.flags.q.invariant)
+         _mesa_glsl_error(&@1, state, "layout(...) cannot be used with "
+                          "the \"invariant\" qualifier\n");
+
+      if ($2.has_interpolation()) {
+         _mesa_glsl_error(&@1, state, "layout(...) cannot be used with "
+                          "interpolation qualifiers.\n");
+      }
+
+      $$ = $1;
+      $$.flags.i |= $2.flags.i;
    }
-   | INVARIANT
+   | storage_qualifier type_qualifier
    {
-      memset(& $$, 0, sizeof($$));
-      $$.flags.q.invariant = 1;
+      /* Section 4.3 of the GLSL 1.20 specification states:
+       * "Variable declarations may have a storage qualifier specified..."
+       *  1.30 clarifies this to "may have one storage qualifier".
+       */
+      if ($2.has_storage())
+         _mesa_glsl_error(&@1, state, "Duplicate storage qualifier.\n");
+
+      if ($2.flags.q.invariant || $2.has_interpolation() || $2.has_layout()) {
+         _mesa_glsl_error(&@1, state, "Storage qualifiers must come after "
+                          "invariant, interpolation, and layout qualifiers.\n");
+      }
+
+      $$ = $1;
+      $$.flags.i |= $2.flags.i;
    }
    ;
 
-- 
1.8.3.2