[Mesa-dev] [PATCH] ast_to_hir: Only use the local 'type' variable as a temporary.

Kenneth Graunke kenneth at whitecape.org
Tue Apr 12 00:36:59 PDT 2011


Lots of code (deleted in this patch) tried to make type == result->type,
but not all cases did.  Don't pretend; just use result->type.
---
 src/glsl/ast_to_hir.cpp |   23 ++++-------------------
 1 files changed, 4 insertions(+), 19 deletions(-)

Why, so it does!  Good catch.  Of course, "type" is silly.  This patch
starts the process of cleaning up some of the pointless cases.  There's
probably more that could be done - for example, the cases that call
unary_arithmetic_result_type and completely ignore the type returned
(they just use it for the error checking).  But meh for now.

IMHO the only real reason for 'type' to exist is to have a temporary
available without having to use { } around case clauses.

If you like this patch I can rebase Eric's changes on top of it.

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 9538aa6..874ddb7 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -910,7 +910,7 @@ ast_expression::hir(exec_list *instructions,
    };
    ir_rvalue *result = NULL;
    ir_rvalue *op[3];
-   const struct glsl_type *type = glsl_type::error_type;
+   const struct glsl_type *type; /* a temporary variable for switch cases */
    bool error_emitted = false;
    YYLTYPE loc;
 
@@ -924,7 +924,6 @@ ast_expression::hir(exec_list *instructions,
       result = do_assignment(instructions, state, op[0], op[1], false,
 			     this->subexpressions[0]->get_location());
       error_emitted = result->type->is_error();
-      type = result->type;
       break;
    }
 
@@ -1044,7 +1043,6 @@ ast_expression::hir(exec_list *instructions,
       }
 
       result = do_comparison(ctx, operations[this->oper], op[0], op[1]);
-      type = glsl_type::bool_type;
 
       assert(error_emitted || (result->type == glsl_type::bool_type));
       break;
@@ -1214,7 +1212,6 @@ ast_expression::hir(exec_list *instructions,
 
       result = new(ctx) ir_expression(operations[this->oper], glsl_type::bool_type,
 				      op[0], op[1]);
-      type = glsl_type::bool_type;
       break;
 
    case ast_logic_not:
@@ -1230,7 +1227,6 @@ ast_expression::hir(exec_list *instructions,
 
       result = new(ctx) ir_expression(operations[this->oper], glsl_type::bool_type,
 				      op[0], NULL);
-      type = glsl_type::bool_type;
       break;
 
    case ast_mul_assign:
@@ -1250,7 +1246,6 @@ ast_expression::hir(exec_list *instructions,
       result = do_assignment(instructions, state,
 			     op[0]->clone(ctx, NULL), temp_rhs, false,
 			     this->subexpressions[0]->get_location());
-      type = result->type;
       error_emitted = (op[0]->type->is_error());
 
       /* GLSL 1.10 does not allow array assignment.  However, we don't have to
@@ -1276,7 +1271,6 @@ ast_expression::hir(exec_list *instructions,
       result = do_assignment(instructions, state,
 			     op[0]->clone(ctx, NULL), temp_rhs, false,
 			     this->subexpressions[0]->get_location());
-      type = result->type;
       error_emitted = type->is_error();
       break;
    }
@@ -1424,7 +1418,6 @@ ast_expression::hir(exec_list *instructions,
       result = do_assignment(instructions, state,
 			     op[0]->clone(ctx, NULL), temp_rhs, false,
 			     this->subexpressions[0]->get_location());
-      type = result->type;
       error_emitted = op[0]->type->is_error();
       break;
    }
@@ -1454,14 +1447,12 @@ ast_expression::hir(exec_list *instructions,
 			  op[0]->clone(ctx, NULL), temp_rhs, false,
 			  this->subexpressions[0]->get_location());
 
-      type = result->type;
       error_emitted = op[0]->type->is_error();
       break;
    }
 
    case ast_field_selection:
       result = _mesa_ast_field_selection_to_hir(this, instructions, state);
-      type = result->type;
       break;
 
    case ast_array_index: {
@@ -1618,7 +1609,6 @@ ast_expression::hir(exec_list *instructions,
       if (error_emitted)
 	 result->type = glsl_type::error_type;
 
-      type = result->type;
       break;
    }
 
@@ -1641,7 +1631,6 @@ ast_expression::hir(exec_list *instructions,
 
       if (var != NULL) {
 	 var->used = true;
-	 type = result->type;
       } else {
 	 _mesa_glsl_error(& loc, state, "`%s' undeclared",
 			  this->primary_expression.identifier);
@@ -1652,22 +1641,18 @@ ast_expression::hir(exec_list *instructions,
    }
 
    case ast_int_constant:
-      type = glsl_type::int_type;
       result = new(ctx) ir_constant(this->primary_expression.int_constant);
       break;
 
    case ast_uint_constant:
-      type = glsl_type::uint_type;
       result = new(ctx) ir_constant(this->primary_expression.uint_constant);
       break;
 
    case ast_float_constant:
-      type = glsl_type::float_type;
       result = new(ctx) ir_constant(this->primary_expression.float_constant);
       break;
 
    case ast_bool_constant:
-      type = glsl_type::bool_type;
       result = new(ctx) ir_constant(bool(this->primary_expression.bool_constant));
       break;
 
@@ -1685,16 +1670,16 @@ ast_expression::hir(exec_list *instructions,
       foreach_list_typed (ast_node, ast, link, &this->expressions)
 	 result = ast->hir(instructions, state);
 
-      type = result->type;
-
       /* Any errors should have already been emitted in the loop above.
        */
       error_emitted = true;
       break;
    }
    }
+   type = NULL; /* use result->type, not type. */
+   assert(result != NULL);
 
-   if (type->is_error() && !error_emitted)
+   if (result->type->is_error() && !error_emitted)
       _mesa_glsl_error(& loc, state, "type mismatch");
 
    return result;
-- 
1.7.4.4



More information about the mesa-dev mailing list