[Mesa-dev] [PATCH] glsl: Clarify error message about whole-array assignment in GLSL 1.10.

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/07/2011 12:51 PM, Eric Anholt wrote:
> Previously, it would produce:
> 
> Failed to compile FS: 0:6(7): error: non-lvalue in assignment
> 
> and now it produces:
> 
> Failed to compile FS: 0:5(7): error: whole array assignment is not 
> allowed in GLSL 1.10 or GLSL ES 1.00.
> 
> Also, add spec quotation to the two places we have code for array 
> lvalues in GLSL 1.10.

There are two places in the code that use is_lvalue.  One is in
do_assignment, and the other is in match_function_by_name (for out and
inout parameters).  Since we've already special case handle whole
arrays in do_assignment, I think we should do the same in
match_function_by_name and remove array_lvalue.

In particular, I think we should check that out and inout parameters
can be arrays when processing the function prototype.  That should be
valid (and "just work") in everything except desktop GLSL 1.10.  Right?

That may be a reasonable thing to do as a follow-on patch...

In any case, the series is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> ---
> 
> And, of course, because I failed to do a full test run, it broke
> ES. Add even more spec quotations to clarify.
> 
> src/glsl/ast_to_hir.cpp |   31 +++++++++++++++++++++++++------ 1
> files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp 
> index 6ef763c..9b3f746 100644 --- a/src/glsl/ast_to_hir.cpp +++
> b/src/glsl/ast_to_hir.cpp @@ -679,16 +679,20 @@
> do_assignment(exec_list *instructions, struct
> _mesa_glsl_parse_state *state, lhs->variable_referenced()->name); 
> error_emitted = true;
> 
> +      } else if (state->language_version <= 110 &&
> lhs->type->is_array()) { +	 /* From page 32 (page 38 of the PDF) of
> the GLSL 1.10 spec: +	  * +	  *    "Other binary or unary
> expressions, non-dereferenced +	  *     arrays, function names,
> swizzles with repeated fields, +	  *     and constants cannot be
> l-values." +	  */ +	 _mesa_glsl_error(&lhs_loc, state, "whole array
> assignment is not " +			  "allowed in GLSL 1.10 or GLSL ES
> 1.00."); +	 error_emitted = true; } else if (!lhs->is_lvalue()) { 
> _mesa_glsl_error(& lhs_loc, state, "non-lvalue in assignment"); 
> error_emitted = true; } - -      if (state->es_shader &&
> lhs->type->is_array()) { -	 _mesa_glsl_error(&lhs_loc, state,
> "whole array assignment is not " -			  "allowed in GLSL ES
> 1.00."); -	 error_emitted = true; -      } }
> 
> ir_rvalue *new_rhs = @@ -2072,6 +2076,21 @@
> apply_type_qualifier_to_variable(const struct ast_type_qualifier
> *qual, else var->depth_layout = ir_depth_layout_none;
> 
> +   /* From page 46 (page 52 of the PDF) of the GLSL ES
> specification: +    * +    *    "Array variables are l-values and
> may be passed to parameters +    *     declared as out or inout.
> However, they may not be used as +    *     the target of an
> assignment." +    * +    * From page 32 (page 38 of the PDF) of the
> GLSL 1.10 spec: +    * +    *    "Other binary or unary
> expressions, non-dereferenced arrays, +    *     function names,
> swizzles with repeated fields, and constants +    *     cannot be
> l-values." +    * +    * So we only mark 1.10 as non-lvalues, and
> check for array +    * assignment in 100 specifically in
> do_assignment. +    */ if (var->type->is_array() &&
> state->language_version != 110) { var->array_lvalue = true; }

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk5pHisACgkQX1gOwKyEAw+NmgCgllFFRxoAwoGGTdu3nNsH7ay1
SLEAn1H8WQxY/OBcWYe8cPER/eX8RXOt
=3cur
-----END PGP SIGNATURE-----