[Mesa-dev] [PATCH 2/4] glsl: Emit errors for assignments to non-l-value expressions

[Paul Berry]

On 23 December 2011 14:35, Ian Romanick <idr at freedesktop.org> wrote:

> From: Ian Romanick <ian.d.romanick at intel.com>
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42755
> ---
>  src/glsl/ast_to_hir.cpp |   28 +++++++++++++++++++++-------
>  1 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 9319313..7e0bd0d 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -664,6 +664,7 @@ mark_whole_array_access(ir_rvalue *access)
>
>  ir_rvalue *
>  do_assignment(exec_list *instructions, struct _mesa_glsl_parse_state
> *state,
> +             const ast_expression *lhs_ast,
>              ir_rvalue *lhs, ir_rvalue *rhs, bool is_initializer,
>              YYLTYPE lhs_loc)
>

Minor suggestion: Since the only reason we are passing lst_ast into this
function is so that it can check the non_lvalue_description string, I'd
prefer if we just pass in the string directly rather than the object that
contains it.  That would simplify the null check below, and it would make
the purpose of passing in the extra data more obvious.  But I'm not married
to this idea; either way, the patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>


>  {
> @@ -671,8 +672,13 @@ do_assignment(exec_list *instructions, struct
> _mesa_glsl_parse_state *state,
>    bool error_emitted = (lhs->type->is_error() || rhs->type->is_error());
>
>    if (!error_emitted) {
> -      if (lhs->variable_referenced() != NULL
> -          && lhs->variable_referenced()->read_only) {
> +      if (lhs_ast != NULL && lhs_ast->non_lvalue_description != NULL) {
> +         _mesa_glsl_error(&lhs_loc, state,
> +                          "assignment to %s",
> +                         lhs_ast->non_lvalue_description);
> +        error_emitted = true;
> +      } else if (lhs->variable_referenced() != NULL
> +                && lhs->variable_referenced()->read_only) {
>          _mesa_glsl_error(&lhs_loc, state,
>                           "assignment to read-only variable '%s'",
>                           lhs->variable_referenced()->name);
> @@ -1030,7 +1036,8 @@ ast_expression::hir(exec_list *instructions,
>       op[0] = this->subexpressions[0]->hir(instructions, state);
>       op[1] = this->subexpressions[1]->hir(instructions, state);
>
> -      result = do_assignment(instructions, state, op[0], op[1], false,
> +      result = do_assignment(instructions, state, this->subexpressions[0],
> +                            op[0], op[1], false,
>                             this->subexpressions[0]->get_location());
>       error_emitted = result->type->is_error();
>       break;
> @@ -1310,6 +1317,7 @@ ast_expression::hir(exec_list *instructions,
>                                                   op[0], op[1]);
>
>       result = do_assignment(instructions, state,
> +                            this->subexpressions[0],
>                             op[0]->clone(ctx, NULL), temp_rhs, false,
>                             this->subexpressions[0]->get_location());
>       error_emitted = (op[0]->type->is_error());
> @@ -1335,6 +1343,7 @@ ast_expression::hir(exec_list *instructions,
>                                        op[0], op[1]);
>
>       result = do_assignment(instructions, state,
> +                            this->subexpressions[0],
>                             op[0]->clone(ctx, NULL), temp_rhs, false,
>                             this->subexpressions[0]->get_location());
>       error_emitted = type->is_error();
> @@ -1349,8 +1358,9 @@ ast_expression::hir(exec_list *instructions,
>                                &loc);
>       ir_rvalue *temp_rhs = new(ctx) ir_expression(operations[this->oper],
>                                                    type, op[0], op[1]);
> -      result = do_assignment(instructions, state, op[0]->clone(ctx, NULL),
> -                             temp_rhs, false,
> +      result = do_assignment(instructions, state,
> +                            this->subexpressions[0],
> +                            op[0]->clone(ctx, NULL), temp_rhs, false,
>                              this->subexpressions[0]->get_location());
>       error_emitted = op[0]->type->is_error() || op[1]->type->is_error();
>       break;
> @@ -1365,8 +1375,9 @@ ast_expression::hir(exec_list *instructions,
>                                    state, &loc);
>       ir_rvalue *temp_rhs = new(ctx) ir_expression(operations[this->oper],
>                                                    type, op[0], op[1]);
> -      result = do_assignment(instructions, state, op[0]->clone(ctx, NULL),
> -                             temp_rhs, false,
> +      result = do_assignment(instructions, state,
> +                            this->subexpressions[0],
> +                            op[0]->clone(ctx, NULL), temp_rhs, false,
>                              this->subexpressions[0]->get_location());
>       error_emitted = op[0]->type->is_error() || op[1]->type->is_error();
>       break;
> @@ -1476,6 +1487,7 @@ ast_expression::hir(exec_list *instructions,
>                                        op[0], op[1]);
>
>       result = do_assignment(instructions, state,
> +                            this->subexpressions[0],
>                             op[0]->clone(ctx, NULL), temp_rhs, false,
>                             this->subexpressions[0]->get_location());
>       error_emitted = op[0]->type->is_error();
> @@ -1503,6 +1515,7 @@ ast_expression::hir(exec_list *instructions,
>       result = get_lvalue_copy(instructions, op[0]->clone(ctx, NULL));
>
>       (void)do_assignment(instructions, state,
> +                         this->subexpressions[0],
>                          op[0]->clone(ctx, NULL), temp_rhs, false,
>                          this->subexpressions[0]->get_location());
>
> @@ -2370,6 +2383,7 @@ process_initializer(ir_variable *var,
> ast_declaration *decl,
>       const glsl_type *initializer_type;
>       if (!type->qualifier.flags.q.uniform) {
>         result = do_assignment(initializer_instructions, state,
> +                               NULL,
>                                lhs, rhs, true,
>                                type->get_location());
>         initializer_type = result->type;
> --
> 1.7.6.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120106/087cb457/attachment-0001.html>