ugorji / go

idiomatic codec and rpc lib for msgpack, cbor, json, etc. msgpack.org[Go]
MIT License
1.87k stars 295 forks source link

embedded (anonymous) pointer is not supported #19

Closed aarondl closed 11 years ago

aarondl commented 11 years ago

Is this expected to fail?

package main

import (
    "bytes"
    "github.com/ugorji/go/codec"
)

var msgpackHandle = new(codec.MsgpackHandle)

type A struct {
    anInt int
}

type B struct {
    *A
    moreInt int
}

func main() {
    var buf bytes.Buffer
    encoder := codec.NewEncoder(&buf, msgpackHandle)
    err := encoder.Encode(&B{&A{5}, 6})
    if err != nil {
        panic(err)
    }
}
ugorji commented 11 years ago

what failure did you get?

aarondl commented 11 years ago

reflect: NumField of non-struct type This is from the reflect package's type.go line# 645.

ugorji commented 11 years ago

Good catch. I updated the issue.

It doesn't cause a panic - the panic is caused because your code called panic.

But yes, an embedded pointer is not supported.

I'm looking into it now. Great catch. Much appreciated.

aarondl commented 11 years ago

Wow ultra fail. Missed the correct button. I meant to say that I didn't say that in panic'd just that it failed to work as expected :)

ugorji commented 11 years ago

Not a quick fix.

The code doesn't work for any embedded pointers.

I've fixed your use case but want to ensure all the different variants work i.e

type Z int 
type A struct {
    AnInt int
}

type B struct {
    *Z
    *A
    MoreInt int
}

Also, FYI, we don't encode/decode private (unexported) fields. So they have to be AnInt, and MoreInt.

I'd probably get the fix out within 24-48 hours. I'd fix now, let it soak, and sleep over it before submitting, make sure I'm not missing anything.

ugorji commented 11 years ago

Fixed by commit 8eedb401112a7ab982fa9fffd49c3d0174399f2e