165 Commits

Author SHA1 Message Date
Joe Tsai
d01061a42e internal/impl: print <nil> for typed nil messages
The Message.String method is intended for debugging,
so outputting <nil> for typed nil messages actually has use.
This matches the current behavior of Message.String
for messages generated by the v1 protoc-gen-go.

Change-Id: I6e31183961c83d7bce6338eb99aa7758cfda1ff4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/199397
Reviewed-by: Damien Neil <dneil@google.com>
2019-10-09 22:33:26 +00:00
Damien Neil
3b65992c65 all: remove {Enum,Message,Extension}Type.GoType methods
These methods are difficult or impossible to use correctly; types
created by the dynamicpb package, for example, all have the same GoType.

Fixes golang/protobuf#938

Change-Id: I33d4ef381579ff18569b11df501d0ba7f38a6b5c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/199060
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-10-07 21:08:26 +00:00
Damien Neil
01fdc63239 internal/impl: use current proto package, not old one
Correct a stray reference to github.com/golang/protobuf.

Updates golang/protobuf#962

Change-Id: I4a5f75d0aee61d28f18919a3ea728dc7a726da62
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/198998
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-10-04 17:42:11 +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
Damien Neil
47d5893acf internal/impl: support non-struct-pointer legacy message types
Support, to some limited degree, types which implement protoV1.Message
but which are not struct pointers. Our ability to work with these types
is largely limited to calling Marshal or Unmarshal methods, when
present.

Change-Id: Ie1b851d9e753e2b2cb189b17ffeefebe2d8b3a8f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/198237
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-10-04 16:13:03 +00:00
Damien Neil
37ef691e6b internal/impl: call Marshal/Unmarshal methods on legacy types
Call the Marshal or Unmarshal method on legacy messages implementing
protoV1.Marshaler or protoV2.Unmarshaler.

We do this in the impl package by creating an appropriate function in
the protoiface.Methods struct for legacy messages.

In proto.MarshalAppend, return the bytes provided by the fast-path
marshal function even when the returned error is non-nil.

Fixes golang/protobuf#955

Change-Id: I36924af9ff959a946c43f2295ef3202216e81b32
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/197357
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-09-26 20:54:39 +00:00
Joe Tsai
bee625258d internal/impl: handle legacy ExtensionDesc form
Add support for an ExtensionDesc with only Field populated as returned by
protoV1.ExtensionDescs. Rather than panicking when TypeDescriptor is called,
return a placeholder that preserves the name and/or number.

Change-Id: I60352a7aec8ccd8a0c1fb08db5891043a441695f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/193725
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-19 23:27:15 +00:00
Joe Tsai
6e095998ae proto, internal/impl: implement support for weak fields
Change-Id: I0a3ff79542a3316295fd6c58e1447e597be97ab9
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189923
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-19 22:41:12 +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
705acadcc7 proto: reset message by default in Unmarshal
We change Unmarshal to reset a message by default.
* We add a Merge option to UnmarshalOptions for explicit merging.
* We speed up Reset by checking for the Reset method.
* Remove TODOs in prototext and protojson about reset behavior.

Fixes golang/protobuf#890

Change-Id: Ibd8963c741053f564acf061fbdb846699942109c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/195457
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-17 21:13:42 +00:00
Joe Tsai
cd4a31e202 encoding/prototext: add MarshalOptions.EmitUnknown
This changes text marshaling to avoid unknown fields by default
and instead adds an option so that unknown fields be emitted.
This ensures that the default marshal/unknown can round-trip.

