[PATCH vkd3d 2/8] vkd3d-shader/hlsl: Support initialization of implicit size arrays.

[Giovanni Mascellani]

Hi,

thanks for reworking again your patches, unfortunately it seems to me 
that there are still points that can be enhanced.

Il 05/05/22 15:16, Giovanni Mascellani ha scritto:
> diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
> index 44e4964f..9fe454d0 100644
> --- a/libs/vkd3d-shader/hlsl.y
> +++ b/libs/vkd3d-shader/hlsl.y
> @@ -779,6 +779,7 @@ static struct list *gen_struct_fields(struct hlsl_ctx *ctx,
>           return NULL;
>       LIST_FOR_EACH_ENTRY_SAFE(v, v_next, fields, struct parse_variable_def, entry)
>       {
> +        bool unbounded_res_array = false;
>           unsigned int i;
>   
>           if (!(field = hlsl_alloc(ctx, sizeof(*field))))
> @@ -789,7 +790,33 @@ static struct list *gen_struct_fields(struct hlsl_ctx *ctx,
>   
>           field->type = type;
>           for (i = 0; i < v->arrays.count; ++i)
> +        {
> +            if (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
> +            {
> +                if (i < v->arrays.count - 1)
> +                {
> +                    hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
> +                            "Inner array size cannot be implicit.");
> +                }
> +                else if (type->type == HLSL_CLASS_OBJECT)
> +                {
> +                    unbounded_res_array = true;
> +                }
> +                else
> +                {
> +                    hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
> +                            "Implicit size arrays not allowed in struct fields.");
> +                }
> +            }
>               field->type = hlsl_new_array_type(ctx, field->type, v->arrays.sizes[i]);
> +        }
> +        if (unbounded_res_array)
> +        {
> +            hlsl_fixme(ctx, &v->loc, "Unbounded resource arrays as struct fields.");
> +            free_parse_variable_def(v);
> +            vkd3d_free(field);
> +            continue;
> +        }
>           vkd3d_free(v->arrays.sizes);
>           field->loc = v->loc;
>           field->name = v->name;

Here we are in the context of a struct field, where is seems that any 
kind of unbounded array is forbidden, isn't it? I mean, it should be an 
hlsl_error(), not an hlsl_fixme(): this program fails to compile with 
native:
---
struct ss
{
     Texture2D x[];
};

float4 main() : sv_target
{
     struct ss arr;

     return 0.0;
}
---

Once this is taken into account, it seems that any kind of implicit 
array is just forbidden inside a struct definition, so you can avoid 
different cases depending on the underlying type: if you see 
_COUNT_IMPLICIT inside gen_struct_field() you just hlsl_error() out 
(just like you do for typedefs, for instance). This would also agree 
with the native d3dcompiler's error message: "array dimensions of 
struct/class members must be explicit", though that's not necessarily a 
strong argument. Or an argument at all. Just a funny coincidence.

> @@ -1601,11 +1634,67 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t
>   
>       LIST_FOR_EACH_ENTRY_SAFE(v, v_next, var_list, struct parse_variable_def, entry)
>       {
> +        bool unbounded_res_array = false;
>           unsigned int i;
>   
>           type = basic_type;
> +
>           for (i = 0; i < v->arrays.count; ++i)
> +        {
> +            if (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
> +            {
> +                unsigned int size = initializer_size(&v->initializer);
> +                unsigned int elem_components = hlsl_type_component_count(type);
> +
> +                if (i < v->arrays.count - 1)
> +                {
> +                    hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
> +                            "Inner array size cannot be implicit.");
> +                    free_parse_initializer(&v->initializer);
> +                    v->initializer.args_count = 0;
> +                }
> +                else if (elem_components == 0)
> +                {
> +                    hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
> +                            "Cannot declare an implicit size array of a size 0 type.");
> +                    free_parse_initializer(&v->initializer);
> +                    v->initializer.args_count = 0;
> +                }
> +                else if (size == 0)
> +                {
> +                    if (type->type == HLSL_CLASS_OBJECT)
> +                    {
> +                        unbounded_res_array = 1;
> +                    }
> +                    else
> +                    {
> +                        hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
> +                                "Implicit size arrays need to be initialized.");
> +                        free_parse_initializer(&v->initializer);
> +                        v->initializer.args_count = 0;
> +                    }
> +                }
> +                else if (size % elem_components != 0)
> +                {
> +                    hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT,
> +                            "Cannot initialize implicit size array with %u components, expected a multiple of %u.",
> +                            size, elem_components);
> +                    free_parse_initializer(&v->initializer);
> +                    v->initializer.args_count = 0;
> +                }
> +                else
> +                {
> +                    v->arrays.sizes[i] = size / elem_components;
> +                }
> +            }
>               type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i]);
> +        }
> +        if (unbounded_res_array)
> +        {
> +            hlsl_fixme(ctx, &v->loc, "Unbounded resource arrays.");
> +            free_parse_variable_def(v);
> +            continue;
> +        }
>           vkd3d_free(v->arrays.sizes);
>   
>           if (type->type != HLSL_CLASS_MATRIX)

Isn't the case "type->type == HLSL_CLASS_OBJECT" already covered by 
"elem_components == 0"? When type is of OBJECT class, then 
hlsl_type_component_count() always spits out zero (and logs an ERR(); 
arguably it should return an error condition which should be handled by 
the caller, but maybe this issue can be postponed).

BTW, notice that you can use "true" instead of "1" to assign to a 
boolean variable.

Giovanni.