Hi,
On Thu, Nov 09, 2023 at 09:34:56AM -0800, Andrea Bolognani wrote:
> On Mon, Oct 16, 2023 at 05:26:57PM +0200, Victor Toso wrote:
> > [*] The exception for optional fields as default is to Types that can
> > accept JSON Null as a value. For this case, we translate NULL to a
> > member type called IsNull, which is boolean in Go. This will be
> > explained better in the documentation patch of this series but the
> > main rationale is around Marshaling to and from JSON and Go data
> > structures.
> >
> > Example:
> >
> > qapi:
> > | { 'alternate': 'StrOrNull',
> > | 'data': { 's': 'str',
> > | 'n': 'null' } }
> >
> > go:
> > | type StrOrNull struct {
> > | S *string
> > | IsNull bool
> > | }
>
> 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.
Cheers,
Victor
signature.asc
Description: PGP signature