Change-Id: I85c84ba6ab7916d538ec6bfd4e9d399a8fcba14e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/195778
Reviewed-by: Herbie Ong <herbie@google.com>
2019-09-17 02:56:29 +00:00
Joe Tsai
cbdefb1530 internal/impl: move filetype.EnumInfo to impl
Change-Id: I5558d9d4618df80dd08969fb8af1bc16191176b5
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/195620
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-16 22:07:05 +00:00
Joe Tsai
fd4c605bfb internal: simplify ExtensionInfo initialization
This CL:
* Make the meaning of impl/ExtensionInfo.goType consistent. Before,
it was sometimes a T and other times a []T depending on the current
state of initialization. Change it so that it is the constructor's
responsibility to pass in a []T if it is repeated.
* Make internal/filetype responsible for constructing a []T for
repeated extension fields.
* Makes filedesc/Extension.Cardinality one of the eagerly initialized
pieces of information since it is useful to internal/filetype.
* Unify ExtensionInfo.desc and ExtensionInfo.tdesc.ExtensionField,
which held the same information.
* Remove the internal implementation for impl.X.ExtensionDescFromType
since we are dropping support for this from v1.

Change-Id: Ie95c4de66cd674c1d886da4f63b133b7d763c7ef
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/195777
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-16 22:06:06 +00:00
Joe Tsai
2df47c918b internal/impl: fix race ExtensionInfo.lazyInitSlow
The xi.init flag should not be set until after we have truly
initialized everything.

Change-Id: I43dcb300917145ce19a7199fa871acd0df325c6c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/194439
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-10 20:33:39 +00:00
Joe Tsai
e87cf53d57 internal/impl: fix panic calling XXX_MessageName on aberrant messages
Call XXX_MessageName with best-effort. If it panics, oh-well.

Change-Id: I605ea074470b0c90b0bea8b36fa7d4a69368692d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/194598
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-10 20:25:26 +00:00
Joe Tsai
8ee364e4bf internal/impl: fix SetLazyValue
Since the internal body calls ExtensionType.ValueOf, it seems that the
intent is for lazyExtensionValue.value to store a:
	func() protoreflect.Value
instead of a:
	func() interface{}

This seems more apparent given that GetValue returns a pref.Value.

Change-Id: I1679fe56088c20d5c8d36360e75dd773850da4c2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/193757
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-09 18:58:35 +00:00
Joe Tsai
531a03af35 internal/impl: preserve nil in ProtoMessageVXOf
ProtoMessageV1(nil) now returns nil.
ProtoMessageV2(nil) now returns nil.

Note that the following continue to panic:
	MessageOf(nil)
	MessageDescriptorOf(nil)
	MessageTypeOf(nil)
It may be reasonable for them to also return nil in the future.

Change-Id: Icc14857252d844eb6d4dbfe0c248cef22023e930
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/193758
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-09 18:40:39 +00:00
Joe Tsai
09217f08d2 all: make error messages unstable
Use internal/detrand in the construction of our error messages.
This alters whether there is one or two spaces following the "proto:" prefix.
While it is easy for users to still work around this mutation,
sit at least forces them to write test infrastructure to more fuzzily
match on error strings.

Change-Id: I4ddca717526ee3fc4dbb1e0b36cfca8c6e0df36d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/194038
Reviewed-by: Herbie Ong <herbie@google.com>
2019-09-07 00:39:30 +00:00
Joe Tsai
ea5ada15be internal/impl: support legacy message names
This change:
* Adds aberrant support for the undocumented XXX_MessageName method.
* Adds LegacyMessageTypeOf so that v1 registration can suggest a
fullname to use with a legacy message with no Descriptor support.

Change-Id: I0265bd3cf67f4d4815358148f5817695c1122dc8
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/193518
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-05 17:56:38 +00:00
Damien Neil
2eaed36627 all: remove NewMessage methods
Superseded by New{Field,Element,Value}.

Change-Id: Iad09f3b0c6bac7b75cf28925eef2982d45267317
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/193598
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-09-05 17:49:46 +00:00
Damien Neil
4b3a82f6b1 internal/impl: clean up Value codecs
Remove the Go type from consideration when creating Value codecs, as it
is unnecessary. Value codecs convert between wire form and Values,
while Converters convert between Values and the Go type.

