Skip to content

Commit

Permalink
cmd/protoc-gen-go: unexport implementation-specific XXX fields
Browse files Browse the repository at this point in the history
We modify protoc-gen-go to stop generating exported XXX fields.
The unsafe implementation is unaffected by this change since unsafe
can access fields regardless of visibility. However, for the purego
implementation, we need to respect Go visibility rules as enforced
by the reflect package.

We work around this by generating a exporter function that given
a reference to the message and the field to export, returns a reference
to the unexported field value. This exporter function is protected by
a constant such that it is not linked into the final binary in non-purego
build environment.

Updates #276

Change-Id: Idf5c1f158973fa1c61187ff41440acb21c5dac94
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185141
Reviewed-by: Damien Neil <[email protected]>
  • Loading branch information
dsnet committed Jul 8, 2019
1 parent 0991227 commit c0e4bb2
Show file tree
Hide file tree
Showing 82 changed files with 11,864 additions and 5,789 deletions.
32 changes: 26 additions & 6 deletions cmd/protoc-gen-go-grpc/testdata/grpc/grpc.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

109 changes: 83 additions & 26 deletions cmd/protoc-gen-go/internal_gengo/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,22 @@ const (
// generateOneofWrapperMethods specifies whether to generate
// XXX_OneofWrappers methods on messages with oneofs.
generateOneofWrapperMethods = false

// generateNoUnkeyedLiteralFields specifies whether to generate
// the XXX_NoUnkeyedLiteral field.
generateNoUnkeyedLiteralFields = false

// generateExportedSizeCacheFields specifies whether to generate an exported
// XXX_sizecache field instead of an unexported sizeCache field.
generateExportedSizeCacheFields = false

// generateExportedUnknownFields specifies whether to generate an exported
// XXX_unrecognized field instead of an unexported unknownFields field.
generateExportedUnknownFields = false

// generateExportedExtensionFields specifies whether to generate an exported
// XXX_InternalExtensions field instead of an unexported extensionFields field.
generateExportedExtensionFields = false
)

