[PATCH vkd3d 1/3] vkd3d-shader/hlsl: Set the name for anonymous structures at creation time.

[Zebediah Figura]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
This series is kind of an RFC, though, because I'm not actually sure we want to
do this. The obvious benefit is that we get rid of a lot of common boilerplate
that's otherwise pretty hard to sidestep—as shown by the diffstat in patch 3. On
the other hand, we might want to do something like GCC's "x <aka y>", in which
case this would be a step in the wrong direction (and a pretty impactful step at
that.)

(This series also has us taking a bit more memory, but it's not a lot. We
already allocate names for all numeric types, since they aren't keywords.)

 libs/vkd3d-shader/hlsl.y     | 9 +++++++--
 libs/vkd3d-shader/hlsl_sm4.c | 3 +--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 6f1794e79..585331062 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -2075,7 +2075,7 @@ struct_declaration:
 
             if (!$3)
             {
-                if (!$2->name)
+                if (!strcmp($2->name, "<unnamed>"))
                     hlsl_error(ctx, @2, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX,
                             "Anonymous struct type must declare a variable.");
                 if (modifiers)
@@ -2116,7 +2116,12 @@ named_struct_spec:
 unnamed_struct_spec:
       KW_STRUCT '{' fields_list '}'
         {
-            $$ = hlsl_new_struct_type(ctx, NULL, $3);
+            char *name;
+
+            if (!(name = hlsl_strdup(ctx, "<unnamed>")))
+                YYABORT;
+
+            $$ = hlsl_new_struct_type(ctx, name, $3);
         }
 
 any_identifier:
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
index 328a8ba1b..8c2bc8f84 100644
--- a/libs/vkd3d-shader/hlsl_sm4.c
+++ b/libs/vkd3d-shader/hlsl_sm4.c
@@ -329,7 +329,6 @@ static D3D_SHADER_VARIABLE_TYPE sm4_base_type(const struct hlsl_type *type)
 static void write_sm4_type(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *buffer, struct hlsl_type *type)
 {
     const struct hlsl_type *array_type = get_array_type(type);
-    const char *name = array_type->name ? array_type->name : "<unnamed>";
     const struct hlsl_profile_info *profile = ctx->profile;
     unsigned int field_count = 0, array_size = 0;
     size_t fields_offset = 0, name_offset = 0;
@@ -339,7 +338,7 @@ static void write_sm4_type(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *b
         return;
 
     if (profile->major_version >= 5)
-        name_offset = put_string(buffer, name);
+        name_offset = put_string(buffer, array_type->name);
 
     if (type->type == HLSL_CLASS_ARRAY)
         array_size = get_array_size(type);
-- 
2.33.0

[PATCH vkd3d 2/3] vkd3d-shader/hlsl: Set the name for array types at creation time.

[Zebediah Figura]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
 libs/vkd3d-shader/hlsl.c | 91 +++++++++++-----------------------------
 1 file changed, 24 insertions(+), 67 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
index f03777a65..077cd4799 100644
--- a/libs/vkd3d-shader/hlsl.c
+++ b/libs/vkd3d-shader/hlsl.c
@@ -223,19 +223,33 @@ static struct hlsl_type *hlsl_new_type(struct hlsl_ctx *ctx, const char *name, e
     return type;
 }
 
-struct hlsl_type *hlsl_new_array_type(struct hlsl_ctx *ctx, struct hlsl_type *basic_type, unsigned int array_size)
+struct hlsl_type *hlsl_new_array_type(struct hlsl_ctx *ctx, struct hlsl_type *element_type, unsigned int array_size)
 {
-    struct hlsl_type *type;
+    struct hlsl_type *type, *named_type, *t;
+    struct vkd3d_string_buffer *string;
+    char *name;
 
-    if (!(type = hlsl_alloc(ctx, sizeof(*type))))
+    if (!(string = hlsl_get_string_buffer(ctx)))
         return NULL;
 
+    for (named_type = element_type; strchr(named_type->name, '['); named_type = named_type->e.array.type)
+        ;
+    vkd3d_string_buffer_printf(string, "%s[%u]", named_type->name, array_size);
+    for (t = element_type; t != named_type; t = t->e.array.type)
+        vkd3d_string_buffer_printf(string, "[%u]", t->e.array.elements_count);
+    if (!(name = hlsl_strdup(ctx, string->buffer)))
+        return NULL;
+    hlsl_release_string_buffer(ctx, string);
+
+    if (!(type = hlsl_alloc(ctx, sizeof(*type))))
+        return NULL;
+    type->name = name;
     type->type = HLSL_CLASS_ARRAY;
-    type->modifiers = basic_type->modifiers;
+    type->modifiers = element_type->modifiers;
     type->e.array.elements_count = array_size;
-    type->e.array.type = basic_type;
-    type->dimx = basic_type->dimx;
-    type->dimy = basic_type->dimy;
+    type->e.array.type = element_type;
+    type->dimx = element_type->dimx;
+    type->dimy = element_type->dimy;
     hlsl_type_calculate_reg_size(ctx, type);
 
     list_add_tail(&ctx->types, &type->entry);
@@ -804,69 +818,12 @@ struct vkd3d_string_buffer *hlsl_type_to_string(struct hlsl_ctx *ctx, const stru
 {
     struct vkd3d_string_buffer *string;
 
-    static const char base_types[HLSL_TYPE_LAST_SCALAR + 1][7] =
-    {
-        "float",
-        "half",
-        "double",
-        "int",
-        "uint",
-        "bool",
-    };
-
     if (!(string = hlsl_get_string_buffer(ctx)))
         return NULL;
 
-    if (type->name)
-    {
-        vkd3d_string_buffer_printf(string, "%s", type->name);
-        return string;
-    }
-
-    switch (type->type)
-    {
-        case HLSL_CLASS_SCALAR:
-            assert(type->base_type < ARRAY_SIZE(base_types));
-            vkd3d_string_buffer_printf(string, "%s", base_types[type->base_type]);
-            return string;
-
-        case HLSL_CLASS_VECTOR:
-            assert(type->base_type < ARRAY_SIZE(base_types));
-            vkd3d_string_buffer_printf(string, "%s%u", base_types[type->base_type], type->dimx);
-            return string;
-
-        case HLSL_CLASS_MATRIX:
-            assert(type->base_type < ARRAY_SIZE(base_types));
-            vkd3d_string_buffer_printf(string, "%s%ux%u", base_types[type->base_type], type->dimx, type->dimy);
-            return string;
-
-        case HLSL_CLASS_ARRAY:
-        {
-            struct vkd3d_string_buffer *inner_string;
-            const struct hlsl_type *t;
-
-            for (t = type; t->type == HLSL_CLASS_ARRAY; t = t->e.array.type)
-                ;
-
-            if ((inner_string = hlsl_type_to_string(ctx, t)))
-            {
-                vkd3d_string_buffer_printf(string, "%s", inner_string->buffer);
-                hlsl_release_string_buffer(ctx, inner_string);
-            }
-
-            for (t = type; t->type == HLSL_CLASS_ARRAY; t = t->e.array.type)
-                vkd3d_string_buffer_printf(string, "[%u]", t->e.array.elements_count);
-            return string;
-        }
-
-        case HLSL_CLASS_STRUCT:
-            vkd3d_string_buffer_printf(string, "<anonymous struct>");
-            return string;
-
-        default:
-            vkd3d_string_buffer_printf(string, "<unexpected type>");
-            return string;
-    }
+    assert(type->name);
+    vkd3d_string_buffer_printf(string, "%s", type->name);
+    return string;
 }
 
 const char *debug_hlsl_type(struct hlsl_ctx *ctx, const struct hlsl_type *type)
-- 
2.33.0

[PATCH vkd3d 3/3] vkd3d-shader/hlsl: Access the type name directly.

[Zebediah Figura]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
 libs/vkd3d-shader/hlsl.c         |  28 +--------
 libs/vkd3d-shader/hlsl.h         |   2 -
 libs/vkd3d-shader/hlsl.y         | 103 +++++++------------------------
 libs/vkd3d-shader/hlsl_codegen.c |   6 +-
 libs/vkd3d-shader/hlsl_sm4.c     |  13 +---
 5 files changed, 30 insertions(+), 122 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
index 077cd4799..39a2f1f24 100644
--- a/libs/vkd3d-shader/hlsl.c
+++ b/libs/vkd3d-shader/hlsl.c
@@ -814,30 +814,6 @@ static int compare_function_decl_rb(const void *key, const struct rb_entry *entr
     return 0;
 }
 
-struct vkd3d_string_buffer *hlsl_type_to_string(struct hlsl_ctx *ctx, const struct hlsl_type *type)
-{
-    struct vkd3d_string_buffer *string;
-
-    if (!(string = hlsl_get_string_buffer(ctx)))
-        return NULL;
-
-    assert(type->name);
-    vkd3d_string_buffer_printf(string, "%s", type->name);
-    return string;
-}
-
-const char *debug_hlsl_type(struct hlsl_ctx *ctx, const struct hlsl_type *type)
-{
-    struct vkd3d_string_buffer *string;
-    const char *ret;
-
-    if (!(string = hlsl_type_to_string(ctx, type)))
-        return NULL;
-    ret = vkd3d_dbg_sprintf("%s", string->buffer);
-    hlsl_release_string_buffer(ctx, string);
-    return ret;
-}
-
 struct vkd3d_string_buffer *hlsl_modifiers_to_string(struct hlsl_ctx *ctx, unsigned int modifiers)
 {
     struct vkd3d_string_buffer *string;
@@ -930,7 +906,7 @@ static void dump_ir_var(struct hlsl_ctx *ctx, struct vkd3d_string_buffer *buffer
             vkd3d_string_buffer_printf(buffer, "%s ", string->buffer);
         hlsl_release_string_buffer(ctx, string);
     }
-    vkd3d_string_buffer_printf(buffer, "%s %s", debug_hlsl_type(ctx, var->data_type), var->name);
+    vkd3d_string_buffer_printf(buffer, "%s %s", var->data_type->name, var->name);
     if (var->semantic.name)
         vkd3d_string_buffer_printf(buffer, " : %s%u", var->semantic.name, var->semantic.index);
 }
@@ -1150,7 +1126,7 @@ static void dump_instr(struct hlsl_ctx *ctx, struct vkd3d_string_buffer *buffer,
     else
         vkd3d_string_buffer_printf(buffer, "%p: ", instr);
 
-    vkd3d_string_buffer_printf(buffer, "%10s | ", instr->data_type ? debug_hlsl_type(ctx, instr->data_type) : "");
+    vkd3d_string_buffer_printf(buffer, "%10s | ", instr->data_type ? instr->data_type->name : "");
 
     switch (instr->type)
     {
diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
index 77b4ce928..f22c2b616 100644
--- a/libs/vkd3d-shader/hlsl.h
+++ b/libs/vkd3d-shader/hlsl.h
@@ -608,10 +608,8 @@ static inline void hlsl_release_string_buffer(struct hlsl_ctx *ctx, struct vkd3d
 }
 
 const char *debug_hlsl_expr_op(enum hlsl_ir_expr_op op);
-const char *debug_hlsl_type(struct hlsl_ctx *ctx, const struct hlsl_type *type);
 const char *debug_hlsl_writemask(unsigned int writemask);
 
-struct vkd3d_string_buffer *hlsl_type_to_string(struct hlsl_ctx *ctx, const struct hlsl_type *type);
 struct vkd3d_string_buffer *hlsl_modifiers_to_string(struct hlsl_ctx *ctx, unsigned int modifiers);
 const char *hlsl_node_type_to_string(enum hlsl_ir_node_type type);
 
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 585331062..dd67e85c0 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -268,15 +268,8 @@ static struct hlsl_ir_node *add_implicit_conversion(struct hlsl_ctx *ctx, struct
 
     if (!implicit_compatible_data_types(src_type, dst_type))
     {
-        struct vkd3d_string_buffer *src_string, *dst_string;
-
-        src_string = hlsl_type_to_string(ctx, src_type);
-        dst_string = hlsl_type_to_string(ctx, dst_type);
-        if (src_string && dst_string)
-            hlsl_error(ctx, *loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                    "Can't implicitly convert from %s to %s.", src_string->buffer, dst_string->buffer);
-        hlsl_release_string_buffer(ctx, src_string);
-        hlsl_release_string_buffer(ctx, dst_string);
+        hlsl_error(ctx, *loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                "Can't implicitly convert from %s to %s.", src_type->name, dst_type->name);
         return NULL;
     }
 
@@ -915,23 +908,15 @@ static struct hlsl_type *expr_common_type(struct hlsl_ctx *ctx, struct hlsl_type
 
     if (t1->type > HLSL_CLASS_LAST_NUMERIC)
     {
-        struct vkd3d_string_buffer *string;
-
-        if ((string = hlsl_type_to_string(ctx, t1)))
-            hlsl_error(ctx, *loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                    "Expression of type \"%s\" cannot be used in a numeric expression.", string->buffer);
-        hlsl_release_string_buffer(ctx, string);
+        hlsl_error(ctx, *loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                "Expression of type \"%s\" cannot be used in a numeric expression.", t1->name);
         return NULL;
     }
 
     if (t2->type > HLSL_CLASS_LAST_NUMERIC)
     {
-        struct vkd3d_string_buffer *string;
-
-        if ((string = hlsl_type_to_string(ctx, t2)))
-            hlsl_error(ctx, *loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                    "Expression of type \"%s\" cannot be used in a numeric expression.", string->buffer);
-        hlsl_release_string_buffer(ctx, string);
+        hlsl_error(ctx, *loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                "Expression of type \"%s\" cannot be used in a numeric expression.", t2->name);
         return NULL;
     }
 
@@ -940,15 +925,8 @@ static struct hlsl_type *expr_common_type(struct hlsl_ctx *ctx, struct hlsl_type
 
     if (!expr_compatible_data_types(t1, t2))
     {
-        struct vkd3d_string_buffer *t1_string = hlsl_type_to_string(ctx, t1);
-        struct vkd3d_string_buffer *t2_string = hlsl_type_to_string(ctx, t2);
-
-        if (t1_string && t2_string)
-            hlsl_error(ctx, *loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                    "Expression data types \"%s\" and \"%s\" are incompatible.",
-                    t1_string->buffer, t2_string->buffer);
-        hlsl_release_string_buffer(ctx, t1_string);
-        hlsl_release_string_buffer(ctx, t2_string);
+        hlsl_error(ctx, *loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                "Expression data types \"%s\" and \"%s\" are incompatible.", t1->name, t2->name);
         return NULL;
     }
 
@@ -1668,13 +1646,9 @@ static struct list *add_call(struct hlsl_ctx *ctx, const char *name,
             {
                 if (params->args[i]->data_type->type > HLSL_CLASS_LAST_NUMERIC)
                 {
-                    struct vkd3d_string_buffer *string;
-
-                    if ((string = hlsl_type_to_string(ctx, params->args[i]->data_type)))
-                        hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                                "Wrong type for argument %u of '%s': expected a numeric type, but got '%s'.\n",
-                                i + 1, name, string->buffer);
-                    hlsl_release_string_buffer(ctx, string);
+                    hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                            "Wrong type for argument %u of '%s': expected a numeric type, but got '%s'.\n",
+                            i + 1, name, params->args[i]->data_type->name);
                     free_parse_initializer(params);
                     return NULL;
                 }
@@ -1721,12 +1695,8 @@ static struct list *add_constructor(struct hlsl_ctx *ctx, struct hlsl_type *type
 
         if (arg->data_type->type == HLSL_CLASS_OBJECT)
         {
-            struct vkd3d_string_buffer *string;
-
-            if ((string = hlsl_type_to_string(ctx, arg->data_type)))
-                hlsl_error(ctx, arg->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                        "Invalid type %s for constructor argument.", string->buffer);
-            hlsl_release_string_buffer(ctx, string);
+            hlsl_error(ctx, arg->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                    "Invalid type %s for constructor argument.", arg->data_type->name);
             continue;
         }
         width = hlsl_type_component_count(arg->data_type);
@@ -2386,13 +2356,8 @@ type:
         {
             if ($3->type != HLSL_CLASS_SCALAR)
             {
-                struct vkd3d_string_buffer *string;
-
-                string = hlsl_type_to_string(ctx, $3);
-                if (string)
-                    hlsl_error(ctx, @3, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                            "Vector base type %s is not scalar.", string->buffer);
-                hlsl_release_string_buffer(ctx, string);
+                hlsl_error(ctx, @3, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                        "Vector base type %s is not scalar.", $3->name);
                 YYABORT;
             }
             if ($5 < 1 || $5 > 4)
@@ -2408,13 +2373,8 @@ type:
         {
             if ($3->type != HLSL_CLASS_SCALAR)
             {
-                struct vkd3d_string_buffer *string;
-
-                string = hlsl_type_to_string(ctx, $3);
-                if (string)
-                    hlsl_error(ctx, @3, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                            "Matrix base type %s is not scalar.", string->buffer);
-                hlsl_release_string_buffer(ctx, string);
+                hlsl_error(ctx, @3, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                        "Matrix base type %s is not scalar.", $3->name);
                 YYABORT;
             }
             if ($5 < 1 || $5 > 4)
@@ -2779,14 +2739,8 @@ selection_statement:
             vkd3d_free($5.then_instrs);
             vkd3d_free($5.else_instrs);
             if (condition->data_type->dimx > 1 || condition->data_type->dimy > 1)
-            {
-                struct vkd3d_string_buffer *string;
-
-                if ((string = hlsl_type_to_string(ctx, condition->data_type)))
-                    hlsl_error(ctx, instr->node.loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                            "if condition type %s is not scalar.", string->buffer);
-                hlsl_release_string_buffer(ctx, string);
-            }
+                hlsl_error(ctx, instr->node.loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                        "if condition type %s is not scalar.", condition->data_type->name);
             $$ = $3;
             list_add_tail($$, &instr->node.entry);
         }
@@ -3023,12 +2977,8 @@ postfix_expr:
             }
             if ($2->type > HLSL_CLASS_LAST_NUMERIC)
             {
-                struct vkd3d_string_buffer *string;
-
-                if ((string = hlsl_type_to_string(ctx, $2)))
-                    hlsl_error(ctx, @2, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                            "Constructor data type %s is not numeric.", string->buffer);
-                hlsl_release_string_buffer(ctx, string);
+                hlsl_error(ctx, @2, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                        "Constructor data type %s is not numeric.", $2->name);
                 free_parse_initializer(&$4);
                 YYABORT;
             }
@@ -3105,15 +3055,8 @@ unary_expr:
 
             if (!compatible_data_types(src_type, dst_type))
             {
-                struct vkd3d_string_buffer *src_string, *dst_string;
-
-                src_string = hlsl_type_to_string(ctx, src_type);
-                dst_string = hlsl_type_to_string(ctx, dst_type);
-                if (src_string && dst_string)
-                    hlsl_error(ctx, @3, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Can't cast from %s to %s.",
-                            src_string->buffer, dst_string->buffer);
-                hlsl_release_string_buffer(ctx, src_string);
-                hlsl_release_string_buffer(ctx, dst_string);
+                hlsl_error(ctx, @3, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Can't cast from %s to %s.",
+                        src_type->name, dst_type->name);
                 YYABORT;
             }
 
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index 9c9a38379..d73807624 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -403,8 +403,7 @@ static bool fold_constants(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, voi
                     if (instr->data_type->dimx != arg1->node.data_type->dimx
                             || instr->data_type->dimy != arg1->node.data_type->dimy)
                     {
-                        FIXME("Cast from %s to %s.\n", debug_hlsl_type(ctx, arg1->node.data_type),
-                                debug_hlsl_type(ctx, instr->data_type));
+                        FIXME("Cast from %s to %s.\n", arg1->node.data_type->name, instr->data_type->name);
                         vkd3d_free(res);
                         return false;
                     }
@@ -422,8 +421,7 @@ static bool fold_constants(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, voi
                             break;
 
                         default:
-                            FIXME("Cast from %s to %s.\n", debug_hlsl_type(ctx, arg1->node.data_type),
-                                    debug_hlsl_type(ctx, instr->data_type));
+                            FIXME("Cast from %s to %s.\n", arg1->node.data_type->name, instr->data_type->name);
                             vkd3d_free(res);
                             return false;
                     }
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
index 8c2bc8f84..c6dedef06 100644
--- a/libs/vkd3d-shader/hlsl_sm4.c
+++ b/libs/vkd3d-shader/hlsl_sm4.c
@@ -122,7 +122,6 @@ bool hlsl_sm4_usage_from_semantic(struct hlsl_ctx *ctx, const struct hlsl_semant
 static void write_sm4_signature(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc, bool output)
 {
     struct vkd3d_bytecode_buffer buffer = {0};
-    struct vkd3d_string_buffer *string;
     const struct hlsl_ir_var *var;
     size_t count_position;
     unsigned int i;
@@ -185,10 +184,8 @@ static void write_sm4_signature(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc,
                 break;
 
             default:
-                if ((string = hlsl_type_to_string(ctx, var->data_type)))
-                    hlsl_error(ctx, var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                            "Invalid data type %s for semantic variable %s.", string->buffer, var->name);
-                hlsl_release_string_buffer(ctx, string);
+                hlsl_error(ctx, var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                        "Invalid data type %s for semantic variable %s.", var->data_type->name, var->name);
                 put_u32(&buffer, D3D_REGISTER_COMPONENT_UNKNOWN);
         }
         put_u32(&buffer, reg_idx);
@@ -1159,11 +1156,7 @@ static void write_sm4_expr(struct hlsl_ctx *ctx,
 
         default:
         {
-            struct vkd3d_string_buffer *string;
-
-            if ((string = hlsl_type_to_string(ctx, expr->node.data_type)))
-                hlsl_fixme(ctx, expr->node.loc, "SM4 %s expression.", string->buffer);
-            hlsl_release_string_buffer(ctx, string);
+            hlsl_fixme(ctx, expr->node.loc, "SM4 %s expression.", expr->node.data_type->name);
             break;
         }
     }
-- 
2.33.0

[Matteo Bruni]

On Mon, Oct 4, 2021 at 3:52 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
> ---
> This series is kind of an RFC, though, because I'm not actually sure we want to
> do this. The obvious benefit is that we get rid of a lot of common boilerplate
> that's otherwise pretty hard to sidestep—as shown by the diffstat in patch 3. On
> the other hand, we might want to do something like GCC's "x <aka y>", in which
> case this would be a step in the wrong direction (and a pretty impactful step at
> that.)

This is generally fine to me, although I don't think I see much of an
improvement from this going forward. Basically all the cases that are
simplified in patch 3 are error messages (unsurprisingly); simplifying
the code for those is nice and good but probably not too big of a
deal. It would be more significant if we were to add many more errors
down the line or if the same kind of boilerplate started to be
required in some other places, possibly far from error paths. Maybe
that's actually the case?

I guess that kind of "x (aka y)" notes in error messages would be nice
to have, although probably not critical. I don't know that the changes
made by this patch series would matter a whole lot in that regard
though? It shouldn't make a difference for named types and those are
often the most interesting ones to "unravel" (I'm thinking of the
usual typedef case here).
> (This series also has us taking a bit more memory, but it's not a lot. We
> already allocate names for all numeric types, since they aren't keywords.)

Yeah, I'm not particularly concerned by that either.

[Zebediah Figura (she/her)]

On 10/5/21 03:21, Matteo Bruni wrote:
> On Mon, Oct 4, 2021 at 3:52 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
> > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
> > ---
> > This series is kind of an RFC, though, because I'm not actually sure we want to
> > do this. The obvious benefit is that we get rid of a lot of common boilerplate
> > that's otherwise pretty hard to sidestep—as shown by the diffstat in patch 3. On
> > the other hand, we might want to do something like GCC's "x <aka y>", in which
> > case this would be a step in the wrong direction (and a pretty impactful step at
> > that.)
>
> This is generally fine to me, although I don't think I see much of an
> improvement from this going forward. Basically all the cases that are
> simplified in patch 3 are error messages (unsurprisingly); simplifying
> the code for those is nice and good but probably not too big of a
> deal. It would be more significant if we were to add many more errors
> down the line or if the same kind of boilerplate started to be
> required in some other places, possibly far from error paths. Maybe
> that's actually the case?

Nothing that occurs to me at the moment, no.

On the other hand I wouldn't be surprised if we add a lot more errors 
going forward...
> I guess that kind of "x (aka y)" notes in error messages would be nice
> to have, although probably not critical. I don't know that the changes
> made by this patch series would matter a whole lot in that regard
> though? It shouldn't make a difference for named types and those are
> often the most interesting ones to "unravel" (I'm thinking of the
> usual typedef case here).

I wasn't really planning on implementing it any time soon, but I don't 
want to step in the wrong direction if it's desirable at all.

The idea is that if you want "x (aka y)" you'd probably add that 
capability to hlsl_type_to_string() [or introduce a similar function 
while keeping hlsl_type_to_string() around].

I guess that name too could always be stored in the type object...

Or we could forgo this series but have special format modifiers for 
various things. That takes work too though. Probably best just to leave 
things alone for now.
> > (This series also has us taking a bit more memory, but it's not a lot. We
> > already allocate names for all numeric types, since they aren't keywords.)
>
> Yeah, I'm not particularly concerned by that either.

[Giovanni Mascellani]

Hi,

Il 04/10/21 03:51, Zebediah Figura ha scritto:
> This series is kind of an RFC, though, because I'm not actually sure we want to
> do this. The obvious benefit is that we get rid of a lot of common boilerplate
> that's otherwise pretty hard to sidestep—as shown by the diffstat in patch 3. On
> the other hand, we might want to do something like GCC's "x <aka y>", in which
> case this would be a step in the wrong direction (and a pretty impactful step at
> that.)

I don't have strong feelings about this series, neither for nor against. 
On one hand, negative diffstats are (at feature parity, and sometimes 
even without it) very welcome. On the other hand, the removed code is 
basically due the necessity of having to explicitly name intermediate 
values because then you have to call their destructors. This is 
something I more or less give up about then working with C, so I see 
less value in removing this single instance.

Another alternative would be to be as we (more or less implicitly) we 
currently do for IR nodes: leave everything that you allocate in some 
container, so that it can be destroyed along with the context. Just loud 
thinking, though, I am not sure it's a smart way to go either.
> (This series also has us taking a bit more memory, but it's not a lot. We
> already allocate names for all numeric types, since they aren't keywords.)

Agree that's not a real concern.
> --- a/libs/vkd3d-shader/hlsl.y
> +++ b/libs/vkd3d-shader/hlsl.y
> @@ -2075,7 +2075,7 @@ struct_declaration:
>
> if (!$3)
>               {
> -                if (!$2->name)
> +                if (!strcmp($2->name, "<unnamed>"))
>                       hlsl_error(ctx, @2, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX,
>                               "Anonymous struct type must declare a variable.");
>                   if (modifiers)

Personally I don't like using user-facing values for business decisions. 
If you want to mark a struct as unnamed, then you should use a bool 
variable for that, not relying on a somewhat arbitrary name.

Granted, it would work, but I don't like the code smell (which is 
arguably present in the old code as well, though).

If you really wanted to mark types as unnamed by storing a well-known 
name, at least I'd ask that this well-known name is represented by a 
named constant.

Thanks, Giovanni.