[Mesa-dev] [PATCH v2 20/31] glsl: add _mesa_glsl_parse_state object to is_lvalue()

[Timothy Arceri]

On 24/04/17 20:35, Samuel Pitoiset wrote:
> Yes, this is a bit hacky but we don't really have the choice.
> Plain GLSL doesn't accept bindless samplers/images as l-values
> while it's allowed when ARB_bindless_texture is enabled.

Are you sure we need this? Can't we just set all the bindless qualifier 
flags to 0 when bindless is disable or something, and then just check if 
any of the flags were set on the var?

Although I'm not sure the current change is so bad. Thoughts?

> 
> The default NULL parameter is because we can't access the
> _mesa_glsl_parse_state object in few places in the compiler.
> One is_lvalue(NULL) call is for IR validation but other checks
> happen elsewhere, should be safe.
> 
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>   src/compiler/glsl/ast_function.cpp | 2 +-
>   src/compiler/glsl/ast_to_hir.cpp   | 2 +-
>   src/compiler/glsl/ir.cpp           | 4 +++-
>   src/compiler/glsl/ir.h             | 8 ++++----
>   4 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/src/compiler/glsl/ast_function.cpp b/src/compiler/glsl/ast_function.cpp
> index 1b90937ec8..00b930eb2c 100644
> --- a/src/compiler/glsl/ast_function.cpp
> +++ b/src/compiler/glsl/ast_function.cpp
> @@ -283,7 +283,7 @@ verify_parameter_modes(_mesa_glsl_parse_state *state,
>                                mode, formal->name,
>                                actual->variable_referenced()->name);
>               return false;
> -         } else if (!actual->is_lvalue()) {
> +         } else if (!actual->is_lvalue(state)) {
>               _mesa_glsl_error(&loc, state,
>                                "function parameter '%s %s' is not an lvalue",
>                                mode, formal->name);
> diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
> index 943c25a224..f90ae3d09a 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -971,7 +971,7 @@ do_assignment(exec_list *instructions, struct _mesa_glsl_parse_state *state,
>             * The restriction on arrays is lifted in GLSL 1.20 and GLSL ES 3.00.
>             */
>            error_emitted = true;
> -      } else if (!lhs->is_lvalue()) {
> +      } else if (!lhs->is_lvalue(state)) {
>            _mesa_glsl_error(& lhs_loc, state, "non-lvalue in assignment");
>            error_emitted = true;
>         }
> diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
> index 01f68a321c..b9c4452f83 100644
> --- a/src/compiler/glsl/ir.cpp
> +++ b/src/compiler/glsl/ir.cpp
> @@ -24,6 +24,8 @@
>   #include "main/core.h" /* for MAX2 */
>   #include "ir.h"
>   #include "compiler/glsl_types.h"
> +#include "glsl_parser_extras.h"
> +
>   
>   ir_rvalue::ir_rvalue(enum ir_node_type t)
>      : ir_instruction(t)
> @@ -1454,7 +1456,7 @@ ir_dereference_record::ir_dereference_record(ir_variable *var,
>   }
>   
>   bool
> -ir_dereference::is_lvalue() const
> +ir_dereference::is_lvalue(const struct _mesa_glsl_parse_state *state) const
>   {
>      ir_variable *var = this->variable_referenced();
>   
> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
> index 584714ef82..bb900233ba 100644
> --- a/src/compiler/glsl/ir.h
> +++ b/src/compiler/glsl/ir.h
> @@ -233,7 +233,7 @@ public:
>   
>      ir_rvalue *as_rvalue_to_saturate();
>   
> -   virtual bool is_lvalue() const
> +   virtual bool is_lvalue(const struct _mesa_glsl_parse_state *state = NULL) const
>      {
>         return false;
>      }
> @@ -1934,9 +1934,9 @@ public:
>      virtual bool equals(const ir_instruction *ir,
>                          enum ir_node_type ignore = ir_type_unset) const;
>   
> -   bool is_lvalue() const
> +   bool is_lvalue(const struct _mesa_glsl_parse_state *state) const
>      {
> -      return val->is_lvalue() && !mask.has_duplicates;
> +      return val->is_lvalue(state) && !mask.has_duplicates;
>      }
>   
>      /**
> @@ -1961,7 +1961,7 @@ class ir_dereference : public ir_rvalue {
>   public:
>      virtual ir_dereference *clone(void *mem_ctx, struct hash_table *) const = 0;
>   
> -   bool is_lvalue() const;
> +   bool is_lvalue(const struct _mesa_glsl_parse_state *state) const;
>   
>      /**
>       * Get the variable that is ultimately referenced by an r-value
>