Change-Id: Iaa4bc7db81ad0a29dabd42c2229e6f33a0c91c67
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/193457
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-09-05 16:31:05 +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
Damien Neil
d91c422d95 all: remove use of deprecated NewMessage
Replace NewMessage calls with NewField, NewElement, or NewValue.

Change-Id: I6d2bb4f11f0eb2ba7a52308b1addb111137ad4b9
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/193266
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-09-04 17:55:31 +00:00
Joe Tsai
6663f3a630 internal/impl: add ProtoMessageV1Of and ProtoMessageV2Of
These functions are used by v1 code to switch between
protoV1.Message and protoV2.Message.
Wrappers over them may be exposed in a hypothetical protoadapt package.

Change-Id: Ib5265420e34e9888b540307962c70189625d663c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/193180
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-03 23:27:40 +00:00
Damien Neil
293dc761cb internal/impl: change Go representation of extension lists to []T
Change-Id: Iebcefe0330c8f858c7735f9362abfd87043ee39d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/192458
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-09-03 21:19:03 +00:00
Damien Neil
68b81c3117 internal/impl: store extension values as Values
Change the storage type of ExtensionField from interface{} to
protoreflect.Value.

Replace the codec functions operating on interface{}s with ones
operating on Values.

Values are potentially more efficient, since they can represent
non-pointer types without allocation. This also reduces the number of
types used to represent field values.

Additionally, this change lays groundwork for changing the
user-visible representation of repeated extension fields from
*[]T to []T. The storage type for extension fields must support mutation
(thus *[]T currently); changing the storage type to a Value permits this
without the need to introduce yet another view on field values.

Change-Id: Ida336be14112bb940f655236eb58df21bf312525
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/192218
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-09-03 20:58:28 +00:00
Damien Neil
835b271169 reflect/protoreflect: add ExtensionType IsValid{Interface,Value} methods
Add a way to typecheck a Value or interface{} without converting it to
the other form.  This permits implementations which store field values as
a Value (such as dynamicpb, or (soon) extensions in generated messages)
to validate inputs without an unnecessary conversion.

Fixes golang/protobuf#905

Change-Id: I1b78612b22ae832efbb55f81ae420871729e3a02
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/192457
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-09-03 20:58:16 +00:00
Damien Neil
3cda377ed2 all: rename ExtensionType Descriptor method to TypeDescriptor (2/2)
Remove the ExtensionType Descriptor method.

Change-Id: I89c985c45f2a5abc4e4e3770f9652bc2a444251e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/192141
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-28 19:46:47 +00:00
Damien Neil
79bfdbe45b all: rename ExtensionType Descriptor method to TypeDescriptor (1/2)
Descriptor methods generally return a Descriptor with no Go type
information. ExtensionType's Descriptor is an exception, returning an
ExtensionTypeDescriptor containing both the proto descriptor and a
reference back to the ExtensionType. The pure descriptor is accessed
by xt.Descriptor().Descriptor().

Rename ExtensionType's Descriptor method to TypeDescriptor to make it
clear that it behaves a bit differently.

Change 1/2: Add the TypeDescriptor method and deprecate Descriptor.

Change-Id: I1806095044d35a474d60f94d2a28bdf528f12238
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/192139
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-28 18:34:29 +00:00
Joe Tsai
2e7817f117 compiler/protogen, internal/strs, internal/impl: expose enum Go name derivation
In order to migrate v1 to wrap v2, we need a way to reproduce
the awful enum "names" that v1 used, which was the concatenation of
the proto package with the Go identifier used for the enum.

To support this:
* Move the camel case logic from compiler/protogen to internal/strs
* Add a small stub in internal/impl to expose this functionality

Change-Id: I8ff31daa9ae541e5788dc04d2e89eae1574877e4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/191637
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-26 17:49:17 +00:00
Damien Neil
c5060d2fe6 reflect/protoreflect: add non-allocating Value constructors
Passing a non-pointer type to protoreflect.NewValue causes an
unnecessary allocation in order to store the value in an interface{}.
While this allocation could be avoided by a smarter compiler, no such
compiler exists today.

