From 7259b467734a1d1adab45105fdef3afc89d61e7c Mon Sep 17 00:00:00 2001 From: Lasse Folger Date: Mon, 18 Mar 2024 15:58:47 +0100 Subject: [PATCH] all: implement proto2/proto3 as editions [1/2] This change removes most usages of Syntax() from the repository and uses edition features for instead. The appropriate edition feature defaults are loaded for proto2/proto3 when the initialization of the descriptors start. All of these changes were tested on the Google corpus. Change-Id: Ieca076a2b38ca8e50e084cd32e725b7b3dcb4171 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/572435 LUCI-TryBot-Result: Go LUCI Reviewed-by: Michael Stapelberg --- cmd/protoc-gen-go/internal_gengo/main.go | 8 +-- internal/cmd/generate-protos/main.go | 5 +- .../editiondefaults/editions_defaults.binpb | Bin 63 -> 78 bytes internal/encoding/tag/tag.go | 4 +- internal/filedesc/desc.go | 32 ++++------- internal/filedesc/desc_init.go | 50 ++++++++++++++++-- internal/filedesc/desc_lazy.go | 44 ++------------- internal/filedesc/editions.go | 2 + internal/impl/legacy_enum.go | 1 + internal/impl/legacy_extension.go | 2 +- internal/impl/legacy_message.go | 10 ++-- .../file_desc_init}/race_test.go | 0 .../messageset_extension_init/race_test.go | 28 ++++++++++ proto/noenforceutf8_test.go | 3 +- reflect/protodesc/desc.go | 6 +-- reflect/protodesc/desc_init.go | 48 +++++------------ 16 files changed, 129 insertions(+), 114 deletions(-) rename internal/{testprotos/race => race_test/file_desc_init}/race_test.go (100%) create mode 100644 internal/race_test/messageset_extension_init/race_test.go diff --git a/cmd/protoc-gen-go/internal_gengo/main.go b/cmd/protoc-gen-go/internal_gengo/main.go index 78a985f1..69e4aeca 100644 --- a/cmd/protoc-gen-go/internal_gengo/main.go +++ b/cmd/protoc-gen-go/internal_gengo/main.go @@ -289,11 +289,11 @@ func genEnum(g *protogen.GeneratedFile, f *fileInfo, e *enumInfo) { genEnumReflectMethods(g, f, e) // UnmarshalJSON method. - needsUnmarshalJSONMethod := e.genJSONMethod && e.Desc.Syntax() == protoreflect.Proto2 - if fde, ok := e.Desc.(*filedesc.Enum); ok && fde.L1.EditionFeatures.GenerateLegacyUnmarshalJSON { - needsUnmarshalJSONMethod = true + needsUnmarshalJSONMethod := false + if fde, ok := e.Desc.(*filedesc.Enum); ok { + needsUnmarshalJSONMethod = fde.L1.EditionFeatures.GenerateLegacyUnmarshalJSON } - if needsUnmarshalJSONMethod { + if e.genJSONMethod && needsUnmarshalJSONMethod { g.P("// Deprecated: Do not use.") g.P("func (x *", e.GoIdent, ") UnmarshalJSON(b []byte) error {") g.P("num, err := ", protoimplPackage.Ident("X"), ".UnmarshalJSONEnum(x.Descriptor(), b)") diff --git a/internal/cmd/generate-protos/main.go b/internal/cmd/generate-protos/main.go index 7daab9ec..b8720d7f 100644 --- a/internal/cmd/generate-protos/main.go +++ b/internal/cmd/generate-protos/main.go @@ -103,6 +103,8 @@ func main() { func generateEditionsDefaults() { dest := filepath.Join(repoRoot, "internal", "editiondefaults", "editions_defaults.binpb") + srcDescriptorProto := filepath.Join(protoRoot, "src", "google", "protobuf", "descriptor.proto") + srcGoFeatures := filepath.Join(repoRoot, "types", "gofeaturespb", "go_features.proto") // The enum in Go string formats to "EDITION_${EDITION}" but protoc expects // the flag in the form "${EDITION}". To work around this, we trim the // "EDITION_" prefix. @@ -113,7 +115,8 @@ func generateEditionsDefaults() { "--experimental_edition_defaults_out", dest, "--experimental_edition_defaults_minimum", minEdition, "--experimental_edition_defaults_maximum", maxEdition, - "--proto_path", protoRoot, "src/google/protobuf/descriptor.proto", + "-I"+filepath.Join(protoRoot, "src"), + "--proto_path", repoRoot, srcDescriptorProto, srcGoFeatures, ) out, err := cmd.CombinedOutput() if err != nil { diff --git a/internal/editiondefaults/editions_defaults.binpb b/internal/editiondefaults/editions_defaults.binpb index 18f0756874367adcdb790ffde125b6a7388b4eaa..f691305eb4f73440cd48caba949023eab52ef304 100644 GIT binary patch literal 78 zcmd-Q6B6WL6kw8IQef6#G+?@9$Hc)X@r<1dB+ewjD8Z<}1Qcfki8Dw%hln$xi@#u3 Kc*d^rf*k;APzzxI literal 63 zcmd-Q6yo7v6kw8IQef6#G+>f=#?A#2ViI7KU{qiN3NcDNhX^qu3B6!fc*d^rf*k<7 Cln3+x diff --git a/internal/encoding/tag/tag.go b/internal/encoding/tag/tag.go index 373d2083..7e87c760 100644 --- a/internal/encoding/tag/tag.go +++ b/internal/encoding/tag/tag.go @@ -32,6 +32,7 @@ var byteType = reflect.TypeOf(byte(0)) func Unmarshal(tag string, goType reflect.Type, evs protoreflect.EnumValueDescriptors) protoreflect.FieldDescriptor { f := new(filedesc.Field) f.L0.ParentFile = filedesc.SurrogateProto2 + f.L1.EditionFeatures = f.L0.ParentFile.L1.EditionFeatures for len(tag) > 0 { i := strings.IndexByte(tag, ',') if i < 0 { @@ -107,8 +108,7 @@ func Unmarshal(tag string, goType reflect.Type, evs protoreflect.EnumValueDescri f.L1.StringName.InitJSON(jsonName) } case s == "packed": - f.L1.HasPacked = true - f.L1.IsPacked = true + f.L1.EditionFeatures.IsPacked = true case strings.HasPrefix(s, "weak="): f.L1.IsWeak = true f.L1.Message = filedesc.PlaceholderMessage(protoreflect.FullName(s[len("weak="):])) diff --git a/internal/filedesc/desc.go b/internal/filedesc/desc.go index 8826bcf4..c17c128f 100644 --- a/internal/filedesc/desc.go +++ b/internal/filedesc/desc.go @@ -251,10 +251,6 @@ type ( StringName stringName IsProto3Optional bool // promoted from google.protobuf.FieldDescriptorProto IsWeak bool // promoted from google.protobuf.FieldOptions - HasPacked bool // promoted from google.protobuf.FieldOptions - IsPacked bool // promoted from google.protobuf.FieldOptions - HasEnforceUTF8 bool // promoted from google.protobuf.FieldOptions - EnforceUTF8 bool // promoted from google.protobuf.FieldOptions Default defaultValue ContainingOneof protoreflect.OneofDescriptor // must be consistent with Message.Oneofs.Fields Enum protoreflect.EnumDescriptor @@ -345,14 +341,7 @@ func (fd *Field) IsPacked() bool { case protoreflect.StringKind, protoreflect.BytesKind, protoreflect.MessageKind, protoreflect.GroupKind: return false } - if fd.L0.ParentFile.L1.Syntax == protoreflect.Editions { - return fd.L1.EditionFeatures.IsPacked - } - if fd.L0.ParentFile.L1.Syntax == protoreflect.Proto3 { - // proto3 repeated fields are packed by default. - return !fd.L1.HasPacked || fd.L1.IsPacked - } - return fd.L1.IsPacked + return fd.L1.EditionFeatures.IsPacked } func (fd *Field) IsExtension() bool { return false } func (fd *Field) IsWeak() bool { return fd.L1.IsWeak } @@ -399,13 +388,7 @@ func (fd *Field) ProtoType(protoreflect.FieldDescriptor) {} // WARNING: This method is exempt from the compatibility promise and may be // removed in the future without warning. func (fd *Field) EnforceUTF8() bool { - if fd.L0.ParentFile.L1.Syntax == protoreflect.Editions { - return fd.L1.EditionFeatures.IsUTF8Validated - } - if fd.L1.HasEnforceUTF8 { - return fd.L1.EnforceUTF8 - } - return fd.L0.ParentFile.L1.Syntax == protoreflect.Proto3 + return fd.L1.EditionFeatures.IsUTF8Validated } func (od *Oneof) IsSynthetic() bool { @@ -433,12 +416,19 @@ type ( Cardinality protoreflect.Cardinality Kind protoreflect.Kind EditionFeatures EditionFeatures + // To resolve the EditionFeatures we need to resolve the Extendee which + // happens at the end of the initialization of L1. Thus, we need to buffer + // the unresolved features (which are parsed when starting to initialize + // L1). We cannot move this to L2 because it is required to initialize + // Kind properly. Because some of the options (i.e. packed) affect the + // EditionFeatures we need to unmarshal the full options after resolving + // the Extendee. + rawOptions []byte } ExtensionL2 struct { Options func() protoreflect.ProtoMessage StringName stringName IsProto3Optional bool // promoted from google.protobuf.FieldDescriptorProto - IsPacked bool // promoted from google.protobuf.FieldOptions Default defaultValue Enum protoreflect.EnumDescriptor Message protoreflect.MessageDescriptor @@ -461,7 +451,7 @@ func (xd *Extension) HasPresence() bool { return xd.L1.Cardi func (xd *Extension) HasOptionalKeyword() bool { return (xd.L0.ParentFile.L1.Syntax == protoreflect.Proto2 && xd.L1.Cardinality == protoreflect.Optional) || xd.lazyInit().IsProto3Optional } -func (xd *Extension) IsPacked() bool { return xd.lazyInit().IsPacked } +func (xd *Extension) IsPacked() bool { return xd.L1.EditionFeatures.IsPacked } func (xd *Extension) IsExtension() bool { return true } func (xd *Extension) IsWeak() bool { return false } func (xd *Extension) IsList() bool { return xd.Cardinality() == protoreflect.Repeated } diff --git a/internal/filedesc/desc_init.go b/internal/filedesc/desc_init.go index 237e64fd..1de5ea9c 100644 --- a/internal/filedesc/desc_init.go +++ b/internal/filedesc/desc_init.go @@ -34,6 +34,20 @@ func newRawFile(db Builder) *File { for i := range fd.allExtensions { xd := &fd.allExtensions[i] xd.L1.Extendee = fd.resolveMessageDependency(xd.L1.Extendee, listExtTargets, int32(i)) + + // If the Extendee is not resolved, we cannot resolve the edition + // features of the parent. This should (to my knowledge) only happen + // for v1 messages for which we don't support editions. In v2 the + // Extendee should be resolved at binary start up time. + if _, ok := xd.L1.Extendee.(PlaceholderMessage); !ok { + xd.L1.EditionFeatures = featuresFromParentDesc(xd.L1.Extendee) + } + if xd.L1.rawOptions != nil { + xd.unmarshalOptions(xd.L1.rawOptions) + } + if xd.L1.Kind == protoreflect.MessageKind && xd.L1.EditionFeatures.IsDelimitedEncoded { + xd.L1.Kind = protoreflect.GroupKind + } } fd.checkDecls() @@ -113,8 +127,10 @@ func (fd *File) unmarshalSeed(b []byte) { switch string(v) { case "proto2": fd.L1.Syntax = protoreflect.Proto2 + fd.L1.Edition = EditionProto2 case "proto3": fd.L1.Syntax = protoreflect.Proto3 + fd.L1.Edition = EditionProto3 case "editions": fd.L1.Syntax = protoreflect.Editions default: @@ -177,11 +193,10 @@ func (fd *File) unmarshalSeed(b []byte) { // If syntax is missing, it is assumed to be proto2. if fd.L1.Syntax == 0 { fd.L1.Syntax = protoreflect.Proto2 + fd.L1.Edition = EditionProto2 } - if fd.L1.Syntax == protoreflect.Editions { - fd.L1.EditionFeatures = getFeaturesFor(fd.L1.Edition) - } + fd.L1.EditionFeatures = getFeaturesFor(fd.L1.Edition) // Parse editions features from options if any if options != nil { @@ -267,6 +282,7 @@ func (ed *Enum) unmarshalSeed(b []byte, sb *strs.Builder, pf *File, pd protorefl ed.L0.ParentFile = pf ed.L0.Parent = pd ed.L0.Index = i + ed.L1.EditionFeatures = featuresFromParentDesc(ed.Parent()) var numValues int for b := b; len(b) > 0; { @@ -467,6 +483,34 @@ func (xd *Extension) unmarshalSeed(b []byte, sb *strs.Builder, pf *File, pd prot xd.L0.FullName = appendFullName(sb, pd.FullName(), v) case genid.FieldDescriptorProto_Extendee_field_number: xd.L1.Extendee = PlaceholderMessage(makeFullName(sb, v)) + case genid.FieldDescriptorProto_Options_field_number: + xd.L1.rawOptions = v + } + default: + m := protowire.ConsumeFieldValue(num, typ, b) + b = b[m:] + } + } +} + +func (xd *Extension) unmarshalOptions(b []byte) { + for len(b) > 0 { + num, typ, n := protowire.ConsumeTag(b) + b = b[n:] + switch typ { + case protowire.VarintType: + v, m := protowire.ConsumeVarint(b) + b = b[m:] + switch num { + case genid.FieldOptions_Packed_field_number: + xd.L1.EditionFeatures.IsPacked = protowire.DecodeBool(v) + } + case protowire.BytesType: + v, m := protowire.ConsumeBytes(b) + b = b[m:] + switch num { + case genid.FieldOptions_Features_field_number: + xd.L1.EditionFeatures = unmarshalFeatureSet(v, xd.L1.EditionFeatures) } default: m := protowire.ConsumeFieldValue(num, typ, b) diff --git a/internal/filedesc/desc_lazy.go b/internal/filedesc/desc_lazy.go index 482a61cc..570181eb 100644 --- a/internal/filedesc/desc_lazy.go +++ b/internal/filedesc/desc_lazy.go @@ -466,10 +466,10 @@ func (fd *Field) unmarshalFull(b []byte, sb *strs.Builder, pf *File, pd protoref b = b[m:] } } - if fd.Syntax() == protoreflect.Editions && fd.L1.Kind == protoreflect.MessageKind && fd.L1.EditionFeatures.IsDelimitedEncoded { + if fd.L1.Kind == protoreflect.MessageKind && fd.L1.EditionFeatures.IsDelimitedEncoded { fd.L1.Kind = protoreflect.GroupKind } - if fd.Syntax() == protoreflect.Editions && fd.L1.EditionFeatures.IsLegacyRequired { + if fd.L1.EditionFeatures.IsLegacyRequired { fd.L1.Cardinality = protoreflect.Required } if rawTypeName != nil { @@ -496,13 +496,11 @@ func (fd *Field) unmarshalOptions(b []byte) { b = b[m:] switch num { case genid.FieldOptions_Packed_field_number: - fd.L1.HasPacked = true - fd.L1.IsPacked = protowire.DecodeBool(v) + fd.L1.EditionFeatures.IsPacked = protowire.DecodeBool(v) case genid.FieldOptions_Weak_field_number: fd.L1.IsWeak = protowire.DecodeBool(v) case FieldOptions_EnforceUTF8: - fd.L1.HasEnforceUTF8 = true - fd.L1.EnforceUTF8 = protowire.DecodeBool(v) + fd.L1.EditionFeatures.IsUTF8Validated = protowire.DecodeBool(v) } case protowire.BytesType: v, m := protowire.ConsumeBytes(b) @@ -548,7 +546,6 @@ func (od *Oneof) unmarshalFull(b []byte, sb *strs.Builder, pf *File, pd protoref func (xd *Extension) unmarshalFull(b []byte, sb *strs.Builder) { var rawTypeName []byte var rawOptions []byte - xd.L1.EditionFeatures = featuresFromParentDesc(xd.L1.Extendee) xd.L2 = new(ExtensionL2) for len(b) > 0 { num, typ, n := protowire.ConsumeTag(b) @@ -572,7 +569,6 @@ func (xd *Extension) unmarshalFull(b []byte, sb *strs.Builder) { case genid.FieldDescriptorProto_TypeName_field_number: rawTypeName = v case genid.FieldDescriptorProto_Options_field_number: - xd.unmarshalOptions(v) rawOptions = appendOptions(rawOptions, v) } default: @@ -580,12 +576,6 @@ func (xd *Extension) unmarshalFull(b []byte, sb *strs.Builder) { b = b[m:] } } - if xd.Syntax() == protoreflect.Editions && xd.L1.Kind == protoreflect.MessageKind && xd.L1.EditionFeatures.IsDelimitedEncoded { - xd.L1.Kind = protoreflect.GroupKind - } - if xd.Syntax() == protoreflect.Editions && xd.L1.EditionFeatures.IsLegacyRequired { - xd.L1.Cardinality = protoreflect.Required - } if rawTypeName != nil { name := makeFullName(sb, rawTypeName) switch xd.L1.Kind { @@ -598,32 +588,6 @@ func (xd *Extension) unmarshalFull(b []byte, sb *strs.Builder) { xd.L2.Options = xd.L0.ParentFile.builder.optionsUnmarshaler(&descopts.Field, rawOptions) } -func (xd *Extension) unmarshalOptions(b []byte) { - for len(b) > 0 { - num, typ, n := protowire.ConsumeTag(b) - b = b[n:] - switch typ { - case protowire.VarintType: - v, m := protowire.ConsumeVarint(b) - b = b[m:] - switch num { - case genid.FieldOptions_Packed_field_number: - xd.L2.IsPacked = protowire.DecodeBool(v) - } - case protowire.BytesType: - v, m := protowire.ConsumeBytes(b) - b = b[m:] - switch num { - case genid.FieldOptions_Features_field_number: - xd.L1.EditionFeatures = unmarshalFeatureSet(v, xd.L1.EditionFeatures) - } - default: - m := protowire.ConsumeFieldValue(num, typ, b) - b = b[m:] - } - } -} - func (sd *Service) unmarshalFull(b []byte, sb *strs.Builder) { var rawMethods [][]byte var rawOptions []byte diff --git a/internal/filedesc/editions.go b/internal/filedesc/editions.go index 0375a49d..443b70cb 100644 --- a/internal/filedesc/editions.go +++ b/internal/filedesc/editions.go @@ -17,6 +17,8 @@ var defaultsCache = make(map[Edition]EditionFeatures) func init() { unmarshalEditionDefaults(editiondefaults.Defaults) + SurrogateProto2.L1.EditionFeatures = getFeaturesFor(EditionProto2) + SurrogateProto3.L1.EditionFeatures = getFeaturesFor(EditionProto3) } func unmarshalGoFeature(b []byte, parent EditionFeatures) EditionFeatures { diff --git a/internal/impl/legacy_enum.go b/internal/impl/legacy_enum.go index c2a803bb..c1c33d00 100644 --- a/internal/impl/legacy_enum.go +++ b/internal/impl/legacy_enum.go @@ -167,6 +167,7 @@ func aberrantLoadEnumDesc(t reflect.Type) protoreflect.EnumDescriptor { ed := &filedesc.Enum{L2: new(filedesc.EnumL2)} ed.L0.FullName = AberrantDeriveFullName(t) // e.g., github_com.user.repo.MyEnum ed.L0.ParentFile = filedesc.SurrogateProto3 + ed.L1.EditionFeatures = ed.L0.ParentFile.L1.EditionFeatures ed.L2.Values.List = append(ed.L2.Values.List, filedesc.EnumValue{}) // TODO: Use the presence of a UnmarshalJSON method to determine proto2? diff --git a/internal/impl/legacy_extension.go b/internal/impl/legacy_extension.go index 87b30d05..6e8677ee 100644 --- a/internal/impl/legacy_extension.go +++ b/internal/impl/legacy_extension.go @@ -118,7 +118,7 @@ func (xi *ExtensionInfo) initFromLegacy() { xd.L1.Number = protoreflect.FieldNumber(xi.Field) xd.L1.Cardinality = fd.L1.Cardinality xd.L1.Kind = fd.L1.Kind - xd.L2.IsPacked = fd.L1.IsPacked + xd.L1.EditionFeatures = fd.L1.EditionFeatures xd.L2.Default = fd.L1.Default xd.L1.Extendee = Export{}.MessageDescriptorOf(xi.ExtendedType) xd.L2.Enum = ed diff --git a/internal/impl/legacy_message.go b/internal/impl/legacy_message.go index 2ab2c629..950e9a1f 100644 --- a/internal/impl/legacy_message.go +++ b/internal/impl/legacy_message.go @@ -204,6 +204,7 @@ func aberrantLoadMessageDescReentrant(t reflect.Type, name protoreflect.FullName } } + md.L1.EditionFeatures = md.L0.ParentFile.L1.EditionFeatures // Obtain a list of oneof wrapper types. var oneofWrappers []reflect.Type methods := make([]reflect.Method, 0, 2) @@ -250,6 +251,7 @@ func aberrantLoadMessageDescReentrant(t reflect.Type, name protoreflect.FullName od := &md.L2.Oneofs.List[n] od.L0.FullName = md.FullName().Append(protoreflect.Name(tag)) od.L0.ParentFile = md.L0.ParentFile + od.L1.EditionFeatures = md.L1.EditionFeatures od.L0.Parent = md od.L0.Index = n @@ -260,6 +262,7 @@ func aberrantLoadMessageDescReentrant(t reflect.Type, name protoreflect.FullName aberrantAppendField(md, f.Type, tag, "", "") fd := &md.L2.Fields.List[len(md.L2.Fields.List)-1] fd.L1.ContainingOneof = od + fd.L1.EditionFeatures = od.L1.EditionFeatures od.L1.Fields.List = append(od.L1.Fields.List, fd) } } @@ -307,14 +310,14 @@ func aberrantAppendField(md *filedesc.Message, goType reflect.Type, tag, tagKey, fd.L0.Parent = md fd.L0.Index = n - if fd.L1.IsWeak || fd.L1.HasPacked { + if fd.L1.IsWeak || fd.L1.EditionFeatures.IsPacked { fd.L1.Options = func() protoreflect.ProtoMessage { opts := descopts.Field.ProtoReflect().New() if fd.L1.IsWeak { opts.Set(opts.Descriptor().Fields().ByName("weak"), protoreflect.ValueOfBool(true)) } - if fd.L1.HasPacked { - opts.Set(opts.Descriptor().Fields().ByName("packed"), protoreflect.ValueOfBool(fd.L1.IsPacked)) + if fd.L1.EditionFeatures.IsPacked { + opts.Set(opts.Descriptor().Fields().ByName("packed"), protoreflect.ValueOfBool(fd.L1.EditionFeatures.IsPacked)) } return opts.Interface() } @@ -344,6 +347,7 @@ func aberrantAppendField(md *filedesc.Message, goType reflect.Type, tag, tagKey, md2.L0.ParentFile = md.L0.ParentFile md2.L0.Parent = md md2.L0.Index = n + md2.L1.EditionFeatures = md.L1.EditionFeatures md2.L1.IsMapEntry = true md2.L2.Options = func() protoreflect.ProtoMessage { diff --git a/internal/testprotos/race/race_test.go b/internal/race_test/file_desc_init/race_test.go similarity index 100% rename from internal/testprotos/race/race_test.go rename to internal/race_test/file_desc_init/race_test.go diff --git a/internal/race_test/messageset_extension_init/race_test.go b/internal/race_test/messageset_extension_init/race_test.go new file mode 100644 index 00000000..87f8c0e8 --- /dev/null +++ b/internal/race_test/messageset_extension_init/race_test.go @@ -0,0 +1,28 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package race_test + +import ( + "sync" + "testing" + + epb "google.golang.org/protobuf/internal/testprotos/messageset/msetextpb" +) + +// There must be no other test in this package as we are testing global +// initialization which only happens once per binary. +func TestConcurrentInitialization(t *testing.T) { + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + epb.E_Ext1_MessageSetExtension.ValueOf(&epb.Ext1{}) + }() + go func() { + defer wg.Done() + epb.E_Ext1_MessageSetExtension.TypeDescriptor().Message() + }() + wg.Wait() +} diff --git a/proto/noenforceutf8_test.go b/proto/noenforceutf8_test.go index 2bd011ef..e4b3ca30 100644 --- a/proto/noenforceutf8_test.go +++ b/proto/noenforceutf8_test.go @@ -134,8 +134,7 @@ var messageInfo_TestNoEnforceUTF8 = protoimpl.MessageInfo{ } md := fd.Messages().Get(0) for i := 0; i < md.Fields().Len(); i++ { - md.Fields().Get(i).(*filedesc.Field).L1.HasEnforceUTF8 = true - md.Fields().Get(i).(*filedesc.Field).L1.EnforceUTF8 = false + md.Fields().Get(i).(*filedesc.Field).L1.EditionFeatures.IsUTF8Validated = false } return md }(), diff --git a/reflect/protodesc/desc.go b/reflect/protodesc/desc.go index baa0cc62..2e67db1d 100644 --- a/reflect/protodesc/desc.go +++ b/reflect/protodesc/desc.go @@ -91,8 +91,10 @@ func (o FileOptions) New(fd *descriptorpb.FileDescriptorProto, r Resolver) (prot switch fd.GetSyntax() { case "proto2", "": f.L1.Syntax = protoreflect.Proto2 + f.L1.Edition = filedesc.EditionProto2 case "proto3": f.L1.Syntax = protoreflect.Proto3 + f.L1.Edition = filedesc.EditionProto3 case "editions": f.L1.Syntax = protoreflect.Editions f.L1.Edition = fromEditionProto(fd.GetEdition()) @@ -114,9 +116,7 @@ func (o FileOptions) New(fd *descriptorpb.FileDescriptorProto, r Resolver) (prot opts = proto.Clone(opts).(*descriptorpb.FileOptions) f.L2.Options = func() protoreflect.ProtoMessage { return opts } } - if f.L1.Syntax == protoreflect.Editions { - initFileDescFromFeatureSet(f, fd.GetOptions().GetFeatures()) - } + initFileDescFromFeatureSet(f, fd.GetOptions().GetFeatures()) f.L2.Imports = make(filedesc.FileImports, len(fd.GetDependency())) for _, i := range fd.GetPublicDependency() { diff --git a/reflect/protodesc/desc_init.go b/reflect/protodesc/desc_init.go index b3278163..ff5be207 100644 --- a/reflect/protodesc/desc_init.go +++ b/reflect/protodesc/desc_init.go @@ -69,9 +69,7 @@ func (r descsByName) initMessagesDeclarations(mds []*descriptorpb.DescriptorProt if m.L0, err = r.makeBase(m, parent, md.GetName(), i, sb); err != nil { return nil, err } - if m.Base.L0.ParentFile.Syntax() == protoreflect.Editions { - m.L1.EditionFeatures = mergeEditionFeatures(parent, md.GetOptions().GetFeatures()) - } + m.L1.EditionFeatures = mergeEditionFeatures(parent, md.GetOptions().GetFeatures()) if opts := md.GetOptions(); opts != nil { opts = proto.Clone(opts).(*descriptorpb.MessageOptions) m.L2.Options = func() protoreflect.ProtoMessage { return opts } @@ -146,13 +144,15 @@ func (r descsByName) initFieldsFromDescriptorProto(fds []*descriptorpb.FieldDesc if f.L0, err = r.makeBase(f, parent, fd.GetName(), i, sb); err != nil { return nil, err } + f.L1.EditionFeatures = mergeEditionFeatures(parent, fd.GetOptions().GetFeatures()) f.L1.IsProto3Optional = fd.GetProto3Optional() if opts := fd.GetOptions(); opts != nil { opts = proto.Clone(opts).(*descriptorpb.FieldOptions) f.L1.Options = func() protoreflect.ProtoMessage { return opts } f.L1.IsWeak = opts.GetWeak() - f.L1.HasPacked = opts.Packed != nil - f.L1.IsPacked = opts.GetPacked() + if opts.Packed != nil { + f.L1.EditionFeatures.IsPacked = opts.GetPacked() + } } f.L1.Number = protoreflect.FieldNumber(fd.GetNumber()) f.L1.Cardinality = protoreflect.Cardinality(fd.GetLabel()) @@ -163,32 +163,12 @@ func (r descsByName) initFieldsFromDescriptorProto(fds []*descriptorpb.FieldDesc f.L1.StringName.InitJSON(fd.GetJsonName()) } - if f.Base.L0.ParentFile.Syntax() == protoreflect.Editions { - f.L1.EditionFeatures = mergeEditionFeatures(parent, fd.GetOptions().GetFeatures()) + if f.L1.EditionFeatures.IsLegacyRequired { + f.L1.Cardinality = protoreflect.Required + } - if f.L1.EditionFeatures.IsLegacyRequired { - f.L1.Cardinality = protoreflect.Required - } - // We reuse the existing field because the old option `[packed = - // true]` is mutually exclusive with the editions feature. - if canBePacked(fd) { - f.L1.HasPacked = true - f.L1.IsPacked = f.L1.EditionFeatures.IsPacked - } - - // We pretend this option is always explicitly set because the only - // use of HasEnforceUTF8 is to determine whether to use EnforceUTF8 - // or to return the appropriate default. - // When using editions we either parse the option or resolve the - // appropriate default here (instead of later when this option is - // requested from the descriptor). - // In proto2/proto3 syntax HasEnforceUTF8 might be false. - f.L1.HasEnforceUTF8 = true - f.L1.EnforceUTF8 = f.L1.EditionFeatures.IsUTF8Validated - - if f.L1.Kind == protoreflect.MessageKind && f.L1.EditionFeatures.IsDelimitedEncoded { - f.L1.Kind = protoreflect.GroupKind - } + if f.L1.Kind == protoreflect.MessageKind && f.L1.EditionFeatures.IsDelimitedEncoded { + f.L1.Kind = protoreflect.GroupKind } } return fs, nil @@ -201,12 +181,10 @@ func (r descsByName) initOneofsFromDescriptorProto(ods []*descriptorpb.OneofDesc if o.L0, err = r.makeBase(o, parent, od.GetName(), i, sb); err != nil { return nil, err } + o.L1.EditionFeatures = mergeEditionFeatures(parent, od.GetOptions().GetFeatures()) if opts := od.GetOptions(); opts != nil { opts = proto.Clone(opts).(*descriptorpb.OneofOptions) o.L1.Options = func() protoreflect.ProtoMessage { return opts } - if parent.Syntax() == protoreflect.Editions { - o.L1.EditionFeatures = mergeEditionFeatures(parent, opts.GetFeatures()) - } } } return os, nil @@ -223,7 +201,9 @@ func (r descsByName) initExtensionDeclarations(xds []*descriptorpb.FieldDescript if opts := xd.GetOptions(); opts != nil { opts = proto.Clone(opts).(*descriptorpb.FieldOptions) x.L2.Options = func() protoreflect.ProtoMessage { return opts } - x.L2.IsPacked = opts.GetPacked() + if opts.Packed != nil { + x.L1.EditionFeatures.IsPacked = opts.GetPacked() + } } x.L1.Number = protoreflect.FieldNumber(xd.GetNumber()) x.L1.Cardinality = protoreflect.Cardinality(xd.GetLabel())