Skip to content

Commit

Permalink
all: remove weak field support
Browse files Browse the repository at this point in the history 
Weak fields were the predecessor to extensions (many many years ago) and were
entirely removed from Google’s production usage by now. (The corresponding field
in descriptor.proto was always documented as “// For Google-internal migration
only. Do not use.”)

Before this change, Go Protobuf still contained support for weak fields behind
the `protolegacy` build tag.

The `protolegacy` build tag was always documented as not being part of the
compatibility agreement:

// WARNING: The compatibility agreement covers nothing provided by this flag.
// As such, functionality may suddenly be removed or changed at our discretion.
const ProtoLegacy = protoLegacy

Fixes golang/protobuf#1666

Change-Id: Ie1675424bc80d9f44345ccb96a858ef847ee1018
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/641655
Reviewed-by: Chressie Himpel <chressie@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
@stapelberg
stapelberg committed Jan 27, 2025
1 parent 89b5638 commit e677ef9
Show file tree
Hide file tree
Showing 53 changed files with 806 additions and 2,050 deletions.
16 changes: 0 additions & 16 deletions cmd/protoc-gen-go/annotation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,27 +56,11 @@ func TestAnnotations(t *testing.T) {
&descriptorpb.GeneratedCodeInfo_Annotation{
Path: []int32{int32(genid.FileDescriptorProto_MessageType_field_number), 0, int32(genid.DescriptorProto_Field_field_number), 0},
},
}, {
"\t", "XXX_weak_M", " ",
&descriptorpb.GeneratedCodeInfo_Annotation{
Path: []int32{int32(genid.FileDescriptorProto_MessageType_field_number), 0, int32(genid.DescriptorProto_Field_field_number), 1},
},
}, {
"func (x *AnnotationsTestMessage) ", "GetAnnotationsTestField", "() string {",
&descriptorpb.GeneratedCodeInfo_Annotation{
Path: []int32{int32(genid.FileDescriptorProto_MessageType_field_number), 0, int32(genid.DescriptorProto_Field_field_number), 0},
},
}, {
"func (x *AnnotationsTestMessage) ", "GetM", "() proto.Message {",
&descriptorpb.GeneratedCodeInfo_Annotation{
Path: []int32{int32(genid.FileDescriptorProto_MessageType_field_number), 0, int32(genid.DescriptorProto_Field_field_number), 1},
},
}, {
"func (x *AnnotationsTestMessage) ", "SetM", "(v proto.Message) {",
&descriptorpb.GeneratedCodeInfo_Annotation{
Path: []int32{int32(genid.FileDescriptorProto_MessageType_field_number), 0, int32(genid.DescriptorProto_Field_field_number), 1},
Semantic: descriptorpb.GeneratedCodeInfo_Annotation_SET.Enum(),
},
}} {
s := want.prefix + want.text + want.suffix
pos := bytes.Index(sourceFile, []byte(s))
Expand Down
4 changes: 0 additions & 4 deletions cmd/protoc-gen-go/internal_gengo/init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,13 @@ type messageInfo struct {

isTracked bool
noInterface bool
hasWeak bool
}

func newMessageInfo(f *fileInfo, message *protogen.Message) *messageInfo {
m := &messageInfo{Message: message}
m.genRawDescMethod = true
m.genExtRangeMethod = true
m.isTracked = isTrackedMessage(m)
for _, field := range m.Fields {
m.hasWeak = m.hasWeak || field.Desc.IsWeak()
}
opaqueNewMessageInfoHook(f, m)
return m
}
Expand Down
58 changes: 2 additions & 56 deletions cmd/protoc-gen-go/internal_gengo/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,12 +215,10 @@ func genImport(gen *protogen.Plugin, g *protogen.GeneratedFile, f *fileInfo, imp
// Don't generate imports or aliases for types in the same Go package.
return
}
// Generate imports for all non-weak dependencies, even if they are not
// Generate imports for all dependencies, even if they are not
// referenced, because other code and tools depend on having the
// full transitive closure of protocol buffer types in the binary.
if !imp.IsWeak {
g.Import(impFile.GoImportPath)
}
g.Import(impFile.GoImportPath)
if !imp.IsPublic {
return
}
Expand Down Expand Up @@ -440,10 +438,6 @@ func genMessageInternalFields(g *protogen.GeneratedFile, f *fileInfo, m *message
sf.append(genid.State_goname)
g.P(genid.SizeCache_goname, " ", protoimplPackage.Ident("SizeCache"))
sf.append(genid.SizeCache_goname)
if m.hasWeak {
g.P(genid.WeakFields_goname, " ", protoimplPackage.Ident("WeakFields"))
sf.append(genid.WeakFields_goname)
}
g.P(genid.UnknownFields_goname, " ", protoimplPackage.Ident("UnknownFields"))
sf.append(genid.UnknownFields_goname)
if m.Desc.ExtensionRanges().Len() > 0 {
Expand Down Expand Up @@ -508,9 +502,6 @@ func genMessageField(g *protogen.GeneratedFile, f *fileInfo, m *messageInfo, fie
}

name := field.GoName
if field.Desc.IsWeak() {
name = genid.WeakFieldPrefix_goname + name
}
g.AnnotateSymbol(m.GoIdent.GoName+"."+name, protogen.Annotation{Location: field.Location})
leadingComments := appendDeprecationSuffix(field.Comments.Leading,
field.Desc.ParentFile(),
Expand Down Expand Up @@ -590,7 +581,6 @@ func genMessageDefaultDecls(g *protogen.GeneratedFile, f *fileInfo, m *messageIn
func genMessageMethods(g *protogen.GeneratedFile, f *fileInfo, m *messageInfo) {
genMessageBaseMethods(g, f, m)
genMessageGetterMethods(g, f, m)
genMessageSetterMethods(g, f, m)
}

func genMessageBaseMethods(g *protogen.GeneratedFile, f *fileInfo, m *messageInfo) {
Expand Down Expand Up @@ -655,17 +645,6 @@ func genMessageGetterMethods(g *protogen.GeneratedFile, f *fileInfo, m *messageI
field.Desc.ParentFile(),
field.Desc.Options().(*descriptorpb.FieldOptions).GetDeprecated())
switch {
case field.Desc.IsWeak():
g.P(leadingComments, "func (x *", m.GoIdent, ") Get", field.GoName, "() ", protoPackage.Ident("Message"), "{")
g.P("var w ", protoimplPackage.Ident("WeakFields"))
g.P("if x != nil {")
g.P("w = x.", genid.WeakFields_goname)
if m.isTracked {
g.P("_ = x.", genid.WeakFieldPrefix_goname+field.GoName)
}
g.P("}")
g.P("return ", protoimplPackage.Ident("X"), ".GetWeak(w, ", field.Desc.Number(), ", ", strconv.Quote(string(field.Message.Desc.FullName())), ")")
g.P("}")
case field.Oneof != nil && !field.Oneof.Desc.IsSynthetic():
g.P(leadingComments, "func (x *", m.GoIdent, ") Get", field.GoName, "() ", goType, " {")
g.P("if x, ok := x.Get", field.Oneof.GoName, "().(*", field.GoIdent, "); ok {")
Expand Down Expand Up @@ -693,43 +672,10 @@ func genMessageGetterMethods(g *protogen.GeneratedFile, f *fileInfo, m *messageI
}
}

func genMessageSetterMethods(g *protogen.GeneratedFile, f *fileInfo, m *messageInfo) {
for _, field := range m.Fields {
if !field.Desc.IsWeak() {
continue
}

genNoInterfacePragma(g, m.noInterface)

g.AnnotateSymbol(m.GoIdent.GoName+".Set"+field.GoName, protogen.Annotation{
Location: field.Location,
Semantic: descriptorpb.GeneratedCodeInfo_Annotation_SET.Enum(),
})
leadingComments := appendDeprecationSuffix("",
field.Desc.ParentFile(),
field.Desc.Options().(*descriptorpb.FieldOptions).GetDeprecated())
g.P(leadingComments, "func (x *", m.GoIdent, ") Set", field.GoName, "(v ", protoPackage.Ident("Message"), ") {")
g.P("var w *", protoimplPackage.Ident("WeakFields"))
g.P("if x != nil {")
g.P("w = &x.", genid.WeakFields_goname)
if m.isTracked {
g.P("_ = x.", genid.WeakFieldPrefix_goname+field.GoName)
}
g.P("}")
g.P(protoimplPackage.Ident("X"), ".SetWeak(w, ", field.Desc.Number(), ", ", strconv.Quote(string(field.Message.Desc.FullName())), ", v)")
g.P("}")
g.P()
}
}

// fieldGoType returns the Go type used for a field.
//
// If it returns pointer=true, the struct field is a pointer to the type.
func fieldGoType(g *protogen.GeneratedFile, f *fileInfo, field *protogen.Field) (goType string, pointer bool) {
if field.Desc.IsWeak() {
return "struct{}", false
}

pointer = field.Desc.HasPresence()
switch field.Desc.Kind() {
case protoreflect.BoolKind:
Expand Down
92 changes: 5 additions & 87 deletions cmd/protoc-gen-go/internal_gengo/opaque.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package internal_gengo

import (
"fmt"
"strconv"
"strings"
"unicode"
"unicode/utf8"
Expand Down Expand Up @@ -109,14 +108,11 @@ func opaqueGenMessageField(g *protogen.GeneratedFile, f *fileInfo, message *mess
}

name := field.GoName
if field.Desc.IsWeak() {
g.P("// Deprecated: Do not use. This will be deleted in the near future.")
name = genid.WeakFieldPrefix_goname + name
} else if message.isOpaque() {
if message.isOpaque() {
name = "xxx_hidden_" + name
}

if message.isOpaque() && !field.Desc.IsWeak() {
if message.isOpaque() {
g.P(name, " ", goType, tags)
sf.append(name)
if message.isTracked {
Expand Down Expand Up @@ -205,10 +201,6 @@ func opaqueGenMessageInternalFields(g *protogen.GeneratedFile, f *fileInfo, mess
g.P("XXX_presence [", (opaqueNumPresenceFields(message)+31)/32, "]uint32")
sf.append("XXX_presence")
}
if message.hasWeak {
g.P(genid.WeakFields_goname, " ", protoimplPackage.Ident("WeakFields"))
sf.append(genid.WeakFields_goname)
}
if message.Desc.ExtensionRanges().Len() > 0 {
g.P(genid.ExtensionFields_goname, " ", protoimplPackage.Ident("ExtensionFields"))
sf.append(genid.ExtensionFields_goname)
Expand All @@ -233,8 +225,8 @@ func opaqueGenMessageMethods(g *protogen.GeneratedFile, f *fileInfo, message *me
opaqueGenGet(g, f, message, field)
}
for _, field := range message.Fields {
// For the plain open mode, we only have set methods for weak fields.
if message.isOpen() && !field.Desc.IsWeak() {
// For the plain open mode, we do not have setters.
if message.isOpen() {
continue
}
opaqueGenSet(g, f, message, field)
Expand Down Expand Up @@ -313,22 +305,6 @@ func opaqueGenGet(g *protogen.GeneratedFile, f *fileInfo, message *messageInfo,
fieldtrackNoInterface(g, message.isTracked)
g.AnnotateSymbol(message.GoIdent.GoName+"."+getterName, protogen.Annotation{Location: field.Location})

// Weak field.
if field.Desc.IsWeak() {
g.P(leadingComments, "func (x *", message.GoIdent, ") ", getterName, "() ", protoPackage.Ident("Message"), "{")
g.P("var w ", protoimplPackage.Ident("WeakFields"))
g.P("if x != nil {")
g.P("w = x.", genid.WeakFields_goname)
if message.isTracked {
g.P("_ = x.", genid.WeakFieldPrefix_goname+field.GoName)
}
g.P("}")
g.P("return ", protoimplPackage.Ident("X"), ".GetWeak(w, ", field.Desc.Number(), ", ", strconv.Quote(string(field.Message.Desc.FullName())), ")")
g.P("}")
g.P()
return
}

defaultValue := fieldDefaultValue(g, f, message, field)

// Oneof field.
Expand Down Expand Up @@ -485,22 +461,6 @@ func opaqueGenSet(g *protogen.GeneratedFile, f *fileInfo, message *messageInfo,
})
fieldtrackNoInterface(g, message.noInterface)

// Weak field.
if field.Desc.IsWeak() {
g.P(leadingComments, "func (x *", message.GoIdent, ") ", setterName, "(v ", protoPackage.Ident("Message"), ") {")
g.P("var w *", protoimplPackage.Ident("WeakFields"))
g.P("if x != nil {")
g.P("w = &x.", genid.WeakFields_goname)
if message.isTracked {
g.P("_ = x.", genid.WeakFieldPrefix_goname+field.GoName)
}
g.P("}")
g.P(protoimplPackage.Ident("X"), ".SetWeak(w, ", field.Desc.Number(), ", ", strconv.Quote(string(field.Message.Desc.FullName())), ", v)")
g.P("}")
g.P()
return
}

// Oneof field.
if oneof := field.Oneof; oneof != nil && !oneof.Desc.IsSynthetic() {
g.P(leadingComments, "func (x *", message.GoIdent, ") ", setterName, "(v ", goType, ") {")
Expand Down Expand Up @@ -610,7 +570,7 @@ func opaqueGenSet(g *protogen.GeneratedFile, f *fileInfo, message *messageInfo,
// is always true for lazy message types. It is also true for all scalar fields.
// repeated, map or message fields are not using the presence map.
func usePresence(message *messageInfo, field *protogen.Field) bool {
if !message.isOpaque() || field.Desc.IsWeak() {
if !message.isOpaque() {
return false
}
return opaqueFieldNeedsPresenceArray(message, field)
Expand All @@ -635,22 +595,6 @@ func opaqueGenHas(g *protogen.GeneratedFile, f *fileInfo, message *messageInfo,
g.AnnotateSymbol(message.GoIdent.GoName+"."+hasserName, protogen.Annotation{Location: field.Location})
fieldtrackNoInterface(g, message.noInterface)

// Weak field.
if field.Desc.IsWeak() {
g.P(leadingComments, "func (x *", message.GoIdent, ") ", hasserName, "() bool {")
g.P("var w ", protoimplPackage.Ident("WeakFields"))
g.P("if x != nil {")
g.P("w = x.", genid.WeakFields_goname)
if message.isTracked {
g.P("_ = x.", genid.WeakFieldPrefix_goname+field.GoName)
}
g.P("}")
g.P("return ", protoimplPackage.Ident("X"), ".HasWeak(w, ", field.Desc.Number(), ")")
g.P("}")
g.P()
return
}

// Oneof field.
if oneof := field.Oneof; oneof != nil && !oneof.Desc.IsSynthetic() {
g.P(leadingComments, "func (x *", message.GoIdent, ") ", hasserName, "() bool {")
Expand Down Expand Up @@ -719,22 +663,6 @@ func opaqueGenClear(g *protogen.GeneratedFile, f *fileInfo, message *messageInfo
})
fieldtrackNoInterface(g, message.noInterface)

// Weak field.
if field.Desc.IsWeak() {
g.P(leadingComments, "func (x *", message.GoIdent, ") ", clearerName, "() {")
g.P("var w *", protoimplPackage.Ident("WeakFields"))
g.P("if x != nil {")
g.P("w = &x.", genid.WeakFields_goname)
if message.isTracked {
g.P("_ = x.", genid.WeakFieldPrefix_goname+field.GoName)
}
g.P("}")
g.P(protoimplPackage.Ident("X"), ".ClearWeak(w, ", field.Desc.Number(), ")")
g.P("}")
g.P()
return
}

// Oneof field.
if oneof := field.Oneof; oneof != nil && !oneof.Desc.IsSynthetic() {
g.P(leadingComments, "func (x *", message.GoIdent, ") ", clearerName, "() {")
Expand Down Expand Up @@ -937,9 +865,6 @@ func opaqueGenMessageBuilder(g *protogen.GeneratedFile, f *fileInfo, message *me
g.P()
for _, field := range message.Fields {
oneof := field.Oneof
if oneof == nil && field.Desc.IsWeak() {
continue
}

goType, pointer := opaqueBuilderFieldGoType(g, f, message, field)
if pointer {
Expand Down Expand Up @@ -1001,9 +926,6 @@ func opaqueGenBuildMethod(g *protogen.GeneratedFile, f *fileInfo, message *messa

for _, field := range message.Fields {
oneof := field.Oneof
if oneof == nil && field.Desc.IsWeak() {
continue
}
if oneof != nil && !oneof.Desc.IsSynthetic() {
qual := ""
if fieldDefaultValue(g, f, message, field) != "nil" {
Expand Down Expand Up @@ -1136,10 +1058,6 @@ func opaqueGenOneofWrapperTypes(g *protogen.GeneratedFile, f *fileInfo, message
//
// If it returns pointer=true, the struct field is a pointer to the type.
func opaqueFieldGoType(g *protogen.GeneratedFile, f *fileInfo, message *messageInfo, field *protogen.Field) (goType string, pointer bool) {
if field.Desc.IsWeak() {
return "struct{}", false
}

pointer = true
switch field.Desc.Kind() {
case protoreflect.BoolKind:
Expand Down
3 changes: 0 additions & 3 deletions cmd/protoc-gen-go/internal_gengo/reflect.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,6 @@ func genReflectFileDescriptor(gen *protogen.Plugin, g *protogen.GeneratedFile, f
depOffsets = append(depOffsets, offsetEntry{len(depIdxs), "field type_name"})
for _, message := range f.allMessages {
for _, field := range message.Fields {
if field.Desc.IsWeak() {
continue
}
source := string(field.Desc.FullName())
genEnum(field.Enum, source+":type_name")
genMessage(field.Message, source+":type_name")
Expand Down
Loading

0 comments on commit e677ef9

Please sign in to comment.