Skip to content

Fix cases where the fix for issue260 was not enough #261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

Fix cases where the fix for issue260 was not enough #261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6065ff4
Fix lack of import in some scenarios with stdtime
Serabe Feb 15, 2017
c81e654
Test for issue261
Serabe Feb 15, 2017
c5cb856
Remove
Serabe Feb 16, 2017
b200b17
Refactor usages
Serabe Feb 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ regenerate:
make -C test/issue260 regenerate
make -C test/issue261 regenerate
make -C test/issue262 regenerate
make -C test/issue261 regenerate
make -C test/enumdecl regenerate
make -C test/typedecl_all regenerate
make -C test/enumdecl_all regenerate
Expand Down
30 changes: 16 additions & 14 deletions plugin/unmarshal/unmarshal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ type unmarshal struct {
mathPkg generator.Single
unsafePkg generator.Single
typesPkg generator.Single
timePkg generator.Single
localName string
}

Expand Down Expand Up @@ -362,10 +363,10 @@ func (p *unmarshal) mapField(varName string, customType bool, field *descriptor.
msgname := p.TypeName(desc)
buf := `dAtA[iNdEx:postmsgIndex]`
if gogoproto.IsStdTime(field) {
p.P(varName, ` := new(time.Time)`)
p.P(varName, ` := new(`, p.timePkg.Use(), `.Time)`)
p.P(`if err := `, p.typesPkg.Use(), `.StdTimeUnmarshal(`, varName, `, `, buf, `); err != nil {`)
} else if gogoproto.IsStdDuration(field) {
p.P(varName, ` := new(time.Duration)`)
p.P(varName, ` := new(`, p.timePkg.Use(), `.Duration)`)
p.P(`if err := `, p.typesPkg.Use(), `.StdDurationUnmarshal(`, varName, `, `, buf, `); err != nil {`)
} else {
p.P(varName, ` := &`, msgname, `{}`)
Expand Down Expand Up @@ -698,18 +699,18 @@ func (p *unmarshal) field(file *generator.FileDescriptor, msg *generator.Descrip
buf := `dAtA[iNdEx:postIndex]`
if gogoproto.IsStdTime(field) {
if nullable {
p.P(`v := new(time.Time)`)
p.P(`v := new(`, p.timePkg.Use(), `.Time)`)
p.P(`if err := `, p.typesPkg.Use(), `.StdTimeUnmarshal(v, `, buf, `); err != nil {`)
} else {
p.P(`v := time.Time{}`)
p.P(`v := `, p.timePkg.Use(), `.Time{}`)
p.P(`if err := `, p.typesPkg.Use(), `.StdTimeUnmarshal(&v, `, buf, `); err != nil {`)
}
} else if gogoproto.IsStdDuration(field) {
if nullable {
p.P(`v := new(time.Duration)`)
p.P(`v := new(`, p.timePkg.Use(), `.Duration)`)
p.P(`if err := `, p.typesPkg.Use(), `.StdDurationUnmarshal(v, `, buf, `); err != nil {`)
} else {
p.P(`v := time.Duration(0)`)
p.P(`v := `, p.timePkg.Use(), `.Duration(0)`)
p.P(`if err := `, p.typesPkg.Use(), `.StdDurationUnmarshal(&v, `, buf, `); err != nil {`)
}
} else {
Expand Down Expand Up @@ -778,14 +779,14 @@ func (p *unmarshal) field(file *generator.FileDescriptor, msg *generator.Descrip
p.P(`} else {`)
p.In()
if gogoproto.IsStdTime(field) {
p.P(`var mapvalue = new(time.Time)`)
p.P(`var mapvalue = new(`, p.timePkg.Use(), `.Time)`)
if nullable {
p.P(s, ` = mapvalue`)
} else {
p.P(s, ` = *mapvalue`)
}
} else if gogoproto.IsStdDuration(field) {
p.P(`var mapvalue = new(time.Duration)`)
p.P(`var mapvalue = new(`, p.timePkg.Use(), `.Duration)`)
if nullable {
p.P(s, ` = mapvalue`)
} else {
Expand All @@ -800,15 +801,15 @@ func (p *unmarshal) field(file *generator.FileDescriptor, msg *generator.Descrip
} else if repeated {
if gogoproto.IsStdTime(field) {
if nullable {
p.P(`m.`, fieldname, ` = append(m.`, fieldname, `, new(time.Time))`)
p.P(`m.`, fieldname, ` = append(m.`, fieldname, `, new(`, p.timePkg.Use(), `.Time))`)
} else {
p.P(`m.`, fieldname, ` = append(m.`, fieldname, `, time.Time{})`)
p.P(`m.`, fieldname, ` = append(m.`, fieldname, `, `, p.timePkg.Use(), `.Time{})`)
}
} else if gogoproto.IsStdDuration(field) {
if nullable {
p.P(`m.`, fieldname, ` = append(m.`, fieldname, `, new(time.Duration))`)
p.P(`m.`, fieldname, ` = append(m.`, fieldname, `, new(`, p.timePkg.Use(), `.Duration))`)
} else {
p.P(`m.`, fieldname, ` = append(m.`, fieldname, `, time.Duration(0))`)
p.P(`m.`, fieldname, ` = append(m.`, fieldname, `, `, p.timePkg.Use(), `.Duration(0))`)
}
} else if nullable && !gogoproto.IsCustomType(field) {
p.P(`m.`, fieldname, ` = append(m.`, fieldname, `, &`, msgname, `{})`)
Expand Down Expand Up @@ -843,9 +844,9 @@ func (p *unmarshal) field(file *generator.FileDescriptor, msg *generator.Descrip
p.P(`if m.`, fieldname, ` == nil {`)
p.In()
if gogoproto.IsStdTime(field) {
p.P(`m.`, fieldname, ` = new(time.Time)`)
p.P(`m.`, fieldname, ` = new(`, p.timePkg.Use(), `.Time)`)
} else if gogoproto.IsStdDuration(field) {
p.P(`m.`, fieldname, ` = new(time.Duration)`)
p.P(`m.`, fieldname, ` = new(`, p.timePkg.Use(), `.Duration)`)
} else {
goType, _ := p.GoType(nil, field)
// remove the star from the type
Expand Down Expand Up @@ -1098,6 +1099,7 @@ func (p *unmarshal) Generate(file *generator.FileDescriptor) {
p.mathPkg = p.NewImport("math")
p.unsafePkg = p.NewImport("unsafe")
p.typesPkg = p.NewImport("github.com/gogo/protobuf/types")
p.timePkg = p.NewImport("time")
fmtPkg := p.NewImport("fmt")
protoPkg := p.NewImport("github.com/gogo/protobuf/proto")
if !gogoproto.ImportsGoGoProto(file.FileDescriptorProto) {
Expand Down
18 changes: 8 additions & 10 deletions protoc-gen-gogo/generator/generator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1465,12 +1465,6 @@ func (g *Generator) generateImports() {
g.P("var _ = ", g.Pkg["proto"], ".Marshal")
g.P("var _ = ", g.Pkg["fmt"], ".Errorf")
g.P("var _ = ", g.Pkg["math"], ".Inf")
for _, cimport := range g.customImports {
if cimport == "time" {
g.P("var _ = time.Kitchen")
break
}
}
g.P()
}

Expand Down Expand Up @@ -1888,10 +1882,8 @@ func (g *Generator) GoType(message *Descriptor, field *descriptor.FieldDescripto
g.customImports = append(g.customImports, packageName)
}
case gogoproto.IsStdTime(field):
g.customImports = append(g.customImports, "time")
typ = "time.Time"
case gogoproto.IsStdDuration(field):
g.customImports = append(g.customImports, "time")
typ = "time.Duration"
}
if needsStar(field, g.file.proto3 && field.Extendee == nil, message != nil && message.allowOneof()) {
Expand Down Expand Up @@ -2452,6 +2444,10 @@ func (g *Generator) generateMessage(message *Descriptor) {
if gogoproto.IsEmbed(field) || gogoproto.IsCustomType(field) {
continue
}
if gogoproto.IsStdTime(field) || gogoproto.IsStdDuration(field) {
g.UseTime()
}

fname := fieldNames[field]
typename, _ := g.GoType(message, field)
if t, ok := mapFieldTypes[field]; ok {
Expand Down Expand Up @@ -2734,6 +2730,7 @@ func (g *Generator) generateMessage(message *Descriptor) {
} else if gogoproto.IsStdTime(field) {
pkg := g.useTypes()
if gogoproto.IsNullable(field) {
g.UseTime()
g.P(`dAtA, err := `, pkg, `.StdTimeMarshal(*`, val, `)`)
} else {
g.P(`dAtA, err := `, pkg, `.StdTimeMarshal(`, val, `)`)
Expand Down Expand Up @@ -2865,12 +2862,13 @@ func (g *Generator) generateMessage(message *Descriptor) {
val = "*c"
} else if gogoproto.IsStdTime(field) {
pkg := g.useTypes()
g.UseTime()
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to UseTime twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it was a problem with my rebase.

g.P(`if err != nil {`)
g.In()
g.P(`return true, err`)
g.Out()
g.P(`}`)
g.P(`c := new(time.Time)`)
g.P(`c := new(`, g.UseTime(), `.Time)`)
g.P(`if err2 := `, pkg, `.StdTimeUnmarshal(c, `, val, `); err2 != nil {`)
g.In()
g.P(`return true, err`)
Expand All @@ -2884,7 +2882,7 @@ func (g *Generator) generateMessage(message *Descriptor) {
g.P(`return true, err`)
g.Out()
g.P(`}`)
g.P(`c := new(time.Duration)`)
g.P(`c := new(`, g.UseTime(), `.Duration)`)
g.P(`if err2 := `, pkg, `.StdDurationUnmarshal(c, `, val, `); err2 != nil {`)
g.In()
g.P(`return true, err`)
Expand Down
6 changes: 6 additions & 0 deletions protoc-gen-gogo/generator/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,3 +445,9 @@ func (g *Generator) useTypes() string {
g.customImports = append(g.customImports, "github.com/gogo/protobuf/types")
return pkg
}

func (g *Generator) UseTime() string {
pkg := "time"
g.customImports = append(g.customImports, "time")
return pkg
}
Loading