[Mesa-dev] [PATCH 2/2] glsl: struct constructors/initializers only allow implicit conversions

[Andres Gomez]

When an argument for a structure constructor or initializer doesn't
match the expected type, only Section 4.1.10 “Implicit Conversions”
are allowed to try to match that expected type.

>From page 32 (page 38 of the PDF) of the GLSL 1.20 spec:

  " The arguments to the constructor will be used to set the structure's
    fields, in order, using one argument per field. Each argument must
    be the same type as the field it sets, or be a type that can be
    converted to the field's type according to Section 4.1.10 “Implicit
    Conversions.”"

>From page 35 (page 41 of the PDF) of the GLSL 4.20 spec:

  " In all cases, the innermost initializer (i.e., not a list of
    initializers enclosed in curly braces) applied to an object must
    have the same type as the object being initialized or be a type that
    can be converted to the object's type according to section 4.1.10
    "Implicit Conversions". In the latter case, an implicit conversion
    will be done on the initializer before the assignment is done."

Fixes GL44-CTS.shading_language_420pack.initializer_list_negative

Signed-off-by: Andres Gomez <agomez at igalia.com>
---
 src/compiler/glsl/ast_function.cpp | 62 ++++++++++++++++++++++++++++----------
 1 file changed, 46 insertions(+), 16 deletions(-)

diff --git a/src/compiler/glsl/ast_function.cpp b/src/compiler/glsl/ast_function.cpp
index 9dcec50..9b09cb6 100644
--- a/src/compiler/glsl/ast_function.cpp
+++ b/src/compiler/glsl/ast_function.cpp
@@ -31,10 +31,6 @@
 static ir_rvalue *
 convert_component(ir_rvalue *src, const glsl_type *desired_type);
 
-bool
-apply_implicit_conversion(const glsl_type *to, ir_rvalue * &from,
-                          struct _mesa_glsl_parse_state *state);
-
 static unsigned
 process_parameters(exec_list *instructions, exec_list *actual_parameters,
 		   exec_list *parameters,
@@ -1703,6 +1699,23 @@ process_record_constructor(exec_list *instructions,
                            struct _mesa_glsl_parse_state *state)
 {
    void *ctx = state;
+   /* From page 32 (page 38 of the PDF) of the GLSL 1.20 spec:
+    *
+    *    "The arguments to the constructor will be used to set the structure's
+    *     fields, in order, using one argument per field. Each argument must
+    *     be the same type as the field it sets, or be a type that can be
+    *     converted to the field's type according to Section 4.1.10 “Implicit
+    *     Conversions.”"
+    *
+    * From page 35 (page 41 of the PDF) of the GLSL 4.20 spec:
+    *
+    *    "In all cases, the innermost initializer (i.e., not a list of
+    *     initializers enclosed in curly braces) applied to an object must
+    *     have the same type as the object being initialized or be a type that
+    *     can be converted to the object's type according to section 4.1.10
+    *     "Implicit Conversions". In the latter case, an implicit conversion
+    *     will be done on the initializer before the assignment is done."
+    */
    exec_list actual_parameters;
 
    process_parameters(instructions, &actual_parameters,
@@ -1710,7 +1723,7 @@ process_record_constructor(exec_list *instructions,
 
    exec_node *node = actual_parameters.get_head_raw();
    for (unsigned i = 0; i < constructor_type->length; i++) {
-      ir_rvalue *ir = (ir_rvalue *) node;
+      ir_rvalue *result = (ir_rvalue *) node;
 
       if (node->is_tail_sentinel()) {
          _mesa_glsl_error(loc, state,
@@ -1719,18 +1732,35 @@ process_record_constructor(exec_list *instructions,
          return ir_rvalue::error_value(ctx);
       }
 
-      if (apply_implicit_conversion(constructor_type->fields.structure[i].type,
-                                 ir, state)) {
-         node->replace_with(ir);
-      } else {
-         _mesa_glsl_error(loc, state,
-                          "parameter type mismatch in constructor for `%s.%s' "
-                          "(%s vs %s)",
-                          constructor_type->name,
-                          constructor_type->fields.structure[i].name,
-                          ir->type->name,
-                          constructor_type->fields.structure[i].type->name);
+      const glsl_base_type element_base_type =
+         constructor_type->fields.structure[i].type->base_type;
+
+      /* Apply implicit conversions (not the scalar constructor rules!). See
+       * the spec quote above. */
+      if (element_base_type != result->type->base_type) {
+         const glsl_type *desired_type =
+            glsl_type::get_instance(element_base_type,
+                                    result->type->vector_elements,
+                                    result->type->matrix_columns);
+
+	 if (result->type->can_implicitly_convert_to(desired_type, state)) {
+	    /* Even though convert_component() implements the constructor
+	     * conversion rules (not the implicit conversion rules), its safe
+	     * to use it here because we already checked that the implicit
+	     * conversion is legal.
+	     */
+	    result = convert_component(result, desired_type);
+	 }
+      }
+
+      if (result->type != constructor_type->fields.structure[i].type) {
+	 _mesa_glsl_error(loc, state, "type error in array constructor: "
+			  "expected: %s, found %s",
+			  constructor_type->fields.structure[i].type->name,
+			  result->type->name);
          return ir_rvalue::error_value(ctx);
+      } else {
+         node->replace_with(result);
       }
 
       node = node->next;
-- 
2.8.1