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.