Re: [PATCH v2 04/11] qapi: golang: Generate qapi's alternate types in Go

[Andrea Bolognani]

On Thu, Nov 09, 2023 at 08:01:48PM +0100, Victor Toso wrote:
> On Thu, Nov 09, 2023 at 09:34:56AM -0800, Andrea Bolognani wrote:
> > We should call this Null instead of IsNull, and also make it a
> > pointer similarly to what I just suggested for union branches
> > that don't have additional data attached to them.
>
> I don't have a strong argument here against what you suggest, I
> just think that the usage of bool is simpler.
>
> The test I have here [0] would have code changing to something
> like:
>
> When is null:
>
>   - val := &StrOrNull{IsNull: true}
>   + myNull := JSONNull{}
>   + val := &StrOrNull{Null: &myNull}
>
> So you need a instance to get a pointer.
>
> When is absent (no fields set), no changes as both bool defaults
> to false and pointer to nil.
>
> The code handling this would change from:
>
>   - if u.IsNull {
>   + if u.Null != nil {
>         ...
>     }
>
> We don't have the same issues as Union, under the hood we Marshal
> to/Unmarshal from "null" and that would not change.
>
> [0] 
> https://gitlab.com/victortoso/qapi-go/-/blob/main/test/type_or_null_test.go
>
> I can change this in the next iteration.

No, leave the type alone. But I still think the name should probably
be Null instead of IsNull.

-- 
Andrea Bolognani / Red Hat / Virtualization