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

[Zebediah Figura]

On 10/14/21 12:37 PM, Giovanni Mascellani wrote:
> 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.

In a sense, yeah, although I probably wouldn't call it 
expr_common_shape() either, which is probably part of the confusion.

> 
> 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?

Hmm, I suppose so...

Granted, you don't *have* to call expr_common_shape() twice; you can 
take the prior type and change it to BOOL.

> 
> 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.

Well, that or proper out parameters, I guess, like HLSL ;-)

Not that HLSL is a pinnacle of good language design, but I'm sure if 
this weren't C I'd find it more idiomatic.

As it is I still have a mild preference for avoiding the output 
parameters, just for the sake of what I see as idiomatic code, but I 
won't object if you keep it the way it is.