Add functions for creating new values of a specific type, avoiding the
allocation. (And also adding a small amount of type safety, although
this is unlikely to be important.)

Update the proto and internal/impl packages to use these functions.

Change-Id: Ic733de22ddf19c530189166c853348e1b54b7391
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/191457
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-26 17:48:05 +00:00
Joe Tsai
945a1706d0 all: rely on message_set_extension name mangling in ExtensionDescV1
Too much code inside Google rely on ExtensionDescV1.Name being the
target message name rather than the real name of the extension.
Double down on this assumption consistently across our codebase.

Change-Id: If0b7cd16dd1f2f7acfb2e562dbf1a2b5487162b5
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186980
Reviewed-by: Herbie Ong <herbie@google.com>
2019-08-21 02:47:33 +00:00
Damien Neil
f247a903b9 internal/impl: clean up obsolete ExtensionInfo fields/funcs
This is change 5/5 in a series of commits changing protoV1.ExtensionDesc
to directly implement protoreflect.ExtensionType.

1. [v2] Add protoimpl.ExtensionInfo as an alias for
   protoiface.ExtensionDescV1.

2. [v1] Update references to protoimpl.ExtensionInfo to use
   protoiface.ExtensionInfo.

3. [v2] Create protoimpl.ExtensionInfo (an alias to a new type in
   the impl package) and remove protoiface.ExtensionDescV1.

4. [v1] Remove unneeded explicit conversions between ExtensionDesc and
   ExtensionType (since the former now directly implements the latter).

5. [v2] Remove stub conversion functions.

Change-Id: I1fdfb18c481e72a362a6f9ee0e440f8f909790ca
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189564
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-20 22:03:04 +00:00
Damien Neil
f1e905b042 all: unify protoV1.ExtensionDesc and proto.ExtensionType
Change protoV1.ExtensionDesc to directly implement ExtensionType
rather than delegating to one.

Unify the previous types protoiface.ExtensionDescV1 and
filetype.Extension in impl.ExtensionInfo. The protoV1.ExtensionDesc
type becomes an alias to ExtensionInfo.

This gives us:

  - Just one implementation of ExtensionType.
  - Generated foopb.E_Ext vars are canonical ExtensionTypes.
  - Generated foopb.E_Ext vars are also v1.ExtensionDescs for backwards
    compatibility.
  - Conversion between legacy and modern representations happens
    transparently when lazily initializing an ExtensionInfo.

Overall, a simplification for users of generated code, since they can
mostly ignore the ExtensionDesc/ExtentionType distinction and use the
same value in either the old or new API.

This is change 3/5 in a series of commits changing protoV1.ExtensionDesc
to directly implement protoreflect.ExtensionType.

1. [v2] Add protoimpl.ExtensionInfo as an alias for
   protoiface.ExtensionDescV1.

2. [v1] Update references to protoimpl.ExtensionInfo to use
   protoiface.ExtensionInfo.

3. [v2] Create protoimpl.ExtensionInfo (an alias to a new type in
   the impl package) and remove protoiface.ExtensionDescV1.

4. [v1] Remove unneeded explicit conversions between ExtensionDesc and
   ExtensionType (since the former now directly implements the latter).

5. [v2] Remove stub conversion functions.

Change-Id: I96ee890541ec11b2412e1a72c9d7b96e4d7f66b4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189563
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-20 21:32:57 +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
Damien Neil
16163b4f67 all: drop reflect/prototype package
Remove the remaining uses of the prototype package.

The most significant change is to impl.MessageInfo, which now directly
implements the MessageType interface. This involves two notable changes
to exported fields of MessageInfo:

  - PBType is now Desc.
  - GoType is now GoReflectType. (Name changed to avoid a conflict with
    the GoType method of the MessageType interface.)

Fixes golang/protobuf#911

