Commit Graph

27 Commits

Author SHA1 Message Date
Joe Tsai
4fddebafc0 all: move v1 types over to the v2 repository
As a goal, v2 should not depend on v1. As another step towards that end,
we move all the types that used to be in the v1 protoapi package over to v2.

For now, we place MessageV1, ExtensionRangeV1, and ExtensionDescV1
in runtime/protoiface since these are types that generated messages will
probably have to reference forever. An alternative location could be
reflect/protoreflect, but it seems unfortunate to have to dirty the
namespace of that package with these types.

We move ExtensionFieldV1, ExtensionFieldsV1, and ExtensionFieldsOf
to internal/impl, since these are related to the implementation of a
generated message.

Since moving these types from v1 to v2 implies that the v1 protoapi
package is useless, we update all usages of v1 protoapi in the v2
repository to point to the relevant v2 type or functionality.

CL/168538 is the corresponding change to alter v1.
There will be a temporary build failure as it is not possible
to submit CL/168519 and CL/168538 atomically.

Change-Id: Ide4025c1b6af5b7f0696f4b65b988b4d10a50f0b
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/168519
Reviewed-by: Herbie Ong <herbie@google.com>
2019-03-22 20:01:07 +00:00
Joe Tsai
559d47f1da cmd/protoc-gen-go: fix init order for v1 registration
The v1 registration leaks the message types out to the proto package.
When doing that, it must ensure that the reflection data structures
for those types are properly initialized first. We achieve that by
doing v1 registration at the end of the reflection init function.

Change-Id: If6df18df59d05bad50ff39c2eff6beb19e7466cc
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/168348
Reviewed-by: Damien Neil <dneil@google.com>
2019-03-20 04:16:33 +00:00
Joe Tsai
8e506a8704 cmd/protoc-gen-go: rely on protoimpl for basic helpers
The EnumName, UnmarshalJSONEnum, and CompressGZIP helpers currently live
in v1 protoapi, which would cause all generated messages to depend on v1.
In an effort to break the dependency of v2 on v1, we move these helper
functions to v2 (and re-written to take advantage of protobuf reflection).

These helpers are unfortunate, but we cannot eliminate the functionality
that they implement since they are exposed in the publicly generated API.

Since EnumName does not rely on the enum maps, it removes another dependency
on those variables. Eventually, we can get to the point where these variables
(though declared) are not linked into the binary if the user does not use them.

Also, we rely on the v1 proto package for registration instead of v1 protoapi.
This may re-introduce a cyclic dependency on descriptor proto again in the
future, but the better approach is to just start registering with v2.

Change-Id: Id755585a7a1df14e4a6a2dfa650df221a3c153fb
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167921
Reviewed-by: Damien Neil <dneil@google.com>
2019-03-18 18:50:16 +00:00
Joe Tsai
afb455eaf8 cmd/protoc-gen-go: correlate v1 ExtensionDesc with v2 ExtensionType
Unfortunately a good amount of code uses pointer comparisons on the
v1 ExtensionDesc to determine exactly which extension field is set,
rather than checking whether the extension descriptor semantically
describes the field that they are interested in.

To preserve this behavior in v1, we need a 1:1 mapping between
a v2 ExtensionType and a specific v1 ExtensionDesc.

Change-Id: I852b3cefb4585bd656e48e5adad6cc28795d02df
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167759
Reviewed-by: Damien Neil <dneil@google.com>
2019-03-15 01:24:42 +00:00
Damien Neil
0fc224513b cmd/protoc-gen-go: enforce init order within packages
Ensure that the init funcs for files within a Go package run in the
dependency order of the source .proto files. That is, if a.proto and b.proto
are part of the same Go package, and a.proto imports b.proto, then b.pb.go's
init funcs must run before a.pb.go's.

Change-Id: I0e86ff22e5c4cab9df7a73fe4805390fadd34b0d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/166419
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Herbie Ong <herbie@google.com>
2019-03-10 22:19:14 +00:00
Damien Neil
6bb8dec7f6 cmd/protoc-gen-go: change some arrays to slices to save bytes
Using arrays in the generated reflection information adds unnecessary
eq and hash functions being added to the package. Change to slices
to reduce bloat.

Change-Id: I1a4f6d59021644d93dd6c24679b9233141e89a75
Reviewed-on: https://go-review.googlesource.com/c/164640
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-03-01 23:53:03 +00:00
Joe Tsai
19058431cd internal/cmd/generate-protos: initial commit
Create a single binary for handling generation of protos.
This replaces previous logic spread throughout the repo in:
* regenerate.bash
* cmd/protoc-gen-go/golden_test.go
* cmd/protoc-gen-go-grpc/golden_test.go
* (indirectly) internal/protogen/goldentest

One of the problems with the former approaches is that they relied on
a version of protoc that was specific to a developer's workstation.
This meant that the result of generation was not hermetic.
To address this, we rely on the hard-coded version of protobuf specified
in the test.bash script.

A summary of changes in this CL are:
* The internal_gengo.GenerateFile and internal_gengogrpc.GenerateFile
functions are unified to have consistent signatures. It seems that the
former accepted a *protogen.GeneratedFile to support v1 where gRPC code
was generated into the same file as the base .pb.go file. However, the
same functionality can be achieved by having the function return
the generated file object.
* The test.bash script patches the protobuf toolchain to have properly
specified go_package options in each proto source file.
* The test.bash script accepts a "-regenerate" argument.
* Add generation for the well-known types. Contrary to how these were
laid out in the v1 repo, all the well-known types are placed in the
same Go package.
* Add generation for the conformance proto.
* Remove regenerate.bash
* Remove internal/protogen
* Remove cmd/protoc-gen-go/golden_test.go
* Remove cmd/protoc-gen-go-grpc/golden_test.go
* Add cmd/protoc-gen-go/annotation_test.go

Change-Id: I4a1a97ae6f66e2fabcf4e4d292c95ab2a2db0248
Reviewed-on: https://go-review.googlesource.com/c/164477
Reviewed-by: Damien Neil <dneil@google.com>
2019-03-01 20:47:52 +00:00
Joe Tsai
4069211bcd protogen: use full path for generated file variable name
Use the full path (including the extension) for the generation of
the per-file variable name. Several reasons for this:

* The current logic is buggy in the case where pathType == pathTypeImport
since the prefix variable will be mangled with the Go import path.

* The extension is technically part of the path.
Thus, "path/to/foo.proto" and "path/to/foo.protodevel" are two
distinctly different imports.

* Style-wise, it subjectively looks better. Rather than being a mixture
of camelCase and snake_case, it is all snake_case for the common case:
	before: ProtoFile_google_protobuf_any
	after:  File_google_protobuf_any_proto

* Since the extension is almost always ".proto", this results in a
suffix of "_proto", which provides an additional layer of protection
against possible name conflicts. The previous approach could possibly
have a conflict between "Foo.proto" and a message named ProtoFile
with a sub-message called Foo.

Also, use the per-file variable name for the raw descriptor variables
instead of the hashed version.

Change-Id: Ic91e326b7593e5985cee6ececc60539c27fe32fe
Reviewed-on: https://go-review.googlesource.com/c/164379
Reviewed-by: Damien Neil <dneil@google.com>
2019-03-01 00:13:31 +00:00
Joe Tsai
cf81e67b53 cmd/protoc-gen-go: use protoapi.CompressGZIP
Calling a helper function directly should reduce binary bloat slightly.

Change-Id: I6068dc4cd00c8d90d2e6e6d99633b81388bc8781
Reviewed-on: https://go-review.googlesource.com/c/164679
Reviewed-by: Damien Neil <dneil@google.com>
2019-02-28 22:28:32 +00:00
Damien Neil
71c6603a26 protogen: use full filename in per-file vars
For a file "foo/bar.proto", put the FileDescriptor in "ProtoFile_foo_bar"
rather than "Bar_fileDescriptor".

Avoid name clashes when a package contains "a/foo.proto" and "b/foo.proto".

Don't camelcase the filename: These vars weren't fully camelcased to begin
with, and leaving the filename relatively unchanged is clearer and more
predictable.

Move "ProtoFile" from the end of the var name to the start, so that vars
will sort better in packages with multiple descriptors.

These changes do add a chance of name collision when the input filename
begins with an uppercase letter: Foo.proto becomes "ProtoFile_Foo", which
could be the result of camelcasing "proto_file.foo". The readability
benefits seem worth it.

Change-Id: If27d3a0d7b5bf3535aa1607a8579eb057c74d2dc
Reviewed-on: https://go-review.googlesource.com/c/163199
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Herbie Ong <herbie@google.com>
2019-02-21 22:37:58 +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
5681bb2587 protogen: use _protoFile suffix for file descriptor variable
A "_ProtoFile" suffix can potentially conflict with a sub-message named
"ProtoFile" nested within a message that matches the camel-cased
form of the basename of the .proto source file.

Avoid unlikely conflicts and rename this to use a "_protoFile" suffix,
which can never conflict except with an enum value that is also named
"protoFile" (which is a violation of the style guide).