const (
Expand All @@ -54,11 +70,28 @@ const (
type fileInfo struct {
*protogen.File

allEnums []*protogen.Enum
allEnumsByPtr map[*protogen.Enum]int // value is index into allEnums
allMessages []*protogen.Message
allMessagesByPtr map[*protogen.Message]int // value is index into allMessages
allExtensions []*protogen.Extension
allEnums []*protogen.Enum
allMessages []*protogen.Message
allExtensions []*protogen.Extension

allEnumsByPtr map[*protogen.Enum]int // value is index into allEnums
allMessagesByPtr map[*protogen.Message]int // value is index into allMessages
allMessageFieldsByPtr map[*protogen.Message]*structFields
}

type structFields struct {
count int
unexported map[int]string
}

func (sf *structFields) append(name string) {
if r, _ := utf8.DecodeRuneInString(name); !unicode.IsUpper(r) {
if sf.unexported == nil {
sf.unexported = make(map[int]string)
}
sf.unexported[sf.count] = name
}
sf.count++
}

// GenerateFile generates the contents of a .pb.go file.
Expand Down Expand Up @@ -90,8 +123,10 @@ func GenerateFile(gen *protogen.Plugin, file *protogen.File) *protogen.Generated
}
if len(f.allMessages) > 0 {
f.allMessagesByPtr = make(map[*protogen.Message]int)
f.allMessageFieldsByPtr = make(map[*protogen.Message]*structFields)
for i, m := range f.allMessages {
f.allMessagesByPtr[m] = i
f.allMessageFieldsByPtr[m] = new(structFields)
}
}

Expand Down Expand Up @@ -347,15 +382,28 @@ func genMessage(gen *protogen.Plugin, g *protogen.GeneratedFile, f *fileInfo, me
}
g.Annotate(message.GoIdent.GoName, message.Location)
g.P("type ", message.GoIdent, " struct {")
sf := f.allMessageFieldsByPtr[message]
for _, field := range message.Fields {
if field.Oneof != nil {
// It would be a bit simpler to iterate over the oneofs below,
// but generating the field here keeps the contents of the Go
// struct in the same order as the contents of the source
// .proto file.
if field == field.Oneof.Fields[0] {
genOneofField(gen, g, f, message, field.Oneof)
oneof := field.Oneof
if field != oneof.Fields[0] {
continue // already generated oneof field for first entry
}
if g.PrintLeadingComments(oneof.Location) {
g.P("//")
}
g.P("// Types that are valid to be assigned to ", oneofFieldName(oneof), ":")
for _, field := range oneof.Fields {
g.PrintLeadingComments(field.Location)
g.P("//\t*", fieldOneofType(field))
}
g.Annotate(message.GoIdent.GoName+"."+oneofFieldName(oneof), oneof.Location)
g.P(oneofFieldName(oneof), " ", oneofInterfaceName(oneof), " `protobuf_oneof:\"", oneof.Desc.Name(), "\"`")
sf.append(oneofFieldName(oneof))
continue
}
g.PrintLeadingComments(field.Location)
Expand All @@ -378,19 +426,42 @@ func genMessage(gen *protogen.Plugin, g *protogen.GeneratedFile, f *fileInfo, me
g.Annotate(message.GoIdent.GoName+"."+field.GoName, field.Location)
g.P(field.GoName, " ", goType, " `", strings.Join(tags, " "), "`",
deprecationComment(field.Desc.Options().(*descriptorpb.FieldOptions).GetDeprecated()))
sf.append(field.GoName)
}
g.P("XXX_NoUnkeyedLiteral struct{} `json:\"-\"`")

if generateNoUnkeyedLiteralFields {
g.P("XXX_NoUnkeyedLiteral", " struct{} `json:\"-\"`")
sf.append("XXX_NoUnkeyedLiteral")
}
if generateExportedSizeCacheFields {
g.P("XXX_sizecache", " ", protoimplPackage.Ident("SizeCache"), " `json:\"-\"`")
sf.append("XXX_sizecache")
} else {
g.P("sizeCache", " ", protoimplPackage.Ident("SizeCache"))
sf.append("sizeCache")
}
if generateExportedUnknownFields {
g.P("XXX_unrecognized", " ", protoimplPackage.Ident("UnknownFields"), " `json:\"-\"`")
sf.append("XXX_unrecognized")
} else {
g.P("unknownFields", " ", protoimplPackage.Ident("UnknownFields"))
sf.append("unknownFields")
}
if message.Desc.ExtensionRanges().Len() > 0 {
// TODO: Remove this tag when we drop v1 support.
var tags []string
if message.Desc.Options().(*descriptorpb.MessageOptions).GetMessageSetWireFormat() {
tags = append(tags, `protobuf_messageset:"1"`)
}
tags = append(tags, `json:"-"`)
g.P("XXX_InternalExtensions ", protoimplPackage.Ident("ExtensionFields"), " `", strings.Join(tags, " "), "`")
if generateExportedExtensionFields {
tags = append(tags, `json:"-"`)
g.P("XXX_InternalExtensions", " ", protoimplPackage.Ident("ExtensionFields"), " `", strings.Join(tags, " "), "`")
sf.append("XXX_InternalExtensions")
} else {
g.P("extensionFields", " ", protoimplPackage.Ident("ExtensionFields"), " `", strings.Join(tags, " "), "`")
sf.append("extensionFields")
}
}
g.P("XXX_unrecognized ", protoimplPackage.Ident("UnknownFields"), " `json:\"-\"`")
g.P("XXX_sizecache ", protoimplPackage.Ident("SizeCache"), " `json:\"-\"`")
g.P("}")
g.P()

Expand Down Expand Up @@ -744,20 +815,6 @@ var wellKnownTypes = map[protoreflect.FullName]bool{
"google.protobuf.Value": true,
}

// genOneofField generates the struct field for a oneof.
func genOneofField(gen *protogen.Plugin, g *protogen.GeneratedFile, f *fileInfo, message *protogen.Message, oneof *protogen.Oneof) {
if g.PrintLeadingComments(oneof.Location) {
g.P("//")
}
g.P("// Types that are valid to be assigned to ", oneofFieldName(oneof), ":")
for _, field := range oneof.Fields {
g.PrintLeadingComments(field.Location)
g.P("//\t*", fieldOneofType(field))
}
g.Annotate(message.GoIdent.GoName+"."+oneofFieldName(oneof), oneof.Location)
g.P(oneofFieldName(oneof), " ", oneofInterfaceName(oneof), " `protobuf_oneof:\"", oneof.Desc.Name(), "\"`")
}

// genOneofGetter generate a Get method for a oneof.
func genOneofGetter(gen *protogen.Plugin, g *protogen.GeneratedFile, f *fileInfo, message *protogen.Message, oneof *protogen.Oneof) {
g.Annotate(message.GoIdent.GoName+".Get"+oneof.GoName, oneof.Location)
Expand Down
21 changes: 21 additions & 0 deletions cmd/protoc-gen-go/internal_gengo/reflect.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,27 @@ func genReflectFileDescriptor(gen *protogen.Plugin, g *protogen.GeneratedFile, f
}

if len(f.allMessages) > 0 {
// Populate MessageInfo.Exporters.
g.P("if !", protoimplPackage.Ident("UnsafeEnabled"), " {")
for _, message := range f.allMessages {
if sf := f.allMessageFieldsByPtr[message]; len(sf.unexported) > 0 {
idx := f.allMessagesByPtr[message]
typesVar := messageTypesVarName(f)

g.P(typesVar, "[", idx, "].Exporter = func(v interface{}, i int) interface{} {")
g.P("switch v := v.(*", message.GoIdent, "); i {")
for i := 0; i < sf.count; i++ {
if name := sf.unexported[i]; name != "" {
g.P("case ", i, ": return &v.", name)
}
}
g.P("default: return nil")
g.P("}")
g.P("}")
}
}
g.P("}")

// Populate MessageInfo.OneofWrappers.
for _, message := range f.allMessages {
if len(message.Oneofs) > 0 {
Expand Down
19 changes: 15 additions & 4 deletions cmd/protoc-gen-go/testdata/annotations/annotations.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -1 +1 @@
annotation:{path:5 path:0 source_file:"annotations/annotations.proto" begin:631 end:650} annotation:{path:5 path:0 path:2 path:0 source_file:"annotations/annotations.proto" begin:667 end:714} annotation:{path:4 path:0 source_file:"annotations/annotations.proto" begin:1821 end:1843} annotation:{path:4 path:0 path:2 path:0 source_file:"annotations/annotations.proto" begin:1854 end:1874} annotation:{path:4 path:0 path:2 path:0 source_file:"annotations/annotations.proto" begin:2895 end:2918}
annotation:{path:5 path:0 source_file:"annotations/annotations.proto" begin:631 end:650} annotation:{path:5 path:0 path:2 path:0 source_file:"annotations/annotations.proto" begin:667 end:714} annotation:{path:4 path:0 source_file:"annotations/annotations.proto" begin:1821 end:1843} annotation:{path:4 path:0 path:2 path:0 source_file:"annotations/annotations.proto" begin:1854 end:1874} annotation:{path:4 path:0 path:2 path:0 source_file:"annotations/annotations.proto" begin:2796 end:2819}
Loading

0 comments on commit c0e4bb2

Please sign in to comment.