From 26a52f39da85b22417ba99e9383db1d2000149b3 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Tue, 9 Jan 2024 14:33:17 +0100 Subject: [PATCH] reflect/protodesc: fix packed field conditional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this change, calling dynamicpb.NewMessage() on a protoreflect.MessageDescriptor that uses Editions would fail with: message field "[…].relatedqueries" is not packable (While Go Protobuf does not yet support Editions, C++ programs can save descriptors to a file that is later loaded by a Go program.) This was an oversight in commit e8baad6b6c9e2bb1c48e4963866248d4f35d4fd7. Change-Id: I45d2b07a85ee40cb7c028098b81c4e76bf2ff555 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/554896 LUCI-TryBot-Result: Go LUCI Reviewed-by: Cassondra Foesch Reviewed-by: Christian Höppner Auto-Submit: Michael Stapelberg --- reflect/protodesc/desc_init.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/reflect/protodesc/desc_init.go b/reflect/protodesc/desc_init.go index aff6fd49..e67a1541 100644 --- a/reflect/protodesc/desc_init.go +++ b/reflect/protodesc/desc_init.go @@ -114,6 +114,27 @@ func (r descsByName) initMessagesDeclarations(mds []*descriptorpb.DescriptorProt return ms, nil } +// canBePacked returns whether the field can use packed encoding: +// https://protobuf.dev/programming-guides/encoding/#packed +func canBePacked(fd *descriptorpb.FieldDescriptorProto) bool { + if fd.GetLabel() != descriptorpb.FieldDescriptorProto_LABEL_REPEATED { + return false // not a repeated field + } + + switch protoreflect.Kind(fd.GetType()) { + case protoreflect.MessageKind, protoreflect.GroupKind: + return false // not a scalar type field + + case protoreflect.StringKind, protoreflect.BytesKind: + // string and bytes can explicitly not be declared as packed, + // see https://protobuf.dev/programming-guides/encoding/#packed + return false + + default: + return true + } +} + func (r descsByName) initFieldsFromDescriptorProto(fds []*descriptorpb.FieldDescriptorProto, parent protoreflect.Descriptor, sb *strs.Builder) (fs []filedesc.Field, err error) { fs = make([]filedesc.Field, len(fds)) // allocate up-front to ensure stable pointers for i, fd := range fds { @@ -142,7 +163,7 @@ func (r descsByName) initFieldsFromDescriptorProto(fds []*descriptorpb.FieldDesc f.L1.Presence = resolveFeatureHasFieldPresence(f.Base.L0.ParentFile, fd) // We reuse the existing field because the old option `[packed = // true]` is mutually exclusive with the editions feature. - if fd.GetLabel() == descriptorpb.FieldDescriptorProto_LABEL_REPEATED { + if canBePacked(fd) { f.L1.HasPacked = true f.L1.IsPacked = resolveFeatureRepeatedFieldEncodingPacked(f.Base.L0.ParentFile, fd) }