code review 12681044: encoding/gob: support new generic interfaces in package...

encoding/gob: support new generic interfaces in package encoding

[rsc, 2013-08-09 18:00:45]

Hello r (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[r, 2013-08-10 21:53:34]

https://codereview.appspot.com/12681044/diff/6001/src/pkg/encoding/gob/doc.go
File src/pkg/encoding/gob/doc.go (right):

https://codereview.appspot.com/12681044/diff/6001/src/pkg/encoding/gob/doc.go...
src/pkg/encoding/gob/doc.go:87: by calling the corresponding method, again in
that order of preference.
minor point but these comments are formatted to a narrower line length than the
others

https://codereview.appspot.com/12681044/diff/6001/src/pkg/encoding/gob/type.go
File src/pkg/encoding/gob/type.go (right):

https://codereview.appspot.com/12681044/diff/6001/src/pkg/encoding/gob/type.g...
src/pkg/encoding/gob/type.go:31: encIndir            int8         // number of
indirections to reach the receiver type; may be negative
with six booleans and the usage pattern i see, i suggest making these bits in a
pair of words accessed by methods. you can then have one method per enc/dec,
something like

func (ut *userTypeInfo) externallyEncoded() bool {
  return ut.externalEncoders != 0
}

plus

const (
   isGobEncoder = 1 << iota
   isBinaryMarshaler
   isTextMarshaler
)

and the obvious getters. i don't think you need the setters.

[rsc, 2013-08-13 14:24:37]

PTAL

https://codereview.appspot.com/12681044/diff/6001/src/pkg/encoding/gob/doc.go
File src/pkg/encoding/gob/doc.go (right):

https://codereview.appspot.com/12681044/diff/6001/src/pkg/encoding/gob/doc.go...
src/pkg/encoding/gob/doc.go:87: by calling the corresponding method, again in
that order of preference.
On 2013/08/10 21:53:35, r wrote:
> minor point but these comments are formatted to a narrower line length than
the
> others

godoc will reflow them in both text and html mode. i ran |9 fmt -l 80

https://codereview.appspot.com/12681044/diff/6001/src/pkg/encoding/gob/type.go
File src/pkg/encoding/gob/type.go (right):

https://codereview.appspot.com/12681044/diff/6001/src/pkg/encoding/gob/type.g...
src/pkg/encoding/gob/type.go:31: encIndir            int8         // number of
indirections to reach the receiver type; may be negative
On 2013/08/10 21:53:35, r wrote:
> with six booleans and the usage pattern i see, i suggest making these bits in
a
> pair of words accessed by methods. you can then have one method per enc/dec,
> something like
> 
> func (ut *userTypeInfo) externallyEncoded() bool {
>   return ut.externalEncoders != 0
> }
> 
> plus
> 
> const (
>    isGobEncoder = 1 << iota
>    isBinaryMarshaler
>    isTextMarshaler
> )
> 
> and the obvious getters. i don't think you need the setters.

Done.

[r, 2013-08-13 22:29:32]

LGTM

[rsc, 2013-08-14 04:18:52]

*** Submitted as https://code.google.com/p/go/source/detail?r=730fafd3bf98 ***

encoding/gob: support new generic interfaces in package encoding

R=r
CC=golang-dev
https://codereview.appspot.com/12681044