7 Commits

Author SHA1 Message Date
Damien Neil
9f1165c3bf all: fix reflection behavior for empty lists and maps
The protoreflect documentation states that Get on an empty repeated or
map field returns an invalid (empty, read-only) List or Map. We weren't
doing this; fix it.

The documentation also states that an invalid List or Map may not be
assigned via Set. We were permitting Set with an invalid map; fix this.

Add tests for this behavior in prototest.

Change-Id: I4678af532e192210af0bde7c96a1439a4cd26efa
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/209019
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-11-26 22:58:51 +00:00
Damien Neil
6e40b32926 internal/impl: weak field bugfixes
Fix a reversed error check in impl.Export{}.WeakNil.

Check to see if we have a type for the weak field on marshal/size.

Treat a typed nil valued in XXX_Weak as not indicating presence for
the field.

Change-Id: Id667ac7eb4f53236be9e181017082bd8cd21d115
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/198717
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-10-04 17:07:45 +00:00
Joe Tsai
9d637ca923 internal/impl: optimize reflect methods
This change performs two optimizations:
* It uses a pre-constructed rangeInfos slice to iterate over
all the fields. This is more performant since iterating over a slice
is faster than iterating over a map. Furthermore, this slice
does not contain fields that are part of a oneof. If a oneof has
N fields, the time to check presence on the oneof is now O(1)
instead of O(N).
* It uses a dense field info slice that is optmized for the common
case where the field number is relatively low and close in value
to the index itself.

We also fix a minor bug in the construction of oneofInfo where
it wasn't treating a typed nil pointer to a wrapper struct as if
it were unset. This ensures WhichOneof and Has always agree.

name             old time/op    new time/op    delta
Reflect/Has-4      7.81µs ± 3%    6.74µs ± 3%  -13.61%  (p=0.000 n=9+9)
Reflect/Get-4      12.7µs ± 1%    11.3µs ± 4%  -10.85%  (p=0.000 n=8+10)
Reflect/Set-4      19.5µs ± 5%    17.8µs ± 2%   -8.99%  (p=0.000 n=10+10)
Reflect/Clear-4    12.0µs ± 4%    10.2µs ± 3%  -14.86%  (p=0.000 n=9+10)
Reflect/Range-4    6.58µs ± 1%    4.17µs ± 2%  -36.65%  (p=0.000 n=8+9)

Change-Id: I2c48b4d3fb6103ab238924950529ded0d37f8c8a
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/196358
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-19 17:45:34 +00:00
Joe Tsai
84177c9bf3 all: use typed variant of protoreflect.ValueOf
Change-Id: I7479632b57e7c8efade12a2eb2b855e9c321adb1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/196037
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-17 21:33:16 +00:00
Joe Tsai
fd528ff53f internal/impl: unexport Unwrapper
This interface is only ever used within the impl package.
Unexport it to avoid potential abuse by users.

Change-Id: I58d0cc2e8cb01c8c9d36cdec469c8d2196c4f836
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/193197
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-05 06:07:39 +00:00
Joe Tsai
9b22b9382e internal/impl: treat a nil oneof wrapper as if it were unset
The old implementation had the behavior where a nil wrapper value:
	m := new(foopb.Message)
	m.OneofField = (*foopb.Message_OneofUint32)(nil)
was functionally equivalent to it being directly set to nil:
	m := new(foopb.Message)
	m.OneofField = nil
preserve this semantic in both the table-drive implementation
and the reflection implementation.

Change-Id: Ie44d51e044d4822e61d0e646fbc44aa8d9b90c1f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189559
Reviewed-by: Herbie Ong <herbie@google.com>
2019-08-16 00:24:53 +00:00
Joe Tsai
0484b1a125 internal/impl: refactor MessageInfo
Changes made:
* Move MessageInfo.{methods,extensionFieldInfosMu,extensionFieldInfos}
to the coderMessageInfo type since they are fields all related to the
fast-path implementation, which is what the coderMessageInfo is for.
* Rename message_field.go -> message_reflect_field.go to make it obvious
from the file name that this only deals with the reflection implementation.
* Rename message_test.go -> message_reflect_test.go.
* Move reflection-specific implementation functions from message.go
to message_reflect.go. The intention is to make it such that message.go
is the entry point to message implementations and is agnostic towards
whether we are implementing reflection or the table-driven fast path.

Overall, there is no semantic changes. Just code movement.

Change-Id: I7743c39ba84dc63cd2a02934c3319285e16d6b1c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/190100
Reviewed-by: Herbie Ong <herbie@google.com>
2019-08-15 22:26:40 +00:00