From 493757ed0093b937332f17f123e98b5734edbafd Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Thu, 14 Jul 2016 20:57:30 +0300 Subject: [PATCH 01/32] add ability to use numeric labels in map --- README.md | 9 +++ gen/decode.go | 42 ++++++------ gen/elem.go | 6 +- gen/encode.go | 17 +---- gen/marshal.go | 19 +----- gen/size.go | 11 ++- gen/spec.go | 162 +++++++++++++++++++++++++++++++++++++++++++- gen/unmarshal.go | 48 ++++++------- msgp/file_test.go | 3 +- msgp/read.go | 1 + parse/directives.go | 3 +- parse/getast.go | 37 +++++++++- printer/print.go | 7 +- 13 files changed, 271 insertions(+), 94 deletions(-) diff --git a/README.md b/README.md index a7cc849c..520c8752 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,15 @@ type Person struct { unexported bool // this field is also ignored } ``` +If you need to have numeric labels in your structs, you could set it in `msg:"(int/uint)value"` notation: +```go +type Person struct { + Name string `msg:"(int)0x01"` + Address string `msg:"(int)0x02"` + Age int `msg:"(int)0x03"` +} +``` +> Note that `uint` typed field labels will be serialized as `msgp.fixint` in case when label value is <= (1<<7)-1 By default, the code generator will satisfy `msgp.Sizer`, `msgp.Encodable`, `msgp.Decodable`, `msgp.Marshaler`, and `msgp.Unmarshaler`. Carefully-designed applications can use these methods to do diff --git a/gen/decode.go b/gen/decode.go index 5367ad3e..7e22d256 100644 --- a/gen/decode.go +++ b/gen/decode.go @@ -63,14 +63,32 @@ func (d *decodeGen) gStruct(s *Struct) { return } -func (d *decodeGen) assignAndCheck(name string, typ string) { +func (d *decodeGen) assignAndCheck(name string, base string) { if !d.p.ok() { return } - d.p.printf("\n%s, err = dc.Read%s()", name, typ) + if base == mapKey { + d.p.printf("\n%s, err = dc.ReadMapKeyPtr()", name) + } else { + d.p.printf("\n%s, err = dc.Read%s()", name, base) + } + d.p.print(errcheck) } +func (u *decodeGen) nextTypeAndCheck(name string) { + if !u.p.ok() { + return + } + u.p.printf("\n%s, err = dc.NextType()", name) + u.p.print(errcheck) +} + +func (u *decodeGen) skipAndCheck() { + u.p.print("\nerr = dc.Skip()") + u.p.print(errcheck) +} + func (d *decodeGen) structAsTuple(s *Struct) { nfields := len(s.Fields) @@ -87,25 +105,7 @@ func (d *decodeGen) structAsTuple(s *Struct) { } func (d *decodeGen) structAsMap(s *Struct) { - d.needsField() - sz := randIdent() - d.p.declare(sz, u32) - d.assignAndCheck(sz, mapHeader) - - d.p.printf("\nfor %s > 0 {\n%s--", sz, sz) - d.assignAndCheck("field", mapKey) - d.p.print("\nswitch msgp.UnsafeString(field) {") - for i := range s.Fields { - d.p.printf("\ncase \"%s\":", s.Fields[i].FieldTag) - next(d, s.Fields[i].FieldElem) - if !d.p.ok() { - return - } - } - d.p.print("\ndefault:\nerr = dc.Skip()") - d.p.print(errcheck) - d.p.closeblock() // close switch - d.p.closeblock() // close for loop + genStructFieldsParser(d, d.p, s.Fields) } func (d *decodeGen) gBase(b *BaseElem) { diff --git a/gen/elem.go b/gen/elem.go index 250c187e..8f639e3e 100644 --- a/gen/elem.go +++ b/gen/elem.go @@ -396,9 +396,9 @@ func (s *Struct) Complexity() int { } type StructField struct { - FieldTag string // the string inside the `msg:""` tag - FieldName string // the name of the struct field - FieldElem Elem // the field type + FieldTag interface{} // the label of the struct field in msgpack + FieldName string // the name of the struct field + FieldElem Elem // the field type } // BaseElem is an element that diff --git a/gen/encode.go b/gen/encode.go index b3415286..596a2378 100644 --- a/gen/encode.go +++ b/gen/encode.go @@ -2,8 +2,9 @@ package gen import ( "fmt" - "github.com/tinylib/msgp/msgp" "io" + + "github.com/tinylib/msgp/msgp" ) func encode(w io.Writer) *encodeGen { @@ -101,19 +102,7 @@ func (e *encodeGen) appendraw(bts []byte) { } func (e *encodeGen) structmap(s *Struct) { - nfields := len(s.Fields) - data := msgp.AppendMapHeader(nil, uint32(nfields)) - e.p.printf("\n// map header, size %d", nfields) - e.Fuse(data) - for i := range s.Fields { - if !e.p.ok() { - return - } - data = msgp.AppendString(nil, s.Fields[i].FieldTag) - e.p.printf("\n// write %q", s.Fields[i].FieldTag) - e.Fuse(data) - next(e, s.Fields[i].FieldElem) - } + genStructFieldsSerializer(e, e.p, s.Fields) } func (e *encodeGen) gMap(m *Map) { diff --git a/gen/marshal.go b/gen/marshal.go index 8e9a4765..c7ae18f3 100644 --- a/gen/marshal.go +++ b/gen/marshal.go @@ -2,8 +2,9 @@ package gen import ( "fmt" - "github.com/tinylib/msgp/msgp" "io" + + "github.com/tinylib/msgp/msgp" ) func marshal(w io.Writer) *marshalGen { @@ -96,21 +97,7 @@ func (m *marshalGen) tuple(s *Struct) { } func (m *marshalGen) mapstruct(s *Struct) { - data := make([]byte, 0, 64) - data = msgp.AppendMapHeader(data, uint32(len(s.Fields))) - m.p.printf("\n// map header, size %d", len(s.Fields)) - m.Fuse(data) - for i := range s.Fields { - if !m.p.ok() { - return - } - data = msgp.AppendString(nil, s.Fields[i].FieldTag) - - m.p.printf("\n// string %q", s.Fields[i].FieldTag) - m.Fuse(data) - - next(m, s.Fields[i].FieldElem) - } + genStructFieldsSerializer(m, m.p, s.Fields) } // append raw data diff --git a/gen/size.go b/gen/size.go index 5c71ec72..c8dbae44 100644 --- a/gen/size.go +++ b/gen/size.go @@ -2,9 +2,10 @@ package gen import ( "fmt" - "github.com/tinylib/msgp/msgp" "io" "strconv" + + "github.com/tinylib/msgp/msgp" ) type sizeState uint8 @@ -107,7 +108,7 @@ func (s *sizeGen) gStruct(st *Struct) { s.addConstant(strconv.Itoa(len(data))) for i := range st.Fields { data = data[:0] - data = msgp.AppendString(data, st.Fields[i].FieldTag) + data, s.p.err = msgp.AppendIntf(data, st.Fields[i].FieldTag) s.addConstant(strconv.Itoa(len(data))) next(s, st.Fields[i].FieldElem) } @@ -238,8 +239,12 @@ func fixedsizeExpr(e Elem) (string, bool) { mhdr := msgp.AppendMapHeader(nil, uint32(len(e.Fields))) hdrlen += len(mhdr) var strbody []byte + var err error for _, f := range e.Fields { - strbody = msgp.AppendString(strbody[:0], f.FieldTag) + strbody, err = msgp.AppendIntf(strbody[:0], f.FieldTag) + if err != nil { + return "", false + } hdrlen += len(strbody) } return fmt.Sprintf("%d + %s", hdrlen, str), true diff --git a/gen/spec.go b/gen/spec.go index 26d26df6..a2c7abf1 100644 --- a/gen/spec.go +++ b/gen/spec.go @@ -3,9 +3,14 @@ package gen import ( "fmt" "io" + "math" + + "github.com/tinylib/msgp/msgp" ) const ( + field = "field" + typ = "typ" errcheck = "\nif err != nil { return }" lenAsUint32 = "uint32(len(%s))" literalFmt = "%s" @@ -13,7 +18,7 @@ const ( quotedFmt = `"%s"` mapHeader = "MapHeader" arrayHeader = "ArrayHeader" - mapKey = "MapKeyPtr" + mapKey = "MapKey" stringTyp = "String" u32 = "uint32" ) @@ -199,6 +204,161 @@ type traversal interface { gStruct(*Struct) } +type assigner interface { + assignAndCheck(name, base string) + nextTypeAndCheck(name string) + skipAndCheck() +} + +type fuser interface { + Fuse([]byte) +} + +type traversalAssigner interface { + traversal + assigner +} + +type traversalFuser interface { + traversal + fuser +} + +func genStructFieldsSerializer(t traversalFuser, p printer, fields []StructField) { + data := msgp.AppendMapHeader(nil, uint32(len(fields))) + p.printf("\n// map header, size %d", len(fields)) + t.Fuse(data) + for _, f := range fields { + if !p.ok() { + return + } + + data, p.err = msgp.AppendIntf(nil, f.FieldTag) + p.printf( + "\n// write struct field key %q = %v; declared as type %T, eventually become msgp.%s", + f.FieldName, f.FieldTag, f.FieldTag, msgp.NextType(data), + ) + t.Fuse(data) + + next(t, f.FieldElem) + } +} + +func genStructFieldsParser(t traversalAssigner, p printer, fields []StructField) { + sz := randIdent() + p.declare(sz, u32) + t.assignAndCheck(sz, mapHeader) + + p.printf("\nfor %s > 0 {\n%s--", sz, sz) + + groups := groupFieldsByType(fields) + hasUint := len(groups[msgp.UintType]) > 0 + hasInt := len(groups[msgp.IntType]) > 0 + hasStr := len(groups[msgp.StrType]) > 0 + + if !hasUint && !hasInt { // there are no numerically labeled fields, so parse every field label as string + p.declare(field, "[]byte") + t.assignAndCheck(field, mapKey) + switchFieldKeysStr(t, p, fields) + } else { + // switch on inferred type of next field + p.declare(typ, "msgp.Type") + t.nextTypeAndCheck(typ) + p.printf("\nswitch %s {", typ) + + if hasUint { + p.print("\ncase msgp.UintType:") + p.declare(field, "uint64") + t.assignAndCheck(field, "Uint64") + switchFieldKeys(t, p, groups[msgp.UintType]) + + // Append to int fields also uint fields that do not overflow int64. + // This is necessary because uint field with value <= (1<<7)-1 could be serialized as fixint. + // and become a msgp.IntType. This is done for best compatibility with other libraries + // (and with other languages). That is, some endpoint could serialize uint16 key with value <= (1<<7)-1 as + // real msgpack uint16, but also could serialize it like fixint. + for _, f := range groups[msgp.UintType] { + v := f.FieldTag.(uint64) + if v <= math.MaxInt64 { + groups[msgp.IntType] = append(groups[msgp.IntType], f) + hasInt = true + } + } + } + if hasInt { + p.print("\ncase msgp.IntType:") + p.declare(field, "int64") + t.assignAndCheck(field, "Int64") + switchFieldKeys(t, p, groups[msgp.IntType]) + } + if hasStr { + // double case is done for backward compatibility with previous implementation + p.print("\ncase msgp.StrType, msgp.BinType:") + p.declare(field, "[]byte") + t.assignAndCheck(field, mapKey) + switchFieldKeysStr(t, p, groups[msgp.StrType]) + } + + p.print("\ndefault:") + t.skipAndCheck() + + p.closeblock() // close switch + } + + p.closeblock() // close loop +} + +func groupFieldsByType(fields []StructField) map[msgp.Type][]StructField { + groups := make(map[msgp.Type][]StructField, len(fields)) + for _, f := range fields { + var t msgp.Type + switch f.FieldTag.(type) { + case int, int8, int16, int32, int64: + t = msgp.IntType + case uint, uint8, byte, uint16, uint32, uint64: + t = msgp.UintType + case string: + t = msgp.StrType + default: + panic(fmt.Errorf("field %q has unknown tag type %T", f.FieldName, f.FieldTag)) + } + groups[t] = append(groups[t], f) + } + return groups +} + +func switchFieldKeysStr(t traversalAssigner, p printer, fields []StructField) { + p.printf("\nswitch msgp.UnsafeString(%s) {", field) + for _, f := range fields { + p.printf("\ncase \"%s\":", f.FieldTag) + next(t, f.FieldElem) + if !p.ok() { + return + } + } + + p.print("\ndefault:") + t.skipAndCheck() + + p.closeblock() // close switch +} + +func switchFieldKeys(t traversalAssigner, p printer, fields []StructField) { + p.printf("\nswitch %s {", field) + for _, f := range fields { + p.printf("\ncase %v:", f.FieldTag) + next(t, f.FieldElem) + if !p.ok() { + return + } + } + + p.print("\ndefault:") + t.skipAndCheck() + + p.closeblock() // close switch +} + // type-switch dispatch to the correct // method given the type of 'e' func next(t traversal, e Elem) { diff --git a/gen/unmarshal.go b/gen/unmarshal.go index eadbd841..753d3922 100644 --- a/gen/unmarshal.go +++ b/gen/unmarshal.go @@ -19,14 +19,6 @@ type unmarshalGen struct { func (u *unmarshalGen) Method() Method { return Unmarshal } -func (u *unmarshalGen) needsField() { - if u.hasfield { - return - } - u.p.print("\nvar field []byte; _ = field") - u.hasfield = true -} - func (u *unmarshalGen) Execute(p Elem) error { u.hasfield = false if !u.p.ok() { @@ -51,7 +43,24 @@ func (u *unmarshalGen) assignAndCheck(name string, base string) { if !u.p.ok() { return } - u.p.printf("\n%s, bts, err = msgp.Read%sBytes(bts)", name, base) + if base == mapKey { + u.p.printf("\n%s, bts, err = msgp.ReadMapKeyZC(bts)", name) + } else { + u.p.printf("\n%s, bts, err = msgp.Read%sBytes(bts)", name, base) + } + u.p.print(errcheck) +} + +func (u *unmarshalGen) nextTypeAndCheck(name string) { + if !u.p.ok() { + return + } + u.p.printf("\n%s = msgp.NextType(bts)", name) + u.p.print(errcheck) +} + +func (u *unmarshalGen) skipAndCheck() { + u.p.print("\nbts, err = msgp.Skip(bts)") u.p.print(errcheck) } @@ -68,7 +77,6 @@ func (u *unmarshalGen) gStruct(s *Struct) { } func (u *unmarshalGen) tuple(s *Struct) { - // open block sz := randIdent() u.p.declare(sz, u32) @@ -83,25 +91,7 @@ func (u *unmarshalGen) tuple(s *Struct) { } func (u *unmarshalGen) mapstruct(s *Struct) { - u.needsField() - sz := randIdent() - u.p.declare(sz, u32) - u.assignAndCheck(sz, mapHeader) - - u.p.printf("\nfor %s > 0 {", sz) - u.p.printf("\n%s--; field, bts, err = msgp.ReadMapKeyZC(bts)", sz) - u.p.print(errcheck) - u.p.print("\nswitch msgp.UnsafeString(field) {") - for i := range s.Fields { - if !u.p.ok() { - return - } - u.p.printf("\ncase \"%s\":", s.Fields[i].FieldTag) - next(u, s.Fields[i].FieldElem) - } - u.p.print("\ndefault:\nbts, err = msgp.Skip(bts)") - u.p.print(errcheck) - u.p.print("\n}\n}") // close switch and for loop + genStructFieldsParser(u, u.p, s.Fields) } func (u *unmarshalGen) gBase(b *BaseElem) { diff --git a/msgp/file_test.go b/msgp/file_test.go index 1cc01cec..9a3c3aed 100644 --- a/msgp/file_test.go +++ b/msgp/file_test.go @@ -5,10 +5,11 @@ package msgp_test import ( "bytes" "crypto/rand" - "github.com/tinylib/msgp/msgp" prand "math/rand" "os" "testing" + + "github.com/tinylib/msgp/msgp" ) type rawBytes []byte diff --git a/msgp/read.go b/msgp/read.go index a493f941..6841a1f7 100644 --- a/msgp/read.go +++ b/msgp/read.go @@ -376,6 +376,7 @@ func (m *Reader) ReadMapKeyPtr() ([]byte, error) { return nil, err } read = int(big.Uint32(p[1:])) + default: return nil, badPrefix(StrType, lead) } diff --git a/parse/directives.go b/parse/directives.go index fb78974b..1110046c 100644 --- a/parse/directives.go +++ b/parse/directives.go @@ -2,9 +2,10 @@ package parse import ( "fmt" - "github.com/tinylib/msgp/gen" "go/ast" "strings" + + "github.com/tinylib/msgp/gen" ) const linePrefix = "//msgp:" diff --git a/parse/getast.go b/parse/getast.go index 355ad772..03ccc953 100644 --- a/parse/getast.go +++ b/parse/getast.go @@ -8,6 +8,7 @@ import ( "os" "reflect" "sort" + "strconv" "strings" "github.com/tinylib/msgp/gen" @@ -346,7 +347,39 @@ func (fs *FileSet) getField(f *ast.Field) []gen.StructField { if tags[0] == "-" { return nil } - sf[0].FieldTag = tags[0] + + if tags[0][0] == '(' { // field key type casting + var err error + + pair := strings.Split(tags[0][1:], ")") + cast, val := pair[0], pair[1] + + switch cast { + case "uint": + if strings.HasPrefix(val, "0x") { + sf[0].FieldTag, err = strconv.ParseUint(strings.TrimPrefix(val, "0x"), 16, 64) + } else { + sf[0].FieldTag, err = strconv.ParseUint(val, 10, 64) + } + + case "int": + if strings.HasPrefix(val, "0x") { + sf[0].FieldTag, err = strconv.ParseInt(strings.TrimPrefix(val, "0x"), 16, 64) + } else { + sf[0].FieldTag, err = strconv.ParseInt(val, 10, 64) + } + + default: + sf[0].FieldTag = val + } + + if err != nil { + panic(fmt.Sprintf("could not parse field %q annotation: %s", f.Names[0].Name, err)) + } + } else { + sf[0].FieldTag = tags[0] + } + } ex := fs.parseExpr(f.Type) @@ -374,7 +407,7 @@ func (fs *FileSet) getField(f *ast.Field) []gen.StructField { return sf } sf[0].FieldElem = ex - if sf[0].FieldTag == "" { + if sf[0].FieldTag == nil { sf[0].FieldTag = sf[0].FieldName } diff --git a/printer/print.go b/printer/print.go index 4766871f..d3ef52d0 100644 --- a/printer/print.go +++ b/printer/print.go @@ -3,13 +3,14 @@ package printer import ( "bytes" "fmt" + "io" + "io/ioutil" + "strings" + "github.com/tinylib/msgp/gen" "github.com/tinylib/msgp/parse" "github.com/ttacon/chalk" "golang.org/x/tools/imports" - "io" - "io/ioutil" - "strings" ) func infof(s string, v ...interface{}) { From fd9ac4f82a4bc09230dbf3a7026222e82d4c3ab7 Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Thu, 14 Jul 2016 21:00:13 +0300 Subject: [PATCH 02/32] readme fixes --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 520c8752..5d22bbde 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ type Person struct { unexported bool // this field is also ignored } ``` -If you need to have numeric labels in your structs, you could set it in `msg:"(int/uint)value"` notation: +If you need to have numeric labels for a struct fields, you could set it in `msg:"(int/uint)value"` notation: ```go type Person struct { Name string `msg:"(int)0x01"` @@ -50,7 +50,7 @@ type Person struct { Age int `msg:"(int)0x03"` } ``` -> Note that `uint` typed field labels will be serialized as `msgp.fixint` in case when label value is <= (1<<7)-1 +> Note that field labels with `uint` value will be serialized as `msgp.fixint` in case when label value is <= (1<<7)-1 By default, the code generator will satisfy `msgp.Sizer`, `msgp.Encodable`, `msgp.Decodable`, `msgp.Marshaler`, and `msgp.Unmarshaler`. Carefully-designed applications can use these methods to do From d57ab0aae507eab1498fb0a2d0ca5514eb020ba4 Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Thu, 14 Jul 2016 21:08:10 +0300 Subject: [PATCH 03/32] bugfix --- gen/spec.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gen/spec.go b/gen/spec.go index a2c7abf1..66b1121b 100644 --- a/gen/spec.go +++ b/gen/spec.go @@ -315,7 +315,7 @@ func groupFieldsByType(fields []StructField) map[msgp.Type][]StructField { switch f.FieldTag.(type) { case int, int8, int16, int32, int64: t = msgp.IntType - case uint, uint8, byte, uint16, uint32, uint64: + case uint, uint8, uint16, uint32, uint64: t = msgp.UintType case string: t = msgp.StrType From 97823d4ddff72a487d97ce7a8a2c14373beb4a1a Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Thu, 14 Jul 2016 21:18:34 +0300 Subject: [PATCH 04/32] fix tests --- parse/getast.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse/getast.go b/parse/getast.go index 03ccc953..bc87c6e9 100644 --- a/parse/getast.go +++ b/parse/getast.go @@ -348,7 +348,7 @@ func (fs *FileSet) getField(f *ast.Field) []gen.StructField { return nil } - if tags[0][0] == '(' { // field key type casting + if len(tags[0]) > 2 && tags[0][0] == '(' { // field key type casting var err error pair := strings.Split(tags[0][1:], ")") From 0108cd32344d6130a031e6ed38fabde055bda4b8 Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Fri, 15 Jul 2016 11:35:22 +0300 Subject: [PATCH 05/32] bugfix with empty field tag --- parse/getast.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/parse/getast.go b/parse/getast.go index bc87c6e9..b40647f8 100644 --- a/parse/getast.go +++ b/parse/getast.go @@ -370,13 +370,12 @@ func (fs *FileSet) getField(f *ast.Field) []gen.StructField { } default: - sf[0].FieldTag = val + panic(fmt.Sprintf("unsupported field %q cast annotation: type %s is not acceptable", f.Names[0].Name, cast)) } - if err != nil { panic(fmt.Sprintf("could not parse field %q annotation: %s", f.Names[0].Name, err)) } - } else { + } else if tags[0] != "" { sf[0].FieldTag = tags[0] } From 94522709ca8a7e71d0a1d9d216ece6d5ddd3ccf3 Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Fri, 15 Jul 2016 11:56:50 +0300 Subject: [PATCH 06/32] add generated files to clean task --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 81b8b126..9ae72018 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,7 @@ # normal `go install`. # generated integration test files -GGEN = ./_generated/generated.go ./_generated/generated_test.go +GGEN = ./_generated/generated.go ./_generated/generated_test.go ./_generated/issue94_gen.go ./_generated/issue94_gen_test.go # generated unit test files MGEN = ./msgp/defgen_test.go From 6da8d3d71e6e96697d5a2e013e1c85355faf43ab Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Fri, 15 Jul 2016 12:48:52 +0300 Subject: [PATCH 07/32] declare vars out of the loop --- gen/spec.go | 95 +++++++++++++++++++++++++++++------------------------ 1 file changed, 52 insertions(+), 43 deletions(-) diff --git a/gen/spec.go b/gen/spec.go index 66b1121b..af2fb55d 100644 --- a/gen/spec.go +++ b/gen/spec.go @@ -9,8 +9,6 @@ import ( ) const ( - field = "field" - typ = "typ" errcheck = "\nif err != nil { return }" lenAsUint32 = "uint32(len(%s))" literalFmt = "%s" @@ -235,8 +233,8 @@ func genStructFieldsSerializer(t traversalFuser, p printer, fields []StructField data, p.err = msgp.AppendIntf(nil, f.FieldTag) p.printf( - "\n// write struct field key %q = %v; declared as type %T, eventually become msgp.%s", - f.FieldName, f.FieldTag, f.FieldTag, msgp.NextType(data), + "\n// write label for the struct field %q: %v typed as msgp.%s", + f.FieldName, f.FieldTag, msgp.NextType(data), ) t.Fuse(data) @@ -245,63 +243,74 @@ func genStructFieldsSerializer(t traversalFuser, p printer, fields []StructField } func genStructFieldsParser(t traversalAssigner, p printer, fields []StructField) { - sz := randIdent() - p.declare(sz, u32) - t.assignAndCheck(sz, mapHeader) - - p.printf("\nfor %s > 0 {\n%s--", sz, sz) + const ( + fieldBytes = "field_b" + fieldInt = "field_i" + fieldUint = "field_u" + typ = "typ" + ) groups := groupFieldsByType(fields) hasUint := len(groups[msgp.UintType]) > 0 hasInt := len(groups[msgp.IntType]) > 0 hasStr := len(groups[msgp.StrType]) > 0 - if !hasUint && !hasInt { // there are no numerically labeled fields, so parse every field label as string - p.declare(field, "[]byte") - t.assignAndCheck(field, mapKey) - switchFieldKeysStr(t, p, fields) + if hasStr { + p.declare(fieldBytes, "[]byte") + } + if hasUint { + p.declare(fieldUint, "uint64") + + // Append to int fields also uint fields that do not overflow int64. + // This is necessary because uint field with value <= (1<<7)-1 could be serialized as fixint. + // and become a msgp.IntType. This is done for best compatibility with other libraries + // (and with other languages). That is, some endpoint could serialize uint16 key with value <= (1<<7)-1 as + // real msgpack uint16, but also could serialize it like fixint. + for _, f := range groups[msgp.UintType] { + v := f.FieldTag.(uint64) + if v <= math.MaxInt64 { + groups[msgp.IntType] = append(groups[msgp.IntType], f) + hasInt = true + } + } + } + if hasInt { + p.declare(fieldInt, "int64") + } + if hasInt || hasUint { + p.declare(typ, "msgp.Type") + } + + sz := randIdent() + p.declare(sz, u32) + t.assignAndCheck(sz, mapHeader) + p.printf("\nfor %s > 0 {\n%s--", sz, sz) + + if hasStr && !hasUint && !hasInt { // there are no numerically labeled fields, so parse every field label as string + t.assignAndCheck(fieldBytes, mapKey) + switchFieldKeysStr(t, p, fields, fieldBytes) } else { // switch on inferred type of next field - p.declare(typ, "msgp.Type") t.nextTypeAndCheck(typ) p.printf("\nswitch %s {", typ) - if hasUint { p.print("\ncase msgp.UintType:") - p.declare(field, "uint64") - t.assignAndCheck(field, "Uint64") - switchFieldKeys(t, p, groups[msgp.UintType]) - - // Append to int fields also uint fields that do not overflow int64. - // This is necessary because uint field with value <= (1<<7)-1 could be serialized as fixint. - // and become a msgp.IntType. This is done for best compatibility with other libraries - // (and with other languages). That is, some endpoint could serialize uint16 key with value <= (1<<7)-1 as - // real msgpack uint16, but also could serialize it like fixint. - for _, f := range groups[msgp.UintType] { - v := f.FieldTag.(uint64) - if v <= math.MaxInt64 { - groups[msgp.IntType] = append(groups[msgp.IntType], f) - hasInt = true - } - } + t.assignAndCheck(fieldUint, "Uint64") + switchFieldKeys(t, p, groups[msgp.UintType], fieldUint) } if hasInt { p.print("\ncase msgp.IntType:") - p.declare(field, "int64") - t.assignAndCheck(field, "Int64") - switchFieldKeys(t, p, groups[msgp.IntType]) + t.assignAndCheck(fieldInt, "Int64") + switchFieldKeys(t, p, groups[msgp.IntType], fieldInt) } if hasStr { // double case is done for backward compatibility with previous implementation p.print("\ncase msgp.StrType, msgp.BinType:") - p.declare(field, "[]byte") - t.assignAndCheck(field, mapKey) - switchFieldKeysStr(t, p, groups[msgp.StrType]) + t.assignAndCheck(fieldBytes, mapKey) + switchFieldKeysStr(t, p, groups[msgp.StrType], fieldBytes) } - p.print("\ndefault:") t.skipAndCheck() - p.closeblock() // close switch } @@ -327,8 +336,8 @@ func groupFieldsByType(fields []StructField) map[msgp.Type][]StructField { return groups } -func switchFieldKeysStr(t traversalAssigner, p printer, fields []StructField) { - p.printf("\nswitch msgp.UnsafeString(%s) {", field) +func switchFieldKeysStr(t traversalAssigner, p printer, fields []StructField, label string) { + p.printf("\nswitch msgp.UnsafeString(%s) {", label) for _, f := range fields { p.printf("\ncase \"%s\":", f.FieldTag) next(t, f.FieldElem) @@ -343,8 +352,8 @@ func switchFieldKeysStr(t traversalAssigner, p printer, fields []StructField) { p.closeblock() // close switch } -func switchFieldKeys(t traversalAssigner, p printer, fields []StructField) { - p.printf("\nswitch %s {", field) +func switchFieldKeys(t traversalAssigner, p printer, fields []StructField, label string) { + p.printf("\nswitch %s {", label) for _, f := range fields { p.printf("\ncase %v:", f.FieldTag) next(t, f.FieldElem) From 77232d3e7c450c265a4b9bbd40aabdf72f24d75e Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Fri, 15 Jul 2016 14:23:29 +0300 Subject: [PATCH 08/32] use declareOnce pattern --- gen/decode.go | 17 ++++------------- gen/spec.go | 37 ++++++++++++++++++++++++++++++++----- gen/unmarshal.go | 6 +++--- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/gen/decode.go b/gen/decode.go index 7e22d256..3e1a73a5 100644 --- a/gen/decode.go +++ b/gen/decode.go @@ -7,33 +7,24 @@ import ( func decode(w io.Writer) *decodeGen { return &decodeGen{ - p: printer{w: w}, - hasfield: false, + p: printer{w: w}, } } type decodeGen struct { passes - p printer - hasfield bool + fields + p printer } func (d *decodeGen) Method() Method { return Decode } -func (d *decodeGen) needsField() { - if d.hasfield { - return - } - d.p.print("\nvar field []byte; _ = field") - d.hasfield = true -} - func (d *decodeGen) Execute(p Elem) error { p = d.applyall(p) if p == nil { return nil } - d.hasfield = false + d.fields.drop() if !d.p.ok() { return d.p.err } diff --git a/gen/spec.go b/gen/spec.go index af2fb55d..8e2c88f5 100644 --- a/gen/spec.go +++ b/gen/spec.go @@ -193,6 +193,28 @@ func (p *passes) applyall(e Elem) Elem { return e } +type fields struct { + cache map[string]bool +} + +func (f *fields) declareOnce(p printer, name, typ string) { + key := name + "." + typ + if f.cache == nil { + f.cache = make(map[string]bool) + } else if f.cache[key] { + return + } + + p.printf("\nvar %s %s;", name, typ) + f.cache[key] = true +} + +func (f *fields) drop() { + for k := range f.cache { + delete(f.cache, k) + } +} + type traversal interface { gMap(*Map) gSlice(*Slice) @@ -202,6 +224,10 @@ type traversal interface { gStruct(*Struct) } +type declarer interface { + declareOnce(p printer, name, typ string) +} + type assigner interface { assignAndCheck(name, base string) nextTypeAndCheck(name string) @@ -215,6 +241,7 @@ type fuser interface { type traversalAssigner interface { traversal assigner + declarer } type traversalFuser interface { @@ -233,7 +260,7 @@ func genStructFieldsSerializer(t traversalFuser, p printer, fields []StructField data, p.err = msgp.AppendIntf(nil, f.FieldTag) p.printf( - "\n// write label for the struct field %q: %v typed as msgp.%s", + "\n// [field %q] write label `%v` as msgp.%s", f.FieldName, f.FieldTag, msgp.NextType(data), ) t.Fuse(data) @@ -256,10 +283,10 @@ func genStructFieldsParser(t traversalAssigner, p printer, fields []StructField) hasStr := len(groups[msgp.StrType]) > 0 if hasStr { - p.declare(fieldBytes, "[]byte") + t.declareOnce(p, fieldBytes, "[]byte") } if hasUint { - p.declare(fieldUint, "uint64") + t.declareOnce(p, fieldUint, "uint64") // Append to int fields also uint fields that do not overflow int64. // This is necessary because uint field with value <= (1<<7)-1 could be serialized as fixint. @@ -275,10 +302,10 @@ func genStructFieldsParser(t traversalAssigner, p printer, fields []StructField) } } if hasInt { - p.declare(fieldInt, "int64") + t.declareOnce(p, fieldInt, "int64") } if hasInt || hasUint { - p.declare(typ, "msgp.Type") + t.declareOnce(p, typ, "msgp.Type") } sz := randIdent() diff --git a/gen/unmarshal.go b/gen/unmarshal.go index 753d3922..ddf8c764 100644 --- a/gen/unmarshal.go +++ b/gen/unmarshal.go @@ -13,14 +13,14 @@ func unmarshal(w io.Writer) *unmarshalGen { type unmarshalGen struct { passes - p printer - hasfield bool + fields + p printer } func (u *unmarshalGen) Method() Method { return Unmarshal } func (u *unmarshalGen) Execute(p Elem) error { - u.hasfield = false + u.fields.drop() if !u.p.ok() { return u.p.err } From 79ca299c9aae5b29e9709e36ba6119d1f9bc7e6d Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Fri, 15 Jul 2016 14:51:36 +0300 Subject: [PATCH 09/32] optimize gen for single typed labels --- gen/spec.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/gen/spec.go b/gen/spec.go index 8e2c88f5..fe594b14 100644 --- a/gen/spec.go +++ b/gen/spec.go @@ -281,6 +281,7 @@ func genStructFieldsParser(t traversalAssigner, p printer, fields []StructField) hasUint := len(groups[msgp.UintType]) > 0 hasInt := len(groups[msgp.IntType]) > 0 hasStr := len(groups[msgp.StrType]) > 0 + singleType := len(groups) == 1 if hasStr { t.declareOnce(p, fieldBytes, "[]byte") @@ -304,7 +305,7 @@ func genStructFieldsParser(t traversalAssigner, p printer, fields []StructField) if hasInt { t.declareOnce(p, fieldInt, "int64") } - if hasInt || hasUint { + if !singleType || hasUint { t.declareOnce(p, typ, "msgp.Type") } @@ -312,11 +313,14 @@ func genStructFieldsParser(t traversalAssigner, p printer, fields []StructField) p.declare(sz, u32) t.assignAndCheck(sz, mapHeader) p.printf("\nfor %s > 0 {\n%s--", sz, sz) - - if hasStr && !hasUint && !hasInt { // there are no numerically labeled fields, so parse every field label as string + switch { + case singleType && hasStr: t.assignAndCheck(fieldBytes, mapKey) switchFieldKeysStr(t, p, fields, fieldBytes) - } else { + case singleType && hasInt: + t.assignAndCheck(fieldInt, "Int64") + switchFieldKeys(t, p, fields, fieldInt) + default: // switch on inferred type of next field t.nextTypeAndCheck(typ) p.printf("\nswitch %s {", typ) @@ -340,7 +344,6 @@ func genStructFieldsParser(t traversalAssigner, p printer, fields []StructField) t.skipAndCheck() p.closeblock() // close switch } - p.closeblock() // close loop } @@ -372,10 +375,8 @@ func switchFieldKeysStr(t traversalAssigner, p printer, fields []StructField, la return } } - p.print("\ndefault:") t.skipAndCheck() - p.closeblock() // close switch } From 84311a78aec1fd5af4a50f8516fb9f6da91d3737 Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Fri, 15 Jul 2016 14:51:45 +0300 Subject: [PATCH 10/32] add tests for numeric labels --- _generated/def.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/_generated/def.go b/_generated/def.go index 13b37e99..9bdf285a 100644 --- a/_generated/def.go +++ b/_generated/def.go @@ -53,6 +53,17 @@ type TestType struct { Slice2 []string } +type TestNumericLabels struct { + One string `msg:"(int)0x01"` + Two string `msg:"(uint)0xffffffffffffffff"` +} + +type TestOnlyIntLabels struct { + A string `msg:"(int)0xfa"` + B string `msg:"(int)0xfb"` + C string `msg:"(int)0xfc"` +} + //msgp:tuple Object type Object struct { ObjectNo string `msg:"objno"` From 97f80c96af5223a711e2a3fcaa1ba29969c1f40f Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Fri, 15 Jul 2016 15:00:56 +0300 Subject: [PATCH 11/32] code style --- gen/spec.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/gen/spec.go b/gen/spec.go index fe594b14..d6666722 100644 --- a/gen/spec.go +++ b/gen/spec.go @@ -389,10 +389,8 @@ func switchFieldKeys(t traversalAssigner, p printer, fields []StructField, label return } } - p.print("\ndefault:") t.skipAndCheck() - p.closeblock() // close switch } From f7c196e11b50d360ba131a4012b649acc64bf367 Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Fri, 15 Jul 2016 15:02:32 +0300 Subject: [PATCH 12/32] comment --- parse/getast.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/parse/getast.go b/parse/getast.go index b40647f8..a97e979a 100644 --- a/parse/getast.go +++ b/parse/getast.go @@ -343,12 +343,14 @@ func (fs *FileSet) getField(f *ast.Field) []gen.StructField { if len(tags) == 2 && tags[1] == "extension" { extension = true } + // ignore "-" fields if tags[0] == "-" { return nil } - if len(tags[0]) > 2 && tags[0][0] == '(' { // field key type casting + // field label type casting + if len(tags[0]) > 2 && tags[0][0] == '(' { var err error pair := strings.Split(tags[0][1:], ")") @@ -378,7 +380,6 @@ func (fs *FileSet) getField(f *ast.Field) []gen.StructField { } else if tags[0] != "" { sf[0].FieldTag = tags[0] } - } ex := fs.parseExpr(f.Type) From af829af451564d67f79ff92f2366fbbc12d08ac5 Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Fri, 22 Jul 2016 14:02:17 +0300 Subject: [PATCH 13/32] use different syntax, refactoring --- _generated/def.go | 16 ++++++------- gen/spec.go | 2 ++ parse/getast.go | 61 ++++++++++++++++++++++++++--------------------- 3 files changed, 44 insertions(+), 35 deletions(-) diff --git a/_generated/def.go b/_generated/def.go index 9bdf285a..2e3ff905 100644 --- a/_generated/def.go +++ b/_generated/def.go @@ -41,9 +41,9 @@ type TestType struct { F *float64 `msg:"float"` Els map[string]string `msg:"elements"` Obj struct { // test anonymous struct - ValueA string `msg:"value_a"` - ValueB []byte `msg:"value_b"` - } `msg:"object"` + ValueA string `msg:"value_a"` + ValueB []byte `msg:"value_b"` + } `msg:"object"` Child *TestType `msg:"child"` Time time.Time `msg:"time"` Any interface{} `msg:"any"` @@ -54,14 +54,14 @@ type TestType struct { } type TestNumericLabels struct { - One string `msg:"(int)0x01"` - Two string `msg:"(uint)0xffffffffffffffff"` + One string `msg:"0x01,int"` + Two string `msg:"0xffffffffffffffff,uint"` } type TestOnlyIntLabels struct { - A string `msg:"(int)0xfa"` - B string `msg:"(int)0xfb"` - C string `msg:"(int)0xfc"` + A string `msg:"0xfa,int"` + B string `msg:"0xfb,int"` + C string `msg:"0xfc,int"` } //msgp:tuple Object diff --git a/gen/spec.go b/gen/spec.go index d6666722..e6c82ab1 100644 --- a/gen/spec.go +++ b/gen/spec.go @@ -317,9 +317,11 @@ func genStructFieldsParser(t traversalAssigner, p printer, fields []StructField) case singleType && hasStr: t.assignAndCheck(fieldBytes, mapKey) switchFieldKeysStr(t, p, fields, fieldBytes) + case singleType && hasInt: t.assignAndCheck(fieldInt, "Int64") switchFieldKeys(t, p, fields, fieldInt) + default: // switch on inferred type of next field t.nextTypeAndCheck(typ) diff --git a/parse/getast.go b/parse/getast.go index a97e979a..0f83c09c 100644 --- a/parse/getast.go +++ b/parse/getast.go @@ -325,7 +325,7 @@ func (fs *FileSet) parseFieldList(fl *ast.FieldList) []gen.StructField { if len(fds) > 0 { out = append(out, fds...) } else { - warnln("ignored.") + warnln("ignored") } popstate() } @@ -340,45 +340,33 @@ func (fs *FileSet) getField(f *ast.Field) []gen.StructField { if f.Tag != nil { body := reflect.StructTag(strings.Trim(f.Tag.Value, "`")).Get("msg") tags := strings.Split(body, ",") - if len(tags) == 2 && tags[1] == "extension" { - extension = true - } // ignore "-" fields if tags[0] == "-" { return nil } - // field label type casting - if len(tags[0]) > 2 && tags[0][0] == '(' { - var err error + if tags[0] != "" { + sf[0].FieldTag = tags[0] + } - pair := strings.Split(tags[0][1:], ")") - cast, val := pair[0], pair[1] + if len(tags) > 1 { + last := len(tags) - 1 + extension = tags[last] == "extension" - switch cast { + var err error + switch tags[1] { case "uint": - if strings.HasPrefix(val, "0x") { - sf[0].FieldTag, err = strconv.ParseUint(strings.TrimPrefix(val, "0x"), 16, 64) - } else { - sf[0].FieldTag, err = strconv.ParseUint(val, 10, 64) - } - + str, base := numeric(tags[0]) + sf[0].FieldTag, err = strconv.ParseUint(str, base, 64) case "int": - if strings.HasPrefix(val, "0x") { - sf[0].FieldTag, err = strconv.ParseInt(strings.TrimPrefix(val, "0x"), 16, 64) - } else { - sf[0].FieldTag, err = strconv.ParseInt(val, 10, 64) - } - - default: - panic(fmt.Sprintf("unsupported field %q cast annotation: type %s is not acceptable", f.Names[0].Name, cast)) + str, base := numeric(tags[0]) + sf[0].FieldTag, err = strconv.ParseInt(str, base, 64) } if err != nil { - panic(fmt.Sprintf("could not parse field %q annotation: %s", f.Names[0].Name, err)) + warnf("could not parse field label %q as msgp.%s: %s\n", tags[0], tags[1], err) + return nil } - } else if tags[0] != "" { - sf[0].FieldTag = tags[0] } } @@ -431,6 +419,25 @@ func (fs *FileSet) getField(f *ast.Field) []gen.StructField { return sf } +// numeric extracts the base and string representation of number literal. +func numeric(s string) (string, int) { + var base int + switch { + case len(s) > 2 && s[:2] == "0x": + base = 16 + s = s[2:] + case len(s) > 2 && s[:2] == "0b": + base = 2 + s = s[2:] + case len(s) > 1 && s[0] == '0': + base = 8 + s = s[1:] + default: + base = 10 + } + return s, base +} + // extract embedded field name // // so, for a struct like From b2cdf6e1fea819739d1a8852a8dde259e62c587c Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Fri, 22 Jul 2016 14:04:36 +0300 Subject: [PATCH 14/32] update readme --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 5d22bbde..06ced63b 100644 --- a/README.md +++ b/README.md @@ -42,12 +42,12 @@ type Person struct { unexported bool // this field is also ignored } ``` -If you need to have numeric labels for a struct fields, you could set it in `msg:"(int/uint)value"` notation: +If you need to have numeric labels for a struct fields, you could set it as following: ```go type Person struct { - Name string `msg:"(int)0x01"` - Address string `msg:"(int)0x02"` - Age int `msg:"(int)0x03"` + Name string `msg:"0x01,int"` + Address string `msg:"0x02,int"` + Age int `msg:"0x03,int"` } ``` > Note that field labels with `uint` value will be serialized as `msgp.fixint` in case when label value is <= (1<<7)-1 From 91edff861841c132f1eecb9dd6afe82966566d8a Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Fri, 22 Jul 2016 14:07:22 +0300 Subject: [PATCH 15/32] use different literals in readme example --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 06ced63b..8b186c5b 100644 --- a/README.md +++ b/README.md @@ -46,8 +46,9 @@ If you need to have numeric labels for a struct fields, you could set it as foll ```go type Person struct { Name string `msg:"0x01,int"` - Address string `msg:"0x02,int"` - Age int `msg:"0x03,int"` + Address string `msg:"0b10,int"` + Email string `msg:"03,int"` + Age int `msg:"4,int"` } ``` > Note that field labels with `uint` value will be serialized as `msgp.fixint` in case when label value is <= (1<<7)-1 From 62450e625977a84b4df1e868fa1c9ed890b435d2 Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Fri, 22 Jul 2016 14:08:55 +0300 Subject: [PATCH 16/32] add different number literals test --- _generated/def.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/_generated/def.go b/_generated/def.go index 2e3ff905..e1a9e0ee 100644 --- a/_generated/def.go +++ b/_generated/def.go @@ -64,6 +64,14 @@ type TestOnlyIntLabels struct { C string `msg:"0xfc,int"` } + +type TestIntLiterals struct { + A string `msg:"0x01,int"` + B string `msg:"0b10,int"` + C string `msg:"03,int"` + D string `msg:"4,int"` +} + //msgp:tuple Object type Object struct { ObjectNo string `msg:"objno"` From 56a4935da5354802568fa3c8021e2ffba9cec6bc Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Fri, 22 Jul 2016 19:09:03 +0300 Subject: [PATCH 17/32] bug fix with single numeric field --- _generated/def.go | 5 ++++- gen/spec.go | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/_generated/def.go b/_generated/def.go index e1a9e0ee..10bb2d6e 100644 --- a/_generated/def.go +++ b/_generated/def.go @@ -64,7 +64,6 @@ type TestOnlyIntLabels struct { C string `msg:"0xfc,int"` } - type TestIntLiterals struct { A string `msg:"0x01,int"` B string `msg:"0b10,int"` @@ -72,6 +71,10 @@ type TestIntLiterals struct { D string `msg:"4,int"` } +type SingleFieldNumeric struct { + Message string `msg:"0x00,uint"` +} + //msgp:tuple Object type Object struct { ObjectNo string `msg:"objno"` diff --git a/gen/spec.go b/gen/spec.go index e6c82ab1..7c2d8e9c 100644 --- a/gen/spec.go +++ b/gen/spec.go @@ -318,7 +318,7 @@ func genStructFieldsParser(t traversalAssigner, p printer, fields []StructField) t.assignAndCheck(fieldBytes, mapKey) switchFieldKeysStr(t, p, fields, fieldBytes) - case singleType && hasInt: + case singleType && !hasUint && hasInt: t.assignAndCheck(fieldInt, "Int64") switchFieldKeys(t, p, fields, fieldInt) From 26008c68f461b6a4ba827bab411067d6981793ef Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Sat, 23 Jul 2016 11:48:01 +0300 Subject: [PATCH 18/32] change panic message --- gen/spec.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gen/spec.go b/gen/spec.go index 7c2d8e9c..fcb60aa5 100644 --- a/gen/spec.go +++ b/gen/spec.go @@ -361,7 +361,10 @@ func groupFieldsByType(fields []StructField) map[msgp.Type][]StructField { case string: t = msgp.StrType default: - panic(fmt.Errorf("field %q has unknown tag type %T", f.FieldName, f.FieldTag)) + panic(fmt.Sprintf( + "could not generate code to work with field's %q label: has unknown type %T", + f.FieldName, f.FieldTag, + )) } groups[t] = append(groups[t], f) } From 0c1cb94b16e8e03d34c3bff9a695fd6d58e9eb6c Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Tue, 2 Aug 2016 16:27:39 +0300 Subject: [PATCH 19/32] apply review comments --- gen/spec.go | 6 +++--- msgp/read.go | 1 - parse/getast.go | 25 ++----------------------- 3 files changed, 5 insertions(+), 27 deletions(-) diff --git a/gen/spec.go b/gen/spec.go index fcb60aa5..ec3b5cfd 100644 --- a/gen/spec.go +++ b/gen/spec.go @@ -271,9 +271,9 @@ func genStructFieldsSerializer(t traversalFuser, p printer, fields []StructField func genStructFieldsParser(t traversalAssigner, p printer, fields []StructField) { const ( - fieldBytes = "field_b" - fieldInt = "field_i" - fieldUint = "field_u" + fieldBytes = "fieldBytes" + fieldInt = "fieldInt" + fieldUint = "fieldUint" typ = "typ" ) diff --git a/msgp/read.go b/msgp/read.go index 6841a1f7..a493f941 100644 --- a/msgp/read.go +++ b/msgp/read.go @@ -376,7 +376,6 @@ func (m *Reader) ReadMapKeyPtr() ([]byte, error) { return nil, err } read = int(big.Uint32(p[1:])) - default: return nil, badPrefix(StrType, lead) } diff --git a/parse/getast.go b/parse/getast.go index 0f83c09c..cec1ebe2 100644 --- a/parse/getast.go +++ b/parse/getast.go @@ -357,11 +357,9 @@ func (fs *FileSet) getField(f *ast.Field) []gen.StructField { var err error switch tags[1] { case "uint": - str, base := numeric(tags[0]) - sf[0].FieldTag, err = strconv.ParseUint(str, base, 64) + sf[0].FieldTag, err = strconv.ParseUint(tags[0], 0, 64) case "int": - str, base := numeric(tags[0]) - sf[0].FieldTag, err = strconv.ParseInt(str, base, 64) + sf[0].FieldTag, err = strconv.ParseInt(tags[0], 0, 64) } if err != nil { warnf("could not parse field label %q as msgp.%s: %s\n", tags[0], tags[1], err) @@ -419,25 +417,6 @@ func (fs *FileSet) getField(f *ast.Field) []gen.StructField { return sf } -// numeric extracts the base and string representation of number literal. -func numeric(s string) (string, int) { - var base int - switch { - case len(s) > 2 && s[:2] == "0x": - base = 16 - s = s[2:] - case len(s) > 2 && s[:2] == "0b": - base = 2 - s = s[2:] - case len(s) > 1 && s[0] == '0': - base = 8 - s = s[1:] - default: - base = 10 - } - return s, base -} - // extract embedded field name // // so, for a struct like From 7a547e1422917de005979d218264b9ff699a6309 Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Tue, 2 Aug 2016 16:56:23 +0300 Subject: [PATCH 20/32] common internal log package --- internal/log/log.go | 56 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 internal/log/log.go diff --git a/internal/log/log.go b/internal/log/log.go new file mode 100644 index 00000000..7111f3a3 --- /dev/null +++ b/internal/log/log.go @@ -0,0 +1,56 @@ +package log + +import ( + "fmt" + "github.com/ttacon/chalk" + "strings" + "os" +) + +var logctx []string + +// push logging state +func PushState(s string) { + logctx = append(logctx, s) +} + +// pop logging state +func PopState() { + logctx = logctx[:len(logctx)-1] +} + +func Infof(s string, v ...interface{}) { + PushState(s) + fmt.Printf(chalk.Green.Color(strings.Join(logctx, ": ")), v...) + PopState() +} + +func Infoln(s string) { + PushState(s) + fmt.Println(chalk.Green.Color(strings.Join(logctx, ": "))) + PopState() +} + +func Warnf(s string, v ...interface{}) { + PushState(s) + fmt.Printf(chalk.Yellow.Color(strings.Join(logctx, ": ")), v...) + PopState() +} + +func Warnln(s string) { + PushState(s) + fmt.Println(chalk.Yellow.Color(strings.Join(logctx, ": "))) + PopState() +} + +func Fatal(s string) { + PushState(s) + fmt.Print(chalk.Red.Color(strings.Join(logctx, ": "))) + os.Exit(1) +} + +func Fatalf(s string, v ...interface{}) { + PushState(s) + fmt.Printf(chalk.Red.Color(strings.Join(logctx, ": ")), v...) + os.Exit(1) +} From 5fd38c2b32e46c32bd949ab564ddd757e7c9877f Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Tue, 2 Aug 2016 16:56:42 +0300 Subject: [PATCH 21/32] use common log package --- gen/spec.go | 5 ++- parse/directives.go | 14 +++---- parse/getast.go | 100 +++++++++++++------------------------------- parse/inline.go | 13 +++--- printer/print.go | 10 ++--- 5 files changed, 49 insertions(+), 93 deletions(-) diff --git a/gen/spec.go b/gen/spec.go index ec3b5cfd..8e0b4cf8 100644 --- a/gen/spec.go +++ b/gen/spec.go @@ -6,6 +6,7 @@ import ( "math" "github.com/tinylib/msgp/msgp" + "github.com/tinylib/msgp/internal/log" ) const ( @@ -361,10 +362,10 @@ func groupFieldsByType(fields []StructField) map[msgp.Type][]StructField { case string: t = msgp.StrType default: - panic(fmt.Sprintf( + log.Fatalf( "could not generate code to work with field's %q label: has unknown type %T", f.FieldName, f.FieldTag, - )) + ) } groups[t] = append(groups[t], f) } diff --git a/parse/directives.go b/parse/directives.go index 1110046c..a5b010c6 100644 --- a/parse/directives.go +++ b/parse/directives.go @@ -31,12 +31,12 @@ var passDirectives = map[string]passDirective{ } func passignore(m gen.Method, text []string, p *gen.Printer) error { - pushstate(m.String()) + log.PushState(m.String()) for _, a := range text { p.ApplyDirective(m, gen.IgnoreTypename(a)) - infof("ignoring %s\n", a) + log.Infof("ignoring %s\n", a) } - popstate() + log.PopState() return nil } @@ -77,7 +77,7 @@ func applyShim(text []string, f *FileSet) error { be.ShimToBase = methods[0] be.ShimFromBase = methods[1] - infof("%s -> %s\n", name, be.Value.String()) + log.Infof("%s -> %s\n", name, be.Value.String()) f.findShim(name, be) return nil @@ -92,7 +92,7 @@ func ignore(text []string, f *FileSet) error { name := strings.TrimSpace(item) if _, ok := f.Identities[name]; ok { delete(f.Identities, name) - infof("ignoring %s\n", name) + log.Infof("ignoring %s\n", name) } } return nil @@ -108,9 +108,9 @@ func astuple(text []string, f *FileSet) error { if el, ok := f.Identities[name]; ok { if st, ok := el.(*gen.Struct); ok { st.AsTuple = true - infoln(name) + log.Infoln(name) } else { - warnf("%s: only structs can be tuples\n", name) + log.Warnf("%s: only structs can be tuples\n", name) } } } diff --git a/parse/getast.go b/parse/getast.go index cec1ebe2..dabc068d 100644 --- a/parse/getast.go +++ b/parse/getast.go @@ -12,7 +12,7 @@ import ( "strings" "github.com/tinylib/msgp/gen" - "github.com/ttacon/chalk" + "github.com/tinylib/msgp/internal/log" ) // A FileSet is the in-memory representation of a @@ -32,8 +32,8 @@ type FileSet struct { // If unexport is false, only exported identifiers are included in the FileSet. // If the resulting FileSet would be empty, an error is returned. func File(name string, unexported bool) (*FileSet, error) { - pushstate(name) - defer popstate() + log.PushState(name) + defer log.PopState() fs := &FileSet{ Specs: make(map[string]ast.Expr), Identities: make(map[string]gen.Elem), @@ -59,13 +59,13 @@ func File(name string, unexported bool) (*FileSet, error) { } fs.Package = one.Name for _, fl := range one.Files { - pushstate(fl.Name.Name) + log.PushState(fl.Name.Name) fs.Directives = append(fs.Directives, yieldComments(fl.Comments)...) if !unexported { ast.FileExports(fl) } fs.getTypeSpecs(fl) - popstate() + log.PopState() } } else { f, err := parser.ParseFile(fset, name, nil, parser.ParseComments) @@ -100,12 +100,12 @@ func (f *FileSet) applyDirectives() { chunks := strings.Split(d, " ") if len(chunks) > 0 { if fn, ok := directives[chunks[0]]; ok { - pushstate(chunks[0]) + log.PushState(chunks[0]) err := fn(chunks, f) if err != nil { - warnln(err.Error()) + log.Warnln(err.Error()) } - popstate() + log.PopState() } else { newdirs = append(newdirs, d) } @@ -159,7 +159,7 @@ func (f *FileSet) resolve(ls linkset) { // what's left can't be resolved for name, elem := range ls { - warnf("couldn't resolve type %s (%s)\n", name, elem.TypeName()) + log.Warnf("couldn't resolve type %s (%s)\n", name, elem.TypeName()) } } @@ -170,11 +170,11 @@ func (f *FileSet) process() { deferred := make(linkset) parse: for name, def := range f.Specs { - pushstate(name) + log.PushState(name) el := f.parseExpr(def) if el == nil { - warnln("failed to parse") - popstate() + log.Warnln("failed to parse") + log.PopState() continue parse } // push unresolved identities into @@ -182,12 +182,12 @@ parse: // we've handled every possible named type. if be, ok := el.(*gen.BaseElem); ok && be.Value == gen.IDENT { deferred[name] = be - popstate() + log.PopState() continue parse } el.Alias(name) f.Identities[name] = el - popstate() + log.PopState() } if len(deferred) > 0 { @@ -228,21 +228,21 @@ loop: } m := strToMethod(chunks[0]) if m == 0 { - warnf("unknown pass name: %q\n", chunks[0]) + log.Warnf("unknown pass name: %q\n", chunks[0]) continue loop } if fn, ok := passDirectives[chunks[1]]; ok { - pushstate(chunks[1]) + log.PushState(chunks[1]) err := fn(m, chunks[2:], p) if err != nil { - warnf("error applying directive: %s\n", err) + log.Warnf("error applying directive: %s\n", err) } - popstate() + log.PopState() } else { - warnf("unrecognized directive %q\n", chunks[1]) + log.Warnf("unrecognized directive %q\n", chunks[1]) } } else { - warnf("empty directive: %q\n", d) + log.Warnf("empty directive: %q\n", d) } } } @@ -257,9 +257,9 @@ func (f *FileSet) PrintTo(p *gen.Printer) error { for _, name := range names { el := f.Identities[name] el.SetVarname("z") - pushstate(el.TypeName()) + log.PushState(el.TypeName()) err := p.Print(el) - popstate() + log.PopState() if err != nil { return err } @@ -320,14 +320,14 @@ func (fs *FileSet) parseFieldList(fl *ast.FieldList) []gen.StructField { } out := make([]gen.StructField, 0, fl.NumFields()) for _, field := range fl.List { - pushstate(fieldName(field)) + log.PushState(fieldName(field)) fds := fs.getField(field) if len(fds) > 0 { out = append(out, fds...) } else { - warnln("ignored") + log.Warnln("ignored") } - popstate() + log.PopState() } return out } @@ -362,7 +362,7 @@ func (fs *FileSet) getField(f *ast.Field) []gen.StructField { sf[0].FieldTag, err = strconv.ParseInt(tags[0], 0, 64) } if err != nil { - warnf("could not parse field label %q as msgp.%s: %s\n", tags[0], tags[1], err) + log.Warnf("could not parse field label %q as msgp.%s: %s\n", tags[0], tags[1], err) return nil } } @@ -404,13 +404,13 @@ func (fs *FileSet) getField(f *ast.Field) []gen.StructField { if b, ok := ex.Value.(*gen.BaseElem); ok { b.Value = gen.Ext } else { - warnln("couldn't cast to extension.") + log.Warnln("couldn't cast to extension.") return nil } case *gen.BaseElem: ex.Value = gen.Ext default: - warnln("couldn't cast to extension.") + log.Warnln("couldn't cast to extension.") return nil } } @@ -490,7 +490,7 @@ func (fs *FileSet) parseExpr(e ast.Expr) gen.Elem { // everything else. if b.Value == gen.IDENT { if _, ok := fs.Specs[e.Name]; !ok { - warnf("non-local identifier: %s\n", e.Name) + log.Warnf("non-local identifier: %s\n", e.Name) } } return b @@ -564,45 +564,3 @@ func (fs *FileSet) parseExpr(e ast.Expr) gen.Elem { return nil } } - -func infof(s string, v ...interface{}) { - pushstate(s) - fmt.Printf(chalk.Green.Color(strings.Join(logctx, ": ")), v...) - popstate() -} - -func infoln(s string) { - pushstate(s) - fmt.Println(chalk.Green.Color(strings.Join(logctx, ": "))) - popstate() -} - -func warnf(s string, v ...interface{}) { - pushstate(s) - fmt.Printf(chalk.Yellow.Color(strings.Join(logctx, ": ")), v...) - popstate() -} - -func warnln(s string) { - pushstate(s) - fmt.Println(chalk.Yellow.Color(strings.Join(logctx, ": "))) - popstate() -} - -func fatalf(s string, v ...interface{}) { - pushstate(s) - fmt.Printf(chalk.Red.Color(strings.Join(logctx, ": ")), v...) - popstate() -} - -var logctx []string - -// push logging state -func pushstate(s string) { - logctx = append(logctx, s) -} - -// pop logging state -func popstate() { - logctx = logctx[:len(logctx)-1] -} diff --git a/parse/inline.go b/parse/inline.go index 85d60c92..3a03f274 100644 --- a/parse/inline.go +++ b/parse/inline.go @@ -2,6 +2,7 @@ package parse import ( "github.com/tinylib/msgp/gen" + "github.com/tinylib/msgp/internal/log" ) // This file defines when and how we @@ -32,7 +33,7 @@ const maxComplex = 5 // given name and replace them with be func (f *FileSet) findShim(id string, be *gen.BaseElem) { for name, el := range f.Identities { - pushstate(name) + log.PushState(name) switch el := el.(type) { case *gen.Struct: for i := range el.Fields { @@ -47,7 +48,7 @@ func (f *FileSet) findShim(id string, be *gen.BaseElem) { case *gen.Ptr: f.nextShim(&el.Value, id, be) } - popstate() + log.PopState() } // we'll need this at the top level as well f.Identities[id] = be @@ -77,7 +78,7 @@ func (f *FileSet) nextShim(ref *gen.Elem, id string, be *gen.BaseElem) { // propInline identifies and inlines candidates func (f *FileSet) propInline() { for name, el := range f.Identities { - pushstate(name) + log.PushState(name) switch el := el.(type) { case *gen.Struct: for i := range el.Fields { @@ -92,7 +93,7 @@ func (f *FileSet) propInline() { case *gen.Ptr: f.nextInline(&el.Value, name) } - popstate() + log.PopState() } } @@ -109,7 +110,7 @@ func (f *FileSet) nextInline(ref *gen.Elem, root string) { typ := el.TypeName() if el.Value == gen.IDENT && typ != root { if node, ok := f.Identities[typ]; ok && node.Complexity() < maxComplex { - infof("inlining %s\n", typ) + log.Infof("inlining %s\n", typ) // This should never happen; it will cause // infinite recursion. @@ -125,7 +126,7 @@ func (f *FileSet) nextInline(ref *gen.Elem, root string) { // this is the point at which we're sure that // we've got a type that isn't a primitive, // a library builtin, or a processed type - warnf("unresolved identifier: %s\n", typ) + log.Warnf("unresolved identifier: %s\n", typ) } } case *gen.Struct: diff --git a/printer/print.go b/printer/print.go index d3ef52d0..55bac975 100644 --- a/printer/print.go +++ b/printer/print.go @@ -9,14 +9,10 @@ import ( "github.com/tinylib/msgp/gen" "github.com/tinylib/msgp/parse" - "github.com/ttacon/chalk" + "github.com/tinylib/msgp/internal/log" "golang.org/x/tools/imports" ) -func infof(s string, v ...interface{}) { - fmt.Printf(chalk.Magenta.Color(s), v...) -} - // PrintFile prints the methods for the provided list // of elements to the given file name and canonical // package path. @@ -39,7 +35,7 @@ func PrintFile(file string, f *parse.FileSet, mode gen.Method) error { if err != nil { return err } - infof(">>> Wrote and formatted \"%s\"\n", testfile) + log.Infof(">>> Wrote and formatted \"%s\"\n", testfile) } err = <-res if err != nil { @@ -60,7 +56,7 @@ func goformat(file string, data []byte) <-chan error { out := make(chan error, 1) go func(file string, data []byte, end chan error) { end <- format(file, data) - infof(">>> Wrote and formatted \"%s\"\n", file) + log.Infof(">>> Wrote and formatted \"%s\"\n", file) }(file, data, out) return out } From efbe90cbe9daf2d58a17074568c6e2cad6295297 Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Tue, 2 Aug 2016 16:59:36 +0300 Subject: [PATCH 22/32] fix import --- parse/directives.go | 1 + 1 file changed, 1 insertion(+) diff --git a/parse/directives.go b/parse/directives.go index a5b010c6..dcd4ecd6 100644 --- a/parse/directives.go +++ b/parse/directives.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/tinylib/msgp/gen" + "github.com/tinylib/msgp/internal/log" ) const linePrefix = "//msgp:" From f1ccc2ad74786af60d15b6ba2b66f00953eb9176 Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Wed, 14 Sep 2016 17:56:11 +0300 Subject: [PATCH 23/32] ignore idea files --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 17f1ccdc..09a3bfd9 100644 --- a/.gitignore +++ b/.gitignore @@ -4,4 +4,5 @@ _generated/*_gen.go _generated/*_gen_test.go msgp/defgen_test.go msgp/cover.out -*~ \ No newline at end of file +*~ +.idea \ No newline at end of file From de20c156f16482eb1d150c1dccd477ae4943bfed Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Wed, 14 Sep 2016 17:59:03 +0300 Subject: [PATCH 24/32] map keys are not just strings now --- _generated/def.go | 7 ++++--- gen/decode.go | 6 +++--- gen/elem.go | 4 +++- gen/encode.go | 2 +- gen/marshal.go | 2 +- gen/size.go | 5 +++-- gen/unmarshal.go | 6 ++++-- parse/getast.go | 11 +++++++---- 8 files changed, 26 insertions(+), 17 deletions(-) diff --git a/_generated/def.go b/_generated/def.go index 10bb2d6e..23046459 100644 --- a/_generated/def.go +++ b/_generated/def.go @@ -40,6 +40,8 @@ type Fixed struct { type TestType struct { F *float64 `msg:"float"` Els map[string]string `msg:"elements"` + Els2 map[int]int `msg:"elements_int"` + Els3 map[uint]uint `msg:"elements_uint"` Obj struct { // test anonymous struct ValueA string `msg:"value_a"` ValueB []byte `msg:"value_b"` @@ -66,9 +68,8 @@ type TestOnlyIntLabels struct { type TestIntLiterals struct { A string `msg:"0x01,int"` - B string `msg:"0b10,int"` - C string `msg:"03,int"` - D string `msg:"4,int"` + B string `msg:"03,int"` + C string `msg:"4,int"` } type SingleFieldNumeric struct { diff --git a/gen/decode.go b/gen/decode.go index 3e1a73a5..fa389f3b 100644 --- a/gen/decode.go +++ b/gen/decode.go @@ -156,10 +156,10 @@ func (d *decodeGen) gMap(m *Map) { // for element in map, read string/value // pair and assign - d.p.printf("\nfor %s > 0 {\n%s--", sz, sz) - d.p.declare(m.Keyidx, "string") + d.p.declare(m.Keyidx, m.Key.TypeName()) d.p.declare(m.Validx, m.Value.TypeName()) - d.assignAndCheck(m.Keyidx, stringTyp) + d.p.printf("\nfor %s > 0 {\n%s--", sz, sz) + next(d, m.Key) next(d, m.Value) d.p.mapAssign(m) d.p.closeblock() diff --git a/gen/elem.go b/gen/elem.go index 8f639e3e..6d088b6a 100644 --- a/gen/elem.go +++ b/gen/elem.go @@ -236,6 +236,7 @@ type Map struct { common Keyidx string // key variable name Validx string // value variable name + Key Elem // key element Value Elem // value element } @@ -250,6 +251,7 @@ ridx: goto ridx } + m.Key.SetVarname(m.Keyidx) m.Value.SetVarname(m.Validx) } @@ -257,7 +259,7 @@ func (m *Map) TypeName() string { if m.common.alias != "" { return m.common.alias } - m.common.Alias("map[string]" + m.Value.TypeName()) + m.common.Alias("map[" + m.Key.TypeName() + "]" + m.Value.TypeName()) return m.common.alias } diff --git a/gen/encode.go b/gen/encode.go index 596a2378..90d9a657 100644 --- a/gen/encode.go +++ b/gen/encode.go @@ -114,7 +114,7 @@ func (e *encodeGen) gMap(m *Map) { e.writeAndCheck(mapHeader, lenAsUint32, vname) e.p.printf("\nfor %s, %s := range %s {", m.Keyidx, m.Validx, vname) - e.writeAndCheck(stringTyp, literalFmt, m.Keyidx) + next(e, m.Key) next(e, m.Value) e.p.closeblock() } diff --git a/gen/marshal.go b/gen/marshal.go index c7ae18f3..522cacc9 100644 --- a/gen/marshal.go +++ b/gen/marshal.go @@ -117,7 +117,7 @@ func (m *marshalGen) gMap(s *Map) { vname := s.Varname() m.rawAppend(mapHeader, lenAsUint32, vname) m.p.printf("\nfor %s, %s := range %s {", s.Keyidx, s.Validx, vname) - m.rawAppend(stringTyp, literalFmt, s.Keyidx) + next(m, s.Key) next(m, s.Value) m.p.closeblock() } diff --git a/gen/size.go b/gen/size.go index c8dbae44..09cf9ed0 100644 --- a/gen/size.go +++ b/gen/size.go @@ -169,9 +169,10 @@ func (s *sizeGen) gMap(m *Map) { vn := m.Varname() s.p.printf("\nif %s != nil {", vn) s.p.printf("\nfor %s, %s := range %s {", m.Keyidx, m.Validx, vn) + s.p.printf("\n_ = %s", m.Keyidx) // we may not use the key s.p.printf("\n_ = %s", m.Validx) // we may not use the value - s.p.printf("\ns += msgp.StringPrefixSize + len(%s)", m.Keyidx) - s.state = expr + s.state = add + next(s, m.Key) next(s, m.Value) s.p.closeblock() s.p.closeblock() diff --git a/gen/unmarshal.go b/gen/unmarshal.go index ddf8c764..fed93ebe 100644 --- a/gen/unmarshal.go +++ b/gen/unmarshal.go @@ -169,9 +169,11 @@ func (u *unmarshalGen) gMap(m *Map) { u.p.resizeMap(sz, m) // loop and get key,value + u.p.declare(m.Keyidx, m.Key.TypeName()) + u.p.declare(m.Validx, m.Value.TypeName()) u.p.printf("\nfor %s > 0 {", sz) - u.p.printf("\nvar %s string; var %s %s; %s--", m.Keyidx, m.Validx, m.Value.TypeName(), sz) - u.assignAndCheck(m.Keyidx, stringTyp) + u.p.printf("\n%s--", sz) + next(u, m.Key) next(u, m.Value) u.p.mapAssign(m) u.p.closeblock() diff --git a/parse/getast.go b/parse/getast.go index dabc068d..60682708 100644 --- a/parse/getast.go +++ b/parse/getast.go @@ -475,9 +475,12 @@ func (fs *FileSet) parseExpr(e ast.Expr) gen.Elem { switch e := e.(type) { case *ast.MapType: - if k, ok := e.Key.(*ast.Ident); ok && k.Name == "string" { - if in := fs.parseExpr(e.Value); in != nil { - return &gen.Map{Value: in} + key := fs.parseExpr(e.Key) + val := fs.parseExpr(e.Value) + if key != nil && val != nil { + return &gen.Map{ + Key: key, + Value: val, } } return nil @@ -485,7 +488,7 @@ func (fs *FileSet) parseExpr(e ast.Expr) gen.Elem { case *ast.Ident: b := gen.Ident(e.Name) - // work to resove this expression + // work to resolve this expression // can be done later, once we've resolved // everything else. if b.Value == gen.IDENT { From dff3b8561bd4231b66c1198f96762d892d6ba70b Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Wed, 14 Sep 2016 17:59:42 +0300 Subject: [PATCH 25/32] apply gofmt to sources --- gen/elem.go | 2 +- gen/spec.go | 2 +- internal/log/log.go | 2 +- parse/getast.go | 2 +- printer/print.go | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gen/elem.go b/gen/elem.go index 6d088b6a..f9727483 100644 --- a/gen/elem.go +++ b/gen/elem.go @@ -236,7 +236,7 @@ type Map struct { common Keyidx string // key variable name Validx string // value variable name - Key Elem // key element + Key Elem // key element Value Elem // value element } diff --git a/gen/spec.go b/gen/spec.go index 8e0b4cf8..18069ccb 100644 --- a/gen/spec.go +++ b/gen/spec.go @@ -5,8 +5,8 @@ import ( "io" "math" - "github.com/tinylib/msgp/msgp" "github.com/tinylib/msgp/internal/log" + "github.com/tinylib/msgp/msgp" ) const ( diff --git a/internal/log/log.go b/internal/log/log.go index 7111f3a3..26b8a6cf 100644 --- a/internal/log/log.go +++ b/internal/log/log.go @@ -3,8 +3,8 @@ package log import ( "fmt" "github.com/ttacon/chalk" - "strings" "os" + "strings" ) var logctx []string diff --git a/parse/getast.go b/parse/getast.go index 60682708..dbd9639b 100644 --- a/parse/getast.go +++ b/parse/getast.go @@ -479,7 +479,7 @@ func (fs *FileSet) parseExpr(e ast.Expr) gen.Elem { val := fs.parseExpr(e.Value) if key != nil && val != nil { return &gen.Map{ - Key: key, + Key: key, Value: val, } } diff --git a/printer/print.go b/printer/print.go index 55bac975..501c6f6b 100644 --- a/printer/print.go +++ b/printer/print.go @@ -8,8 +8,8 @@ import ( "strings" "github.com/tinylib/msgp/gen" - "github.com/tinylib/msgp/parse" "github.com/tinylib/msgp/internal/log" + "github.com/tinylib/msgp/parse" "golang.org/x/tools/imports" ) From ff02952bef327c6823bd6dcdb3773baa242875b3 Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Wed, 14 Sep 2016 18:07:34 +0300 Subject: [PATCH 26/32] eol --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 09a3bfd9..fe7f1f38 100644 --- a/.gitignore +++ b/.gitignore @@ -5,4 +5,4 @@ _generated/*_gen_test.go msgp/defgen_test.go msgp/cover.out *~ -.idea \ No newline at end of file +.idea From af68dbd14d6142d547162ce5916a5e7ad768319e Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Thu, 15 Sep 2016 12:04:15 +0300 Subject: [PATCH 27/32] fix possible shadowing bytes value in a map --- _generated/def.go | 5 +++-- gen/decode.go | 2 +- gen/unmarshal.go | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/_generated/def.go b/_generated/def.go index 23046459..516317eb 100644 --- a/_generated/def.go +++ b/_generated/def.go @@ -40,8 +40,9 @@ type Fixed struct { type TestType struct { F *float64 `msg:"float"` Els map[string]string `msg:"elements"` - Els2 map[int]int `msg:"elements_int"` - Els3 map[uint]uint `msg:"elements_uint"` + Els2 map[int]int `msg:"elements_2"` + Els3 map[uint]uint `msg:"elements_3"` + Els4 map[uint][]byte `msg:"elements_4"` Obj struct { // test anonymous struct ValueA string `msg:"value_a"` ValueB []byte `msg:"value_b"` diff --git a/gen/decode.go b/gen/decode.go index fa389f3b..55426342 100644 --- a/gen/decode.go +++ b/gen/decode.go @@ -156,9 +156,9 @@ func (d *decodeGen) gMap(m *Map) { // for element in map, read string/value // pair and assign + d.p.printf("\nfor %s > 0 {\n%s--", sz, sz) d.p.declare(m.Keyidx, m.Key.TypeName()) d.p.declare(m.Validx, m.Value.TypeName()) - d.p.printf("\nfor %s > 0 {\n%s--", sz, sz) next(d, m.Key) next(d, m.Value) d.p.mapAssign(m) diff --git a/gen/unmarshal.go b/gen/unmarshal.go index fed93ebe..01d28445 100644 --- a/gen/unmarshal.go +++ b/gen/unmarshal.go @@ -169,10 +169,10 @@ func (u *unmarshalGen) gMap(m *Map) { u.p.resizeMap(sz, m) // loop and get key,value - u.p.declare(m.Keyidx, m.Key.TypeName()) - u.p.declare(m.Validx, m.Value.TypeName()) u.p.printf("\nfor %s > 0 {", sz) u.p.printf("\n%s--", sz) + u.p.declare(m.Keyidx, m.Key.TypeName()) + u.p.declare(m.Validx, m.Value.TypeName()) next(u, m.Key) next(u, m.Value) u.p.mapAssign(m) From 61bf3eeab89dceb05572ca132933e4957a62a28b Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Tue, 18 Oct 2016 14:12:20 +0300 Subject: [PATCH 28/32] read map of interfaces --- msgp/read.go | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/msgp/read.go b/msgp/read.go index a493f941..a8ec447b 100644 --- a/msgp/read.go +++ b/msgp/read.go @@ -1087,6 +1087,33 @@ func (m *Reader) ReadMapStrIntf(mp map[string]interface{}) (err error) { return } +// ReadMapIntfIntf reads a MessagePack map into a map[interface{}]interface{}. +// (You must pass a non-nil map into the function.) +func (m *Reader) ReadMapIntfIntf(mp map[string]interface{}) (err error) { + var sz uint32 + sz, err = m.ReadMapHeader() + if err != nil { + return + } + for key := range mp { + delete(mp, key) + } + for i := uint32(0); i < sz; i++ { + var key interface{} + var val interface{} + key, err = m.ReadIntf() + if err != nil { + return + } + val, err = m.ReadIntf() + if err != nil { + return + } + mp[key] = val + } + return +} + // ReadTime reads a time.Time object from the reader. // The returned time's location will be set to time.Local. func (m *Reader) ReadTime() (t time.Time, err error) { @@ -1172,8 +1199,8 @@ func (m *Reader) ReadIntf() (i interface{}, err error) { return case MapType: - mp := make(map[string]interface{}) - err = m.ReadMapStrIntf(mp) + mp := make(map[interface{}]interface{}) + err = m.ReadMapIntfIntf(mp) i = mp return From 24d54bae5178bb7c1e66b22be74a485d676b531c Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Tue, 18 Oct 2016 14:12:32 +0300 Subject: [PATCH 29/32] remove redundant errcheck --- gen/unmarshal.go | 1 - 1 file changed, 1 deletion(-) diff --git a/gen/unmarshal.go b/gen/unmarshal.go index 01d28445..cbc68151 100644 --- a/gen/unmarshal.go +++ b/gen/unmarshal.go @@ -56,7 +56,6 @@ func (u *unmarshalGen) nextTypeAndCheck(name string) { return } u.p.printf("\n%s = msgp.NextType(bts)", name) - u.p.print(errcheck) } func (u *unmarshalGen) skipAndCheck() { From 9b93c50a7c7d35673fabbfb9657f75213fa3661c Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Tue, 18 Oct 2016 14:20:59 +0300 Subject: [PATCH 30/32] signature fix --- msgp/read.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/msgp/read.go b/msgp/read.go index a8ec447b..3e6e3caf 100644 --- a/msgp/read.go +++ b/msgp/read.go @@ -1089,7 +1089,7 @@ func (m *Reader) ReadMapStrIntf(mp map[string]interface{}) (err error) { // ReadMapIntfIntf reads a MessagePack map into a map[interface{}]interface{}. // (You must pass a non-nil map into the function.) -func (m *Reader) ReadMapIntfIntf(mp map[string]interface{}) (err error) { +func (m *Reader) ReadMapIntfIntf(mp map[interface{}]interface{}) (err error) { var sz uint32 sz, err = m.ReadMapHeader() if err != nil { From 66e724c30103d087e493abc2f5ae6171c854a4cc Mon Sep 17 00:00:00 2001 From: "s.kamardin" Date: Sat, 19 Nov 2016 14:29:06 +0300 Subject: [PATCH 31/32] map of interface to inteface --- msgp/read.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/msgp/read.go b/msgp/read.go index a493f941..8c3d6af1 100644 --- a/msgp/read.go +++ b/msgp/read.go @@ -1087,6 +1087,34 @@ func (m *Reader) ReadMapStrIntf(mp map[string]interface{}) (err error) { return } +// ReadMapIntfIntf reads a MessagePack map into a map[interface{}]interface{}. +// (You must pass a non-nil map into the function.) +func (m *Reader) ReadMapIntfIntf(mp map[interface{}]interface{}) (err error) { + var sz uint32 + sz, err = m.ReadMapHeader() + if err != nil { + return + } + for key := range mp { + delete(mp, key) + } + + for i := uint32(0); i < sz; i++ { + var key interface{} + var val interface{} + key, err = m.ReadIntf() + if err != nil { + return + } + val, err = m.ReadIntf() + if err != nil { + return + } + mp[key] = val + } + return +} + // ReadTime reads a time.Time object from the reader. // The returned time's location will be set to time.Local. func (m *Reader) ReadTime() (t time.Time, err error) { From 1f2d896bf9e180684804b4a6e13ac00aa02cb3aa Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Tue, 13 Dec 2016 19:44:48 +0000 Subject: [PATCH 32/32] Add (*Reader).CopyNext(w) (int64, error) (#167) * Add (*Reader).CopyNext(w) (int64, error) It is useful to be able to efficiently copy objects without decoding them. My use case is filtering when I already know the indices of the objects I want to keep, and for rewriting a dictionary of objects as a column of objects. * Remove ReadNext * Add opportunistic optimization with m.R.Next * Remove unused ReadNextError * Remove commented code * small fixup - only call (*Reader).Next() when we're sure it won't realloc its buffer - promote io.ErrUnexpectedEOF to msgp.ErrShortBytes --- msgp/read.go | 52 +++++++++++++++++++++++++++++++++++++++++++++++ msgp/read_test.go | 45 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/msgp/read.go b/msgp/read.go index 6760cd7a..b3aaa350 100644 --- a/msgp/read.go +++ b/msgp/read.go @@ -146,6 +146,56 @@ func (m *Reader) Read(p []byte) (int, error) { return m.R.Read(p) } +// CopyNext reads the next object from m without decoding it and writes it to w. +// It avoids unnecessary copies internally. +func (m *Reader) CopyNext(w io.Writer) (int64, error) { + sz, o, err := getNextSize(m.R) + if err != nil { + return 0, err + } + + var n int64 + // Opportunistic optimization: if we can fit the whole thing in the m.R + // buffer, then just get a pointer to that, and pass it to w.Write, + // avoiding an allocation. + if int(sz) <= m.R.BufferSize() { + var nn int + var buf []byte + buf, err = m.R.Next(int(sz)) + if err != nil { + if err == io.ErrUnexpectedEOF { + err = ErrShortBytes + } + return 0, err + } + nn, err = w.Write(buf) + n += int64(nn) + } else { + // Fall back to io.CopyN. + // May avoid allocating if w is a ReaderFrom (e.g. bytes.Buffer) + n, err = io.CopyN(w, m.R, int64(sz)) + if err == io.ErrUnexpectedEOF { + err = ErrShortBytes + } + } + if err != nil { + return n, err + } else if n < int64(sz) { + return n, io.ErrShortWrite + } + + // for maps and slices, read elements + for x := uintptr(0); x < o; x++ { + var n2 int64 + n2, err = m.CopyNext(w) + if err != nil { + return n, err + } + n += n2 + } + return n, nil +} + // ReadFull implements `io.ReadFull` func (m *Reader) ReadFull(p []byte) (int, error) { return m.R.ReadFull(p) @@ -194,8 +244,10 @@ func (m *Reader) IsNil() bool { return err == nil && p[0] == mnil } +// getNextSize returns the size of the next object on the wire. // returns (obj size, obj elements, error) // only maps and arrays have non-zero obj elements +// for maps and arrays, obj size does not include elements // // use uintptr b/c it's guaranteed to be large enough // to hold whatever we can fit in memory. diff --git a/msgp/read_test.go b/msgp/read_test.go index 12b4dcfd..fd6a9922 100644 --- a/msgp/read_test.go +++ b/msgp/read_test.go @@ -745,3 +745,48 @@ func BenchmarkSkip(b *testing.B) { } } } + +func TestCopyNext(t *testing.T) { + var buf bytes.Buffer + en := NewWriter(&buf) + + en.WriteMapHeader(6) + + en.WriteString("thing_one") + en.WriteString("value_one") + + en.WriteString("thing_two") + en.WriteFloat64(3.14159) + + en.WriteString("some_bytes") + en.WriteBytes([]byte("nkl4321rqw908vxzpojnlk2314rqew098-s09123rdscasd")) + + en.WriteString("the_time") + en.WriteTime(time.Now()) + + en.WriteString("what?") + en.WriteBool(true) + + en.WriteString("ext") + en.WriteExtension(&RawExtension{Type: 55, Data: []byte("raw data!!!")}) + + en.Flush() + + // Read from a copy of the original buf. + de := NewReader(bytes.NewReader(buf.Bytes())) + + w := new(bytes.Buffer) + + n, err := de.CopyNext(w) + if err != nil { + t.Fatal(err) + } + if n != int64(buf.Len()) { + t.Fatalf("CopyNext returned the wrong value (%d != %d)", + n, buf.Len()) + } + + if !bytes.Equal(buf.Bytes(), w.Bytes()) { + t.Fatalf("not equal! %v, %v", buf.Bytes(), w.Bytes()) + } +}