Change-Id: Ie2aa4766d6887ceaa9cf06b1f109aa6e6e2a208f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189340
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-09 19:06:30 +00:00
Joe Tsai
1799d1111a all: rename tag and flags for legacy support
Rename build tag "proto1_legacy" -> "protolegacy"
to be consistent with the "protoreflect" tag.

Rename flag constant "Proto1Legacy" -> "ProtoLegacy" since
it covers more than simply proto1 legacy features.
For example, it covers alpha-features of proto3 that
were eventually removed from the final proto3 release.

Change-Id: I0f4fcbadd4b5a61c87645e2e5be11d187e59157c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189345
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-08 20:49:00 +00:00
Damien Neil
92f76189a3 all: refactor extensions, add proto.GetExtension etc.
Change protoiface.ExtensionDescV1 to implement protoreflect.ExtensionType.

ExtensionDescV1's Name field conflicts with the Descriptor Name method,
so change the protoreflect.{Message,Enum,Extension}Type types to no
longer implement the corresponding Descriptor interface. This also leads
to a clearer distinction between the two types.

Introduce a protoreflect.ExtensionTypeDescriptor type which bridges
between ExtensionType and ExtensionDescriptor.

Add extension accessor functions to the proto package:
proto.{Has,Clear,Get,Set}Extension. These functions take a
protoreflect.ExtensionType parameter, which allows writing the
same function call using either the old or new API:

  proto.GetExtension(message, somepb.E_ExtensionFoo)

Fixes golang/protobuf#908

Change-Id: Ibc65d12a46666297849114fd3aefbc4a597d9f08
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189199
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-08 18:20:51 +00:00
Damien Neil
d4f0800c42 all: make handling of zero-value composites more consistent
We occasionally need to work with immutable, empty lists, maps, and
messages. Notably, Message.Get on an empty repeated field will return a
"frozen" empty value.

Move handling of these immutable, zero-length composites into Converter,
to unify the behavior of regular and extension fields.

Add a Zero method to Converter, MessageType, and ExtensionType, to
provide a consistent way to get an empty, frozen value of a composite
type. Adding this method to the public {Message,Extension}Type
interfaces does increase our API surface, but lets us (for example)
cleanly represent an empty map as a nil map rather than a non-nil
one wrapped in a frozenMap type.

Drop the frozen{List,Map,Message} types as no longer necessary.
(These types did have support for creating a read-only view of a
non-empty value, but we are not currently using that feature.)

Change-Id: Ia76f149d591da07b40ce75b7404a7ab8a60cb9d8
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189339
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-08 18:20:25 +00:00
Damien Neil
f5274511fe all: add NewField, NewElement, NewValue
Add methods to protoreflect.{Message,List,Map} to constrict values
assignable to a message field, list element, or map value. These
methods return the default value for scalar fields, the zero value for
scalar list elements and map values, and an empty, mutable value for
messages, lists, and maps.

Deprecate the NewMessage methods on these types, which are superseded.

Updates golang/protobuf#879

Change-Id: I0f064f60c89a239330ccea81523f559f14fd2c4f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/188997
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-06 21:54:30 +00:00
Joe Tsai
e815d6a43b all: remove dead code
Change-Id: I1344d6afca9d3348db849c2b5f387ac18b80d2ba
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189021
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-06 21:16:48 +00:00
Joe Tsai
52ec1759a4 internal/filedesc, internal/filetype: rename {Desc,Type}Builder as Builder
Reduce the stutter in the name since the type of Builder
is obvious from the package it is from.

Change-Id: I0046a5122717536cc6bb5ebdb32b67a1560cfc23
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189020
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-05 23:43:37 +00:00
Damien Neil
954bd92c19 all: refactor Converter
A Converter converts between reflect.Values and protoreflect.Values.
The existing usage of Converter is somewhat confusing: The
internal/value package creates Converters for scalar types only, the
internal/impl package creates Converters for legacy messages and enums,
and the reflect/prototype package creates Converters for repeated fields.

