Skip to content

Commit

Permalink
reflect/protodesc: fix packed field conditional
Browse files Browse the repository at this point in the history
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 e8baad6.

Change-Id: I45d2b07a85ee40cb7c028098b81c4e76bf2ff555
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/554896
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Cassondra Foesch <[email protected]>
Reviewed-by: Christian Höppner <[email protected]>
Auto-Submit: Michael Stapelberg <[email protected]>
  • Loading branch information
stapelberg authored and gopherbot committed Jan 9, 2024
1 parent 7c85df2 commit 26a52f3
Showing 1 changed file with 22 additions and 1 deletion.
23 changes: 22 additions & 1 deletion reflect/protodesc/desc_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down

0 comments on commit 26a52f3

Please sign in to comment.