reflect/protoreflect: clarify Get semantics on unpopulated fields
Clearly specify that Get on an unpopulated field:
* returns the default value for scalars
* returns a mutable (but empty) List for repeated fields
* returns a mutable (but empty) Map for map fields
* returns an invalid value for message fields
The difference in semantics between List+Maps and Messages is because
protobuf semantics provide no distinction between an unpopulated and empty list
or map. On the other hand, there is a semantic difference between an unpopulated
message and an empty message.
Default values for scalars is trivial to implement with FieldDescriptor.Default.
A mutable, but empty List and Map is easy to implement for known fields since
known fields are generated as a slice or map field in a struct.
Since struct fields are addressable, the implementation can just return a
reference to the slice or map.
Repeated, extension fields are a little more tricky since extension fields
are implemented under the hood as a map[FieldNumber]Extension.
Rather than allocating an empty list in KnownFields.Get upon first retrieval
(which presents a race), delegate the work to ExtensionFieldTypes.Register,
which must occur before any Get operation. Register is not a concurrent-safe
operation, so that is an excellent time to initilize empty lists.
The implementation of extensions will need to be careful that Clear on a repeated
field simply truncates it zero instead of deleting the object.
For unpopulated messages, we return an invalid value, instead of the prior
behavior of returning a typed nil-pointer to the Go type for the message.
The approach is problematic because it assumes that
1) all messages are always implemented on a pointer reciever
2) a typed nil-pointer is an appropriate "read-only, but empty" message
These assumptions are not true of all message types (e.g., dynamic messages).
Change-Id: Ie96e6744c890308d9de738b6cf01d3b19e7e7c6a
Reviewed-on: https://go-review.googlesource.com/c/150319
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/internal/impl/message_test.go b/internal/impl/message_test.go
index b3393d4..999bdd6 100644
--- a/internal/impl/message_test.go
+++ b/internal/impl/message_test.go
@@ -1017,7 +1017,6 @@
func (*EnumMessages_OneofM3) isEnumMessages_Union() {}
func TestEnumMessages(t *testing.T) {
- // TODO: Test behavior of Get on unpopulated message.
wantL := MessageOf(&proto2_20180125.Message{OptionalFloat: protoV1.Float32(math.E)})
wantM := &EnumMessages{EnumP2: EnumProto2(1234).Enum()}
wantM2a := &ScalarProto2{Float32: protoV1.Float32(math.Pi)}
@@ -1033,7 +1032,7 @@
testMessage(t, nil, &EnumMessages{}, messageOps{
hasFields{1: false, 2: false, 3: false, 4: false, 5: false, 6: false, 7: false, 8: false, 9: false, 10: false, 11: false, 12: false},
- getFields{1: VE(0xbeef), 2: VE(1), 9: VE(0xbeef), 10: VE(1)},
+ getFields{1: VE(0xbeef), 2: VE(1), 3: V(nil), 4: V(nil), 9: VE(0xbeef), 10: VE(1)},
// Test singular enums.
setFields{1: VE(0xdead), 2: VE(0)},