Change the Converter type to an interface. The constructor for
Converter takes a FieldDescriptor and reflect.Type, and directly
handles conversions for all field types: Scalars, lists, maps, and
legacy types.

Move Converter into the internal/impl package, since that package
contains the necessary support for dealing with legacy messages and
enums. Drop the internal/value package.

Replace two uses of prototype.Extension with more focused
implementations, since the implementation is trivial with the
refactored Converter. Drop prototype.Extension for the moment since
it is now unused.

Change-Id: If0c570fefac002cc5925b3d56281b6eb17e90d5f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/187857
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-05 22:33:14 +00:00
Damien Neil
8003f08e51 proto, internal/impl: zero-length proto2 bytes fields should be non-nil
Fix decoding of zero-length bytes fields to produce a non-nil []byte.

Change-Id: Ifb7791a47df81091700f7226523371d1386fb1ad
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/188765
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-05 21:44:30 +00:00
Joe Tsai
afd3633ce3 internal/impl: defer evaluation of weak reference until actual use
Suppose some init logic performs protobuf reflection.
If so, it will cause the table-driven logic for protobuf reflection
to be initialized. This is problematic for weak fields since we
can not be certain that all weak references have been registered
at this point in time. This is a fundamental issue with with weak
dependencies, since it means that we cannot enforce any ordering
constraints on the weak dependency unless we directly import the weakly
referenced package (which would defeat the point of weak imports).

Alleviate the problem by pushing evaluation of weak reference to
actual usage. This does not completely fix the problem,
but signifcantly reduces the probability of it being problematic.
In general, people should avoid interacting with weak fields at init time.

Change-Id: Iebaefddde8cf07b5cd7dee49b7015b05b5428618
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/188980
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-05 21:38:46 +00:00
Joe Tsai
2aea614c5e internal/impl: fix race over messageState.mi
The messageState.mi field is atomically checked and set
in generated code to the *MessageInfo associated with that message.
However, the messageState type accesses the mi field without
any atomic loads, thus being a potential race.
We fix this by always calling a messageInfo method that performs
a atomic.LoadPointer on the *MessageInfo.

There is no performance effect from this change on x86 since
an atomic.LoadPointer is identical to a MOV instruction.
From an assembly perspective, there was no memory race previously.
However, the lack of an atomic.LoadPointer meant that the compiler
could in theory reorder the "normal" load to produce truly racy code.

Change-Id: I8afefaf35c1916872781abc0239cbb63d62edf16
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189017
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-05 21:22:56 +00:00
Damien Neil
a6af044c3f internal/impl: fix data race in atomicNilMessage
The problem is that atomicNilMessage.m.mi is accessed both by atomic and
non-atomic operations. (Init uses an atomic read to verify that m.mi is
non-nil, but then returns a non-atomic m.)

Race condition is demonstrated by this test with
"go test -race -count=1000":

	func TestPointer(t *testing.T) {
		var m atomicNilMessage
		var mi MessageInfo
		ch := make(chan *MessageInfo)
		for i := 0; i < 20; i++ {
			go func() {
				r := m.Init(&mi)
				if &mi != r.mi {
					// This conditional exists just
					// ensure r.mi is touched.
					t.Error("mismatch")
				}
				ch <- r.mi
			}()
		}
		for i := 0; i < 20; i++ {
			<-ch
		}
	}

I chose not to add the test since it seems a bit overfit to the specific
situation.

Change-Id: Id4664ef3cd5b29515ed310851b9aeb7561be30d0
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/188337
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-02 21:09:23 +00:00
Joe Tsai
070c1010d9 internal/impl: simplify getMessageInfo
Our specific protoreflect.Message implementations have a special
ProtoMessageInfo method to obtain the *MessageInfo for v1 compatibility.
Use that instead to implement getMessageInfo.

Change-Id: I6cab9aeaa93714be73bd812c3d9a3be0ec86dd52
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/187777
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-31 16:45:42 +00:00