Skip to content

Conversation

@jspc
Copy link
Member

@jspc jspc commented Jun 15, 2023

Given a type:

type Foo struct {
     Bar []string
}

When Foo.Bar is uninitialised/ nil, we Marshall with a Len of 0. When we then Unmarshall we initialise a []string of length 0.

The problem here is that a nil slice and a slice of len() 0 are two different things. Thus, when we try:

in := new(Foo)
in.Marshall(buf)

out := new(Foo)
out.Unmarshall(buf)

our types don't match up.

By testing whether f.Len() == 0 and setting Foo.Bar as the nil value, rather than anything else, we know our collection should be nil.

Of course, this wont work if the collection should be of length 0.

Given a type:

```golang
type Foo struct {
     Bar []string
}
```

When `Foo.Bar` is uninitialised/ `nil`, we Marshall with a Len of
0. When we then Unmarshall we initialise a `[]string` of length 0.

The problem here is that a `nil` slice and a slice of `len()` 0 are two
different things. Thus, when we try:

```golang
in := new(Foo)
in.Marshall(buf)

out := new(Foo)
out.Unmarshall(buf)
```

our types don't match up.

By testing whether `f.Len() == 0` and setting `Foo.Bar` as the nil
value, rather than anything else, we know our collection should be nil.

Of course, this wont work if the collection _should_ be of length 0.
@github-actions
Copy link

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@jspc
Copy link
Member Author

jspc commented Jun 15, 2023

Note: we could use negative lengths to signify that a slice (etc.) should be nil, and use 0 to signify a 0 length slice, but our format uses uint32 to signify length- which gives us a long number in fewer bytes, at the loss of negative numbers.

We could just as easily change this, but all current serialised stuff will no longer unmarshall.

@jspc jspc merged commit 9f0a3e2 into main Jun 15, 2023
@jspc jspc deleted the nil_empty_collections branch June 15, 2023 09:06
@github-actions github-actions bot locked and limited conversation to collaborators Jun 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants