12 Commits

Author SHA1 Message Date
Damien Neil
fb8bd3a65a internal/legacy, internal/encoding/tag: don't synthesize options
Stop adding options to legacy MessageDescriptors and FieldDescriptors.
We would synthesize options messages containing semantic options
(FieldOptions.is_packed, FieldOptions.is_weak, MessageOptions.map_entry).
This information is already contained in the protoreflect descriptors,
so there is no real need to define a correct options message.

If we do want to include options in legacy descriptors, then we should
get the original value out of the FileDescriptorProto, since it may
include additional extensions or other information.

This change completely removes the dependency from internal/legacy to
descriptor.proto.

Change-Id: Ib6bbe4ca6e0fe7ae501f3e9b11d5fa0222808410
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/171458
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-04-10 21:29:14 +00:00
Damien Neil
987d570c08 internal/legacy: remove dependency on descriptor.proto
We jump through many hoops to avoid generated protos depending on
internal/legacy. Break the cycle in the other direction: Remove
the dependency on descriptor.proto from internal/legacy by
using a hand-written parser for the few descriptor fields we need.

Still to do: Remove the descriptor.proto dependency from
internal/encoding/tag.

Change-Id: I5fd99a2170470ba8530eb2679b6dde899821bf3e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/171457
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-04-10 19:29:59 +00:00
Joe Tsai
35ec98fdcb cmd/protoc-gen-go: generate for v2-only dependencies
This removes yet another set of dependencies of v2 on v1.
The only remaining dependency are in the _test.go files,
primarily for proto.Equal.

Changes made:
* cmd/protoc-gen-go no longer generates any functionality that depends
on the v1 package, and instead only depends on v2.
* internal/fileinit.FileBuilder.MessageOutputTypes is switched from
protoreflect.MessageType to protoimpl.MessageType since the
implementation must be fully inialized before registration occurs.
* The test for internal/legacy/file_test.go is switched to a legacy_test
package to avoid a cyclic dependency.
This requires Load{Enum,Message,File}Desc to be exported.

Change-Id: I43e2fe64cff4eea204258ce11e791aca5eb6e569
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/169298
Reviewed-by: Damien Neil <dneil@google.com>
2019-03-26 17:03:31 +00:00
Joe Tsai
b936504483 internal/legacy: guarantee one-to-one generation of wrapper types
When concurrent requests to wrap a type occurs, it is possible that
two different descriptors are created for the same Go type.
Add sufficient synchronization to ensure that one descriptor is ever
returned for one Go type.

Change-Id: Idbbca4c1877a70317b39900ae83bfc3085d4a9c5
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/168398
Reviewed-by: Herbie Ong <herbie@google.com>
2019-03-19 23:48:09 +00:00
Joe Tsai
990b9f5919 internal/prototype: move from reflect/prototype
The prototype package was initially used by generated reflection support,
but has now been replaced by internal/fileinit.
Eventually, this functionality should be deleted and re-written in terms
of other components in the repo.

Usages that prototype currently provides (but should be moved) are:
* Constructing standalone messages and enums, which is behavior we should
provide in reflect/protodesc. The google.protobuf.{Enum,Type} are well-known
proto messages designed for this purpose.
* Constructing placeholder files, enums, and messages.
* Consructing protoreflect.{Message,Enum,Extension}Types, which are protobuf
descriptors with associated Go type information.

Change-Id: Id7dbefff952682781b439aa555508c59b2629f9e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167383
Reviewed-by: Damien Neil <dneil@google.com>
2019-03-13 20:17:00 +00:00
Damien Neil
8012b444ee internal/fileinit: generate reflect data structures from raw descriptors
This CL takes a significantly different approach to generating support
for protobuf reflection. The previous approach involved generating a
large number of Go literals to represent the reflection information.
While that approach was correct, it resulted in too much binary bloat.

