reflect/prototype: make extensions use proto2 semantics for IsPacked
Extensions are normally thought of as a proto2 feature.
However, proto3 actually allows declarations of field extensions *only*
if they extend messages declared in descriptor.proto.
In such a situation it is not well defined whether the extension field
operates under proto2 or proto3 properties.
In some ways, such declarations are proto3-like.
For example, the declaration in the proto file still follows
proto3's syntactical grammar (e.g., no "optional" label required)
and proto3 semantics (e.g., default values are not allowed).
However, in other ways, such declarations are proto2-like.
For example, a repeated field of numeric scalars is not automatically packed.
This property is determined by the following test.
Compile the following proto file:
syntax = "proto3";
import "google/protobuf/descriptor.proto";
extend google.protobuf.FieldOptions {
repeated sint32 test_field = 20181023;
}
then compile and run a C++ program with the following snippet:
google::protobuf::FieldOptions m;
m.AddExtension(test_field, 1);
m.AddExtension(test_field, 2);
m.AddExtension(test_field, 3);
fstream output("out", ios::out | ios::trunc | ios::binary);
m.SerializeToOstream(&output);
which produces an "out" file with the following contents:
$ pbdump -sints 20181023 out
Message{
Tag{20181023, Varint}, Svarint(1),
Tag{20181023, Varint}, Svarint(2),
Tag{20181023, Varint}, Svarint(3),
}
which is indicative that packed encoding was not used by default (proto2-like).
If we repeat the above experiment and explicit set "[packed = true]",
then packed encoding is used:
$ pbdump -sints 20181023 out
Message{
Tag{20181023, Bytes}, LengthPrefix{Svarint(1), Svarint(2), Svarint(3)},
}
Note that this change does not mean that field extensions are always proto2
since the experiment above does not conclusively prove that. Thus, the Syntax
on extensions derived from the descriptor protos may still report proto3.
The behavior of packed encoding with regards to proto3 says:
In proto3, repeated fields of scalar numeric types use packed encoding by default.
One way to interpret this is:
In proto3 (messages), repeated fields of scalar numeric types use packed encoding by default.
In which case packedness is a property of the message's syntax
rather than the field's syntax (if such a distinction exists).
Since only proto2 messages can be extended, we can safely assume that all
extension fields use proto2 semantics for IsPacked.
Change-Id: Iae595c6d88c6e252cae7552cae083bad42f2494a
Reviewed-on: https://go-review.googlesource.com/c/144278
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/reflect/prototype/protofile_type.go b/reflect/prototype/protofile_type.go
index 83901b8..27d4db1 100644
--- a/reflect/prototype/protofile_type.go
+++ b/reflect/prototype/protofile_type.go
@@ -342,7 +342,7 @@
func (t extensionDesc) Cardinality() pref.Cardinality { return t.x.Cardinality }
func (t extensionDesc) Kind() pref.Kind { return t.x.Kind }
func (t extensionDesc) JSONName() string { return "" }
-func (t extensionDesc) IsPacked() bool { return extIsPacked(t) }
+func (t extensionDesc) IsPacked() bool { return t.x.Options.GetPacked() }
func (t extensionDesc) IsMap() bool { return false }
func (t extensionDesc) IsWeak() bool { return false }
func (t extensionDesc) Default() pref.Value { return t.x.dv.lazyInit(t, t.x.Default) }
@@ -359,15 +359,6 @@
func (t extensionDesc) ProtoType(pref.FieldDescriptor) {}
func (t extensionDesc) ProtoInternal(pragma.DoNotImplement) {}
-func extIsPacked(t extensionDesc) bool {
- // TODO: When a proto3 file extends a proto2 message (permitted only for
- // extending descriptor options), does the extension have proto2 or proto3
- // semantics? If the latter, repeated, scalar, numeric, proto3 extension
- // fields should default to packed. If the former, perhaps the extension syntax
- // should be protoreflect.Proto2.
- return t.x.Options.GetPacked()
-}
-
type enumMeta struct {
inheritedMeta
