Skip to content

Commit

Permalink
codec: do not create Selfer for types which already have implemented …
Browse files Browse the repository at this point in the history 
…Selfer

Update #142
  • Loading branch information
@ugorji
ugorji committed Feb 17, 2016
1 parent d2ab011 commit 5d64d76
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions codec/gen.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,16 @@ type genRunner struct {
//
// Library users: *DO NOT USE IT DIRECTLY. IT WILL CHANGE CONTINOUSLY WITHOUT NOTICE.*
func Gen(w io.Writer, buildTags, pkgName, uid string, useUnsafe bool, ti *TypeInfos, typ ...reflect.Type) {
// trim out all types which already implement Selfer
typ2 := make([]reflect.Type, 0, len(typ))
for _, t := range typ {
if reflect.PtrTo(t).Implements(selferTyp) || t.Implements(selferTyp) {
continue
}
typ2 = append(typ2, t)
}
typ = typ2

if len(typ) == 0 {
return
}
Expand Down

4 comments on commit 5d64d76

@sttts
Copy link

@sttts sttts commented on 5d64d76 Jan 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces a regression if an embedded struct comes from another package:

package xyz

type Bar struct {
  otherpackage.Foo `json:"foo"`
  x string
}

The other package will have generated codecs by the time xyz is processed. The upper test will skip the generation for Bar such that the promoted self-codec funcs are used. These won't en/decode x.

@sttts
Copy link

@sttts sttts commented on 5d64d76 Jan 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cc @ugorji ^^

@sttts
Copy link

@sttts sttts commented on 5d64d76 Jan 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a quick-n-dirty proof of concept to detect promoted selfer funcs: kubernetes/kubernetes@383fd9b Unfortunately, with reflection we don't get this information.

@ugorji
Copy link
Owner Author

@ugorji ugorji commented on 5d64d76 Jan 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sttts i responded to the original issue at #185

Please sign in to comment.