The approach taken here initializes the reflection information from
the raw descriptor proto, which is a relatively dense representation
of the protobuf reflection information. In order to keep initialization
cost low, several measures were taken:
* At program init, the bare minimum is parsed in order to initialize
naming information for enums, messages, extensions, and services declared
in the file. This is done because those top-level declarations are often
relevant for registration.
* Only upon first are most of the other data structures for protobuf
reflection actually initialized.
* Instead of using proto.Unmarshal, a hand-written unmarshaler is used.
This allows us to avoid a dependendency on the descriptor proto and also
because the API for the descriptor proto is fundamentally non-performant
since it requires an allocation for every primitive field.

At a high-level, the new implementation lives in internal/fileinit.

Several changes were made to other parts of the repository:
* cmd/protoc-gen-go:
  * Stop compressing the raw descriptors. While compression does reduce
the size of the descriptors by approximately 2x, it is a pre-mature
optimization since the descriptors themselves are around 1% of the total
binary bloat that is due to generated protobufs.
  * Seeding protobuf reflection from the raw descriptor significantly
simplifies the generator implementation since it is no longer responsible
for constructing a tree of Go literals to represent the same information.
  * We remove the generation of the shadow types and instead call
protoimpl.MessageType.MessageOf. Unfortunately, this incurs an allocation
for every call to ProtoReflect since we need to allocate a tuple that wraps
a pointer to the message value, and a pointer to message type.
* internal/impl:
  * We add a MessageType.GoType field and make it required that it is
set prior to first use. This is done so that we can avoid calling
MessageType.init except for when it is actually needed. The allows code
to call (*FooMessage)(nil).ProtoReflect().Type() without fearing that the
init code will run, possibly triggering a recursive deadlock (where the
init code depends on getting the Type of some dependency which may be
declared within the same file).
* internal/cmd/generate-types:
  * The code to generate reflect/prototype/protofile_list_gen.go was copied
and altered to generated internal/fileinit.desc_list_gen.go.

At a high-level this CL adds significant technical complexity.
However, this is offset by several possible future changes:
* The prototype package can be drastically simplified. We can probably
reimplement internal/legacy to use internal/fileinit instead, allowing us
to drop another dependency on the prototype package. As a result, we can
probably delete most of the constructor types in that package.
* With the prototype package significantly pruned, and the fact that generated
code no longer depend on depends on that package, we can consider merging
what's left of prototype into protodesc.

Change-Id: I6090f023f2e1b6afaf62bd3ae883566242e30715
Reviewed-on: https://go-review.googlesource.com/c/158539
Reviewed-by: Herbie Ong <herbie@google.com>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-01-30 01:33:46 +00:00
Joe Tsai
3bc7d6f5cd reflect: switch MessageType.New to return Message
Most usages of New actually prefer to interact with the reflective view
rather than the native Go type. Thus, change New to return that instead.
This parallels reflect.New, which returns the reflective view
(i.e., reflect.Value) instead of native type (i.e., interface{}).
We make the equivalent change to KnownFields.NewMessage, List.NewMessage,
and Map.NewMessage for consistency.

Since this is a subtle change where the type system will not always
catch the changed type, this change was made by both changing the type
and renaming the function to NewXXX and manually looking at every usage
of the the function to ensure that the usage correctly operates
on either the native Go type or the reflective view of the type.
After the entire codebase was cleaned up, a rename was performed to convert
NewXXX back to New.

Change-Id: I153fef627b4bf0a427e4039ce0aaec52e20c7950
Reviewed-on: https://go-review.googlesource.com/c/157077
Reviewed-by: Damien Neil <dneil@google.com>
2019-01-09 20:29:29 +00:00
Damien Neil
a8593bae57 reflect/protoreflect: drop the ProtoEnum type
Drop the protoreflect.ProtoEnum type (containing a single method
returning a protoreflect.Enum) and make generated enum types
directly implement protoreflect.Enum instead.

Messages have a two-level type split (ProtoMessage and Message) to
minimize conflicts between reflection methods and field names. Enums
need no such split, since enums do not have fields and therefore have
no source of conflicts.

Change-Id: I2b6222e9404253e6bfef2217859e1b760ffcd29b
Reviewed-on: https://go-review.googlesource.com/c/156902
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Damien Neil <dneil@google.com>
2019-01-09 00:40:35 +00:00
Damien Neil
232ea15589 reflect/prototype: hoist semantic options into builders
Add fields to the Message and Field builder structs which hold the value
of MessageOptions.map_entry, FieldOptions.packed, and FieldOptions.weak
options. Remove all access to the contents of options messages from the
prototype package.

Change IsPacked to always return false for unpackable field types,
which is consistent with the equivalent C++ API.

This change helps avoid dependency cycles between prototype and the
options messages. (Previously this was resolved by accessing options
with reflection, but just breaking the dependency from prototype to the
options message is cleaner and simpler.)

Change-Id: I756aefe2e04cfa8fea31eaaaa0b5a99d4ac9e851
Reviewed-on: https://go-review.googlesource.com/c/153517
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2018-12-11 20:25:45 +00:00
Joe Tsai
e1f8d50e17 reflect/protodesc: split descriptor related functionality from prototype
In order to generate descriptor.proto, the generated code would want to depend
on the prototype package to construct the reflection data structures.
However, this is a problem since descriptor itself is one of the dependencies
for prototype. To break this dependency, we do the following:
* Avoid using concrete *descriptorpb.XOptions messages in the public API, and
instead just use protoreflect.ProtoMessage. We do lose some type safety here
as a result.
* Use protobuf reflection to interpret the Options message.
* Split out NewFileFromDescriptorProto into a separate protodesc package since
constructing protobuf reflection from the descriptor proto obviously depends
on the descriptor protos themselves.

As part of this CL, we check in a pre-generated version of descriptor and plugin
that supports protobuf reflection natively and switchover all usages of those
protos to the new definitions. These files were generated by protoc-gen-go
from CL/150074, but hand-modified to remove dependencies on the v1 proto runtime.

Change-Id: I81e03c42eeab480b03764e2fcbe1aae0e058fc57
Reviewed-on: https://go-review.googlesource.com/c/152020
Reviewed-by: Damien Neil <dneil@google.com>
2018-12-05 00:38:30 +00:00
Joe Tsai
6dbffb765a internal/legacy: drop legacy prefix
The legacy prefix made sense when this functionality was part of impl.
Now that it is in its own package called legacy, the legacy prefix is silly.

Change-Id: I9e6ddb6185ce1f701e02768b505e6a05f3986f77
Reviewed-on: https://go-review.googlesource.com/c/152543
Reviewed-by: Herbie Ong <herbie@google.com>
2018-12-05 00:24:04 +00:00
Joe Tsai
08e0030032 internal/legacy: extract legacy support out from the impl package
The impl package currently supports wrapping legacy v1 enums and messages
so that they implement the v2 reflective APIs. This functionality is necessary
for v1 and v2 to interoperate. However, the existence of this functionality
presents several problems:
	* A significant portion of the complexity in impl is for legacy wrapping.
	* This complexity is linked into a Go binary even if all the other messages
	in the binary natively support v2 reflection.
	* It presents a cyclic dependency when trying to generate descriptor proto.

Suppose you are generating descriptor.proto. The generated code would want to
depend on the impl package because impl is the runtime implementation for
protobuf messages. However, impl currently depends depends on descriptor in
order to wrap legacy enum and messages since it needs the ability to dynamically
create new protobuf descriptor types. In the case of descriptor.proto, it would
presumably be generated with native reflection support, so the legacy wrapping
logic is unneccessary.

To break the dependency of impl on descriptor, we move the legacy support logic
to a different package and instead add hooks in impl so that legacy support could
be dynamically registered at runtime. This is dependency injection.

Change-Id: I01a582908ed5629993f6699e9bf2f4bee93857a4
Reviewed-on: https://go-review.googlesource.com/c/151877
Reviewed-by: Herbie Ong <herbie@google.com>
2018-11-30 23:16:16 +00:00