Decoding nil into provided type

[ruz]

Hello,

Code sample:

package main

import (
    "fmt"

    "github.com/ugorji/go/codec"
)

func main() {
    var ch codec.CborHandle
    b := []byte{}
    codec.NewEncoderBytes(&b, &ch).Encode([]interface{}{nil})
    fmt.Printf("%+v\n", b)

    res := []interface{}{(*string)(nil)}
    codec.NewDecoderBytes(b, &ch).Decode(&res)
    fmt.Printf("%T, %+v\n", res[0], res[0])
    _, ok := res[0].(*string)
    if !ok {
        fmt.Println("sad face")
    }
}

Do you agree that this should be fixed?

[ugorji]

I can't reason through this example. Feels too made up. I need a real example.

[ruz]

We serialize results of a function as a slice of empty interfaces. If function returns a pointer, for example *string, then it can return nil.

A function that deserializes this data is generated and has the same results signature (returns *string). Generated code looks very similar to last paragraph of the above code:

    var res0 firstResultType 
    res := []interface{}{res0}
    codec.NewDecoderBytes(b, &ch).Decode(&res)
    return res[0].(firstResultType)

This is very close to real example.

[ugorji]

Got it.

I started implementing a fix for it, but see that it breaks the underlying principle of the library.

If, during decode, we detect nil in the stream, we set the value to be decoded into to the zero value of its type. We do this while respecting the type and kind. For example, if decoding into an element of a []interface{}, and we detect nil in the stream, then we just reset the element in the slice to the zero value of interface{} which is nil.

That is why, in your example, you get a (interface{})(nil) set, not a (interface{})((*string)(nil)).

So - short answer is that your example is not supposed to work like that. I propose that you work within that principle - it keeps the library consistent.

[ruz]

We found a workaround. However, I believe that behaviour requested earlier can be justified. I understood your points regarding "zero value of its type" and "respecting the type and kind". Similar situation of picking target's type happens when value in the stream is not nil, for example a key-value map, then codec uses (*SomeStruct)(nil) just fine even if it's an element in []interface{}.

In theory zero value for an element of slice of any interface types can be smart and check content of the element.

Just a suggestion. We're ok with any decision.

[ugorji]

You are right that it can be justified. However, it is just a different model, and thinking through it opened up too many "special cases".

Right now, the model is simple to reason: nil maps to the zero value of the type. i.e. if nil is seen for a float, we set it to 0.0f, if nil is seen for a nil'able value (ptr, interface, slice, map), we set it to nil.

Also, this simple model fits into the performance characteristics of the library, else we have to do much computation to dig into the value, and only for interface and everywhere they exist.

Changing the model breaks the first principle i.e. a nil string is not equal to a nil interface. So we really are detecting a nil value in the stream, and allowing the value to be a non-nil interface. That is wrong in go terms.

I still started implementing it, but just doing an "ensureNil" became expensive, as we have to use a loop, check if interface or pointer, descend into it iteratively, then check the kind and value to see if it is nil. This is also much harder to do in unsafe mode. And it started blowing out the performance metrics. And I only started implementing it on a []interface{}. Chasing every where this may apply will require quiet deliberation and review of the codebase, all for something that breaks down the simple model we had. That is why it became a non-starter.

I'm writing all this, so you know somewhat that I didn't just disregard your issue (I hate it when people do that). I worked on it for about 10 hours, before I gave up, as I was trying hard to do something that didn't fit in the library. Trust that, if it fit, I would take days to make it happen. It shows in the fact that this library has a running tab of about 0 open issues (ignoring the reminder issue to support XML soon if demand comes).

Hope this sheds some light. Again, thanks for your feedback and bringing this to my attention.

Enjoy.