[PATCH vkd3d 2/6] vkd3d-shader/hlsl: Move the common shape computation to expr_common_shape.

[Giovanni Mascellani]

Hi,

Il 14/10/21 18:09, Zebediah Figura ha scritto:
> Would it make more sense to instead pass the result of 
> expr_common_base_type() to expr_common_shape()? I.e. something like
> 
>      struct hlsl_type *expr_common_shape(ctx, t1, t2, 
> expr_common_base_type(t1, t2));
> 
> or
> 
>      struct hlsl_type *expr_common_shape(ctx, t1, t2, HLSL_TYPE_BOOL);
> 
> One of those things I think of when I see a function with a lot of 
> output parameters :-)

I can do that, if you really want, but I like it less. First because 
IMHO it uselessly subverts the dependencies between different pieces of 
data: the process of computing the result type consists of two parts, 
determining the base type and determining the shape. In principle they 
are nicely independent (well, I think HLSL has some very awful corner 
cases in which this does not happen, but for the moment we're pretending 
only the nice part exists), while reading the code you're proposing I 
would be led to think that determining the shape depends on which base 
type you worked out. I find this confusing.

Also, this means that, for example in the case of the comparison 
operations of patch 6/6, you have to call expr_common_shape twice, with 
two different base types. Ok, not a big deal, but why make the code more 
confusing for computing twice the same thing?

I don't like either passing three pointers to have three outputs from a 
function, but that's what you get when your language doesn't have decent 
support for anonymous product types (tuples). In Python the same thing 
would have been very straightforward and idiomatic. And defining a 
struct just to pass three values seems a bit excessive.

Does this make sense to you?

Thanks for the review, Giovanni.