[Mesa-dev] [PATCH 4/4] glsl: In later GLSL versions, sequence operator is cannot be a constant expression

[Iago Toral]

On Wed, 2015-10-07 at 14:34 -0700, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> Fixes:
>     ES3-CTS.shaders.negative.constant_sequence
> 
>     spec/glsl-es-3.00/compiler/global-initializer/from-sequence.vert
>     spec/glsl-es-3.00/compiler/global-initializer/from-sequence.frag
> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  src/glsl/ast_to_hir.cpp | 43 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 6af0f80..c7444ed 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -3311,8 +3311,49 @@ process_initializer(ir_variable *var, ast_declaration *decl,
>        if (new_rhs != NULL) {
>           rhs = new_rhs;
>  
> +         /* Section 4.3.3 (Constant Expressions) of the GLSL ES 3.00.4 spec
> +          * says:
> +          *
> +          *     "A constant expression is one of
> +          *
> +          *        ...
> +          *
> +          *        - an expression formed by an operator on operands that are
> +          *          all constant expressions, including getting an element of
> +          *          a constant array, or a field of a constant structure, or
> +          *          components of a constant vector.  However, the sequence
> +          *          operator ( , ) and the assignment operators ( =, +=, ...)
> +          *          are not included in the operators that can create a
> +          *          constant expression."
> +          *
> +          * Section 12.43 (Sequence operator and constant expressions) says:
> +          *
> +          *     "Should the following construct be allowed?
> +          *
> +          *         float a[2,3];
> +          *
> +          *     The expression within the brackets uses the sequence operator
> +          *     (',') and returns the integer 3 so the construct is decl aring
> +          *     a single-dimensional array of size 3.  In some languages, the
> +          *     construct declares a two-dimensional array.  It would be
> +          *     preferable to make this construct illegal to avoid confusion.
> +          *
> +          *     One possibility is to change the definition of the sequence
> +          *     operator so that it does not return a constant- expression and
> +          *     hence cannot be used to declare an array size.
> +          *
> +          *     RESOLUTION: The result of a sequence operator is not a
> +          *     constant-expression."
> +          *
> +          * Section 4.3.3 (Constant Expressions) of the GLSL 4.30.9 spec
> +          * contains language almost identical to the section 4.3.3 fomr the
> +          * GLSL ES 3.00.4 spec.  This is a new limitation for these GLSL
> +          * versions.
> +          */
>           ir_constant *constant_value = rhs->constant_expression_value();
> -         if (!constant_value) {
> +         if (!constant_value ||
> +             (state->is_version(430, 300) &&
> +              decl->initializer->has_sequence_subexpression())) {
>              const char *const variable_mode =
>                 (type->qualifier.flags.q.constant)
>                 ? "const"

This only seems to handle the case of sequence operators used in
declaration initializers. Maybe this is all we really care about in
practice, but the spec says that in code such as:

float b = ... ;
...
b = (a++, a > 0.0 ? 2.0 : 0.0);

the RHS in the assignment is not a constant expression, and this patch
would not address that, right? I wonder if this distinction has any
actual implications in practice though.

Iago