Change-Id: Ie9d22f9f741a63021b8f76906b20c6c2f599885b
Reviewed-on: https://go-review.googlesource.com/c/157218
Reviewed-by: Damien Neil <dneil@google.com>
2019-01-14 20:23:59 +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
Damien Neil
97e7f57dbb reflect/protoreflect: replace Mutable with NewMessage
Remove the Mutable methods from KnownFields, List, and Map, replacing
them with methods which return a new, empty message value without adding
that value to the collection.

The new API is simpler, since it clearly applies only to message values,
and more orthogonal, since it provides a way to create a value without
mutating the collection. This latter point is particularly useful in
map deserialization, where the key may be unknown at the time the value
is deserialized.

Drop the Mutable interface, since it is no longer necessary.

Change-Id: Ic5f3d06a2aa331a5d5cd2b4e670a3dba4a74f77c
Reviewed-on: https://go-review.googlesource.com/c/153278
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2018-12-10 21:17:16 +00:00
Joe Tsai
9667c4816d cmd/protoc-gen-go: reduce technical debt
The following TODOs were addressed:
* Consistently collect all enums, messages, and extensions in a breadth-first order.
The practical affect of this is that the declaration order in a Go file may change.
This simplifies reflection generation, which relies on consistent ordering.
* Removal of placeholder declarations (e.g., "var _ = proto.Marshal") since
protogen is intelligent about including imports as necessary.
* Always generate a default variable or constant for explicit empty strings.
The practical effect of this is the addition of new declarations in some cases.
However, it simplifies our logic such that it matches the protobuf data model.
* Generate the registration statements in a consistent order.

Change-Id: I627bb72589432bb65d53b50965ea88e5f7983977
Reviewed-on: https://go-review.googlesource.com/c/152778
Reviewed-by: Damien Neil <dneil@google.com>
2018-12-07 03:13:48 +00:00
Joe Tsai
b6405bd782 cmd/protoc-gen-go: add support for protobuf reflection
Implement support in protoc-gen-go for generating messages and enums
that satisfy the v2 protobuf reflection interfaces. Specifically, the following
are added:
* top-level variable representing the file descriptor
* ProtoReflect method on enums (to implement protoV2.Enum)
* ProtoReflect method on messages (to implement protoV2.Message)

The following are not supported yet:
* resolving transitive dependencies for file imports
* Extension descriptors
* Service descriptors

The implementation approach creates a single array for all the message and enum
declarations and references sections of that array to complete dependencies.
Since protobuf declarations can form a graph (a message may depend on itself),
it is difficult to construct a graph as a single literal. One way is to use
placeholder descriptors, but that is not efficient as it requires encoding
the full name of each dependent enum and message and then later resolving it;
thus, both expanding the binary size and also increasing initialization cost.
Instead, we add a prototype.{Enum,Message}.Reference method to obtain a
descriptor reference for the purposes for satisfying dependencies.
As such, nested declarations and dependencies are populated in an init function.

Other changes to support the implementation:
* Added a protoimpl package to expose the MessageType type and also the
MessageTypeOf and EnumTypeOf helper functions.
* Added a protogen.File.GoIdent field to provide a suggested variable name
for the file descriptor.
* Added prototype.{Enum,Message}.Reference that provides a descriptor reference
for the purposes for satisfying cyclic dependencies.
* Added protoreflect.{Syntax,Cardinality,Kind}.GoString to obtain a Go source
identifier that represents the given constant.

Change-Id: I9455764882dee6ad10f251901e7d419091e8bf1d
Reviewed-on: https://go-review.googlesource.com/c/150074
Reviewed-by: Damien Neil <dneil@google.com>
2018-12-05 19:32:12 +00:00
Joe Tsai
f757390b52 cmd/protoc-gen-go: remove generation of RegisterMessageSetType
PR#741 in the v1 repository deprecated this behavior.

Change-Id: Ife48f1d586f178d875b9b3002a88b3336a2cd3b4
Reviewed-on: https://go-review.googlesource.com/c/151401
Reviewed-by: Damien Neil <dneil@google.com>
2018-11-28 00:40:08 +00:00
Joe Tsai
b6dbf23033 cmd/protoc-gen-go: remove generation of {Unmarshal,Marshal}JSON for MessageSets
PR#741 in the v1 repository deprecated this behavior.

Change-Id: Idffa3884d43f9cc5528ce8d9f303676e0e501b67
Reviewed-on: https://go-review.googlesource.com/c/151400
Reviewed-by: Damien Neil <dneil@google.com>
2018-11-27 23:47:23 +00:00
Joe Tsai
d7e97bc71b cmd/protoc-gen-go: generate XXX_OneofWrappers instead of XXX_OneofFuncs
The marshaler, unmarshaler, and sizer functions are unused ever since
the underlying implementation was switched to be table-driven.
Change the function to only return the wrapper structs.

This change:
* enables generated protos to drop dependencies on certain proto types
* reduces the size of generated protos
* simplifies the implementation of oneofs in protoc-gen-go

Updates #708

Change-Id: I845c9009bc0236d1b51d34b014dc3e184303c0f2
Reviewed-on: https://go-review.googlesource.com/c/151357
Reviewed-by: Damien Neil <dneil@google.com>
2018-11-27 19:36:27 +00:00
Damien Neil
6238696abc cmd/protoc-gen-go: replicate v1 generator behavior for MessageSet extensions
Given:

  package foo
  extend proto2.bridge.MessageSet {
    optional Message message_set_extension = 100;
  }

Register the extension as a message set extension and give it the name
"foo.".

We really shouldn't do this in this case; the special-case treatment of
extensions to MessageSet is only for extensions nested in a parent
message. However, this is consistent with the behavior of the v1 generator.
Match that for now.

Change-Id: I919c409605a197904fd3227efc920192d484f431
Reviewed-on: https://go-review.googlesource.com/c/145957
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2018-10-30 18:46:20 +00:00
Damien Neil
44000a1bea cmd/protoc-gen-go: treat extensions fields as proto2 in some places
The v1 generator doesn't include a "proto3" tag on extension fields,
even when the field is defined in a proto3 file. Match that behavior for
consistency. We can probably change this later if we want to; it's
unlikely anyone is depending on this behavior.

The v1 generator uses pointer types for extension fields, even when the
field is defined in a proto3 file. (e.g., *FooEnum instead of FooEnum.)
Match this behavior. We can't change this without breaking compatibility
in the generated code.

Change-Id: I4072f3dd1c915bf9ab89f1d5198e0144cb4de20f
Reviewed-on: https://go-review.googlesource.com/c/144282
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2018-10-24 19:46:45 +00:00
Damien Neil
154da98aa5 cmd/protoc-gen-go: special cases for MessageSet extensions
Add special-case handling for extension fields named
"message_set_extension" that extend a message_set_wire_format message.

Support special cases for a proto1 feature that was superseded by proto2
extensions.

Change-Id: Icbdb711111c66be547bf8d6f37ab3079c320e2a1
Reviewed-on: https://go-review.googlesource.com/136536
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2018-09-24 17:25:31 +00:00
Joe Tsai
01ab29648e go.mod: rename google.golang.org/proto as github.com/golang/protobuf/v2
This change was created by running:
	git ls-files | xargs sed -i "s|google.golang.org/proto|github.com/golang/protobuf/v2|g"

This change is *not* an endorsement of "github.com/golang/protobuf/v2" as the
final import path when the v2 API is eventually released as stable.
We continue to reserve the right to make breaking changes as we see fit.

This change enables us to host the v2 API on a repository that is go-gettable
(since go.googlesource.com is not a known host by the "go get" tool;
and google.golang.org/proto was just a stub URL that is not currently served).
Thus, we can start work on a forked version of the v1 API that explores
what it would take to implement v1 in terms of v2 in a backwards compatible way.

Change-Id: Ia3ebc41ac4238af62ee140200d3158b53ac9ec48
Reviewed-on: https://go-review.googlesource.com/136736
Reviewed-by: Damien Neil <dneil@google.com>
2018-09-24 16:11:50 +00:00
Damien Neil
a6c374a8f1 cmd/protoc-gen-go: add test data for proto3 extensions
Change-Id: I2d47db2c177553dc2a3dacc66c70db579c1145fd
Reviewed-on: https://go-review.googlesource.com/136535
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2018-09-20 19:56:44 +00:00
Damien Neil
993c04dcf2 cmd/protoc-gen-go: generate extensions
Generate everything related to extensions: extension descriptors,
XXX_InternalExtensions fields, etc.

Tweak the order in which we generate code for descriptors to ensure
consistent output with the previous protoc-gen-go.

Change Field.ContainingType to Field.ParentMessage, since we need to get
at the parent of both message fields and extensions (and the "containing
type" of an extension field is the extended message, under existing
terminology).

Change-Id: I5d045ca80536436e7c987bca3d8fb8c1e1521e55
Reviewed-on: https://go-review.googlesource.com/136155
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2018-09-18 22:49:53 +00:00