Commit Graph

269 Commits

Author SHA1 Message Date
Joe Tsai
dd271b6b63 internal/conformance: make conformance test use specific go version
The use of internal/cmd/conformance/conformance.sh means that the test
is not hermetic since the script uses the system version of go,
rather than the specific versions chosen by integration_test.go.

Change the way the conformance test is invoked by turning it
into a Go unit test that integration_test.go can directly call.

Change-Id: I37d23341e1eda984f23f78757a38e862e5fac3d4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/190518
Reviewed-by: Herbie Ong <herbie@google.com>
2019-08-16 22:09:14 +00:00
Joe Tsai
abd06a8b30 all: update to protobuf-v3.9.1 and go-v1.11.13 and go-v1.12.9
A conformance test has been added for the text format.
Update our conformance runner to handle that format and
update with the list of current failures.

Change-Id: I36a271fcfbe328c4ee8c870caff4661659ad27ef
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/190517
Reviewed-by: Herbie Ong <herbie@google.com>
2019-08-16 18:12:10 +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
7328839f81 cmd/protoc-gen-go: annotate depIdxs list with index comments
Generate the current index into depIdxs for easier human debugging.

Change-Id: Ida42aa95137b2044a4dc267c31cebec5023bdfb1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/190278
Reviewed-by: Herbie Ong <herbie@google.com>
2019-08-15 22:47:44 +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
Joe Tsai
17581daabb cmd/protoc-gen-go: fix struct tag formatting
If a string default value contains a backtick, it breaks the
struct tag formatting logic which assumes such characters never exist.
Fix this by adding a structTags type, whose String method knows how
to properly escape the value.

Change-Id: I96fe5a6630387eac89eef429da5f917125570e4c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189557
Reviewed-by: Herbie Ong <herbie@google.com>
2019-08-15 22:18:29 +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
Damien Neil
4401a0de4b cmd/protoc-gen-go, internal/filetype: clean up EnumType construction
Drop the dependency from generated files on prototype.Enum: Generated
code should only depend on runtime/proto{iface,impl}.

Drop the Enums, Messages, and Extensions returns form
filetype.Builder.Build. Of these, only Enums was used by generated code.
Change the generated init function to pass the builder a slice of values
to fill in (as is done for messages and extensions).

Remove the filetype dependency on prototype in preparation for
eventually dropping the prototype package entirely.

Change-Id: I28a3420f5dfcc13fed531a64ef07b9afddfd9d55
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189200
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-09 19:04:41 +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
Joe Tsai
bab3d4084e runtime/protoimpl, cmd/protoc-gen-go: support release versioning
In order for protoc-gen-go to output the current version,
it needs to know what version it is currently running as.
However, we cannot rely on the git tags since the tags are not
made until *after* the commit has been submitted.
Instead, we manually encode the version into the code and
make sure that git tags match up with the version in the code.

The version.go file in runtime/protoimpl contains instructions
for how to make a release. Essentially:
* Every non-release commit has a version string with "devel" in it.
* Every release commit must not have "devel" in it and must be unique.
* The "release process" involves submitting two CLs.
The first CL creates a version string without "devel",
which is the commit that a git tag will actually reference.
The second CL follows immediately and re-introduces "devel"
into the version string.

The following example shows a possible sequence of VersionStrings
for git commits in time-ascending order:
	v1.19.0-devel      (this CL)
	v1.19.0-devel
	v1.19.0-devel
	v1.19.0-devel
	v1.20.0-rc.1       <- tagged
	v1.20.0-rc.1.devel
	v1.20.0-rc.1.devel
	v1.20.0-rc.1.devel
	v1.20.0-rc.2       <- tagged
	v1.20.0-rc.2.devel
	v1.20.0            <- tagged (future public release)
	v1.20.0-devel
	v1.20.0-devel
	v1.20.0-devel
	v1.20.0-devel
	v1.20.1            <- tagged
	v1.20.1-devel
	v1.20.1-devel
	v1.21.0            <- tagged
	v1.21.0-devel

Note that we start today with v1.19.0-devel, which means that our initial
release will be v1.20.0. This number was intentionally chosen since
1) the number 20 has some correlation to the fact that we keep calling
the new implementation the "v2" implementation, and
2) the set of tagged versions for github.com/golang/protobuf
and google.golang.org/protobuf are unlikely to ever overlap.
This way, the version of protoc-gen-go is never ambiguous which module
it was built from.

Now that we have version information, we add support for generating .pb.go
files with the version information recorded. However, we do not emit
these for .pb.go files in our own repository since they are always guaranteed
to be at the right version (enforced by integration_test.go).

Updates golang/protobuf#524

Change-Id: I25495a45042c2aa39a39cb7e7738ae8e831a9d26
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186117
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-07 22:59:30 +00:00
Joe Tsai
8d5e6d6927 cmd/protoc-gen-go: improve generation of comments
The following improvements were made:
* All standalone comments above the "syntax" marker are preserved
similar to Java and some other generators.
* All standalone comments above the "package" marker are preserved
to be consistent with our former behavior.
* Leading comments are now generated for enums and extension fields.
* Single-line trailing comments are now generated for
enum values, message fields, and extension fields.
* The leading comments for each field that is part of a oneof are now
generated with the wrapper types rather than being shoved into the
comment for the oneof itself in an unreadable way.
* The deprecation marker is always generated as being above the declaration
rather than sometimes being an inlined comment.
* The deprecation marker is now properly generated for weak field setters.

Updates golang/protobuf#666

Change-Id: I7fd832dd4f86d15bfff70d7c22c6ba4934c05fcf
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189238
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-07 17:33:08 +00:00
Joe Tsai
4a7d633604 cmd/protoc-gen-go: group extension variable declarations
For better readability in godoc, group extension fields by the
target message that they are extending.

Change-Id: Icc0a247b37639e3dbf7a107810923b8ca8294724
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189257
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-07 04:26:34 +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
9b8a433283 cmd/protoc-gen-go: group enum map vars in blocks
Since the enum maps are here to stay, group the declarations together
in a var block for better readability in godoc.

Change-Id: I9a313266539e9a60781f98b80a5293379f82607b
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189077
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-06 21:33:36 +00:00
Joe Tsai
d29a71bff0 cmd/protoc-gen-go: group default consts and vars in blocks
Group the default constant and variable declarations together in a block
for better readability in godoc.

Change-Id: I6b62f5374f0302d0f7cb224cbe34102359c8c51d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189057
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-06 21:23:53 +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
f98dd98c15 internal/filedesc: defer evaluation of weak reference until actual use
Suppose some init logic performs protobuf reflection.
If so, it will cause the lazy descriptor init logic to be executed.
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: Ie5957ddedd61333e72ee9a1bba0c53dace65547c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/188982
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-05 21:31:06 +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
Joe Tsai
38b6196f13 cmd/protoc-gen-go: group generation of internal fields together
These were originally kept separate to assist Google-internal patches,
but it turns out that Google-internal patches do not use the
genMessageInternalFields function.

Change-Id: Idfa962b943d3bede9982b5b0875ba90c86c6d181
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/188979
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-05 20:58:04 +00:00
Damien Neil
4e173a5945 internal/filedesc: don't close over descopts values
It is possible for filedesc to construct a lazy options decoder before
the descriptor package has been imported. For example, top-level enum
values are eagerly decoded, so a generated proto package can construct a
lazy options decoder for an enum value at init time.

Don't close over the variables in descopts. Instead, close over a pointer
to the variable.

Panic with an informative message in the case where options are decoded
before the descriptor package has been initialized.

Change-Id: I277a57602b083cb7bbf92c8114c50b467e59521f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/188820
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-02 21:10:38 +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
Damien Neil
45f14b4bdc internal/filedesc: fix parsing of enforce_utf8 option
Remove spurious negation.

Change-Id: I484fa6fecda85943cdedd96a6c6f0f2349f6bfee
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/188338
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-07-31 19:45:36 +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
Herbie Ong
a3369c5dc2 internal/encoding/text: replace use of regular expression in decoding
Improve performance by replacing use of regular expressions with direct
parsing code.

Compared to latest version:

name                                     old time/op    new time/op    delta
Text/Unmarshal/google_message1_proto2-4    21.8µs ± 5%    14.0µs ± 9%  -35.69%  (p=0.000 n=10+9)
Text/Unmarshal/google_message1_proto3-4    19.6µs ± 4%    13.8µs ±10%  -29.47%  (p=0.000 n=10+10)
Text/Unmarshal/google_message2-4           13.4ms ± 4%     4.9ms ± 4%  -63.44%  (p=0.000 n=10+10)
Text/Marshal/google_message1_proto2-4      13.8µs ± 2%    14.1µs ± 4%   +2.42%  (p=0.011 n=9+10)
Text/Marshal/google_message1_proto3-4      11.6µs ± 2%    11.8µs ± 8%     ~     (p=0.573 n=8+10)
Text/Marshal/google_message2-4             8.01ms ±48%    5.97ms ± 5%  -25.44%  (p=0.000 n=10+10)

name                                     old alloc/op   new alloc/op   delta
Text/Unmarshal/google_message1_proto2-4    13.0kB ± 0%    12.6kB ± 0%   -3.40%  (p=0.000 n=10+10)
Text/Unmarshal/google_message1_proto3-4    13.0kB ± 0%    12.5kB ± 0%   -3.50%  (p=0.000 n=10+10)
Text/Unmarshal/google_message2-4           5.67MB ± 0%    5.50MB ± 0%   -3.13%  (p=0.000 n=10+10)
Text/Marshal/google_message1_proto2-4      12.0kB ± 0%    12.1kB ± 0%   +0.02%  (p=0.000 n=10+10)
Text/Marshal/google_message1_proto3-4      11.7kB ± 0%    11.7kB ± 0%   +0.01%  (p=0.000 n=10+10)
Text/Marshal/google_message2-4             5.68MB ± 0%    5.68MB ± 0%   +0.01%  (p=0.000 n=10+10)

name                                     old allocs/op  new allocs/op  delta
Text/Unmarshal/google_message1_proto2-4       142 ± 0%       142 ± 0%     ~     (all equal)
Text/Unmarshal/google_message1_proto3-4       156 ± 0%       156 ± 0%     ~     (all equal)
Text/Unmarshal/google_message2-4            70.1k ± 0%     65.4k ± 0%   -6.76%  (p=0.000 n=10+10)
Text/Marshal/google_message1_proto2-4        91.0 ± 0%      91.0 ± 0%     ~     (all equal)
Text/Marshal/google_message1_proto3-4        80.0 ± 0%      80.0 ± 0%     ~     (all equal)
Text/Marshal/google_message2-4              36.4k ± 0%     36.4k ± 0%     ~     (all equal)

Change-Id: Ia5d3c16e9e33961aae03bac0d53fcfc5b1943d2a
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/173360
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-07-23 22:08:16 +00:00
Joe Tsai
57b4ff63e0 internal/testprotos: remove stale generated file
Change-Id: I7b7f28b265f1bffc77c38159056a46fa37b04127
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186979
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-07-20 08:41:30 +00:00
Damien Neil
9429392195 internal/encoding/pack: fix tests on armv7a
Golden test output doesn't match when math.NaN() has different bits from
the test's NaNs. Drop the NaN-related tests as too fiddly to be worth
keeping.

Change-Id: I89cf961273c2afab3b6b9f6c63878816314e9f43
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186639
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-07-19 15:13:41 +00:00
Joe Tsai
fb3ff727f0 cmd/protoc-gen-go: refactor logic to be more compartmentalized
This CL makes no feature changes except to move code around.
The only change to the actual generated code is the placement of
the default constants and variables. They move because the new logic
generates all methods together, while previously the constants
were interspersed in-between.

Change-Id: I45932d5aeec5ba45180fb255ea17898beb6c3bd2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186878
Reviewed-by: Herbie Ong <herbie@google.com>
2019-07-19 08:16:51 +00:00
Joe Tsai
831b8f59b4 reflect/protoregistry: add conflict override
The ignoreConflict function provides the ability to ignore certain conflicts.
By default, all conflicts are ignored with a log message produced instead.

Change-Id: I67fe56eef492e12421e5c8cb8d618dc2a46c82ed
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186658
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-18 17:47:11 +00:00
Joe Tsai
43761bdfe7 cmd/protoc-gen-go: update deprecation warning
Change-Id: Ie1a854bf6f47d4ca9e941b6ccc64dc24ff32bd19
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186657
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-18 02:26:18 +00:00
Joe Tsai
1ac7b53cc5 internal/filedesc: print warnings on registration conflicts
Rather than panicking at init time due to registration failures,
print a warning to stderr. Historically, the Go protobuf implementation
has not been strict about registration conflicts, which has led users
to unknowningly tolerating conflicts that may or may not expose
themselvs as a bug.

Registration conlicts now produce a log message:
<<<
2019/07/17 17:36:42 WARNING: proto: file "path/to/example.proto" is already registered
	previously from: "example.com/company/example_proto"
	currently from:  "example.com/user/example_proto"
A future release will panic on registration conflicts.

>>>

Change-Id: I2d583f04977c8bc8cb6bbd33d239277690bbec54
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186181
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-18 02:19:58 +00:00
Joe Tsai
f647c82cc3 internal/impl: expose MessageInfo
The v2 MessageInfo is needed by v1 to be able to access the OneofWrappers
and Exporter function. The ProtoMessageInfo method can be deleted
once v1 is entirely implemented in terms of v2.

Change-Id: Iabb1b429af5210faffc6477f52b5020b3aa1fb50
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186577
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-17 23:01:13 +00:00
Joe Tsai
5ae10aa9f0 encoding: unify MessageSet extension handling logic
This CL unifies common MessageSet logic in prototext and protojson
into the messageset package. While we are at it, also enable
MessageSet support only if the proto1_legacy build flag is enabled.

Change-Id: I1a7d475e8bb1dad61ecd286df45e4239e5bef072
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185898
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-15 21:07:58 +00:00
Joe Tsai
af57087245 reflect/protoregistry: provide more informative errors for conflicts
The v2 implementation strictly enforces that there are no conflicts at
all in the protobuf namespace unlike the prior v1 implementation.
This change is almost certainly going to cause loud failures for users
that were unknowingly tolerating registration conflicts.

We modify internal/filedesc to be able to record the Go package path
that the file descriptor is declared within. This information is used
by reflect/protoregistry to print both the previous Go package that
registered some declaration, and current Go package that is attempting
to register some declaration.

Change-Id: Ib5eb21c1c98495afc51aa08bd4404bd9d64b5b57
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186177
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-15 20:39:24 +00:00
Joe Tsai
c51e2e0293 all: support enforce_utf8 override
In 2014, when proto3 was being developed, there were a number of early
adopters of the new syntax. Before the finalization of proto3 when
it was released in open-source in July 2016, a decision was made to
strictly validate strings in proto3. However, some of the early adopters
were already using invalid UTF-8 with string fields.
The google.protobuf.FieldOptions.enforce_utf8 option only exists to support
those grandfathered users where they can opt-out of the validation logic.
Practical use of that option in open source is impossible even if a user
specifies the proto1_legacy build tag since it requires a hacked
variant of descriptor.proto that is not externally available.

This CL supports enforce_utf8 by modifiyng internal/filedesc to
expose the flag if it detects it in the raw descriptor.
We add an strs.EnforceUTF8 function as a centralized place to determine
whether to perform validation. Validation opt-out is supported
only in builds with legacy support.

We implement support for validating UTF-8 in all proto3 string fields,
even if they are backed by a Go []byte.

Change-Id: I9c0628b84909bc7181125f09db730c80d490e485
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186002
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-15 19:53:05 +00:00
Damien Neil
302cb325fb proto: support message_set_wire_format
MessageSets are a deprecated proto1 feature, long since superseded by
extensions. Add disabled-by-default support behind flags.Proto1Legacy.

Change-Id: I7d3ace07f3b0efd59673034f3dc633b908345a88
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185538
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-07-15 19:32:30 +00:00
Joe Tsai
d47ea19d2f encoding: support weak fields in text and JSON unmarshaling
If the message for a weak field is linked in,
we treat it as if it were identical to a normal known field.
However, if the weak field is not linked in,
we treat it as if the field were not known.

Change-Id: I576d911deec98e13211304024a6353734d055465
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185457
Reviewed-by: Herbie Ong <herbie@google.com>
2019-07-15 19:09:08 +00:00
Joe Tsai
3d8e369c4e all: implement proto1 weak fields
This implements generation of and reflection support for weak fields.
Weak fields are a proto1 feature where the "weak" option can be specified
on a singular message field. A weak reference results in generated code
that does not directly link in the dependency containing the weak message.

Weak field support is not added to any of the serialization logic.

Change-Id: I08ccfa72bc80b2ffb6af527a1677a0a81dcf33fb
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185399
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-15 18:44:12 +00:00
Joe Tsai
e182c917f0 reflect/protoreflect: add FileDescriptor.SourceLocations
This adds minimal support for preserving the source context information.

Change-Id: I4b3cac9690b7469ecb4e5434251a809be4d7894c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/183157
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-13 00:15:59 +00:00
Joe Tsai
d421150c3b reflect/protoreflect: add Enum.Type and Message.Type
CL/174938 removed these methods in favor of a method that returned
only the descriptors. This CL adds back in the Type methods alongside
the Descriptor methods.

In a vast majority of protobuf usages, only the descriptor information
is needed. However, there is a small percentage that legitimately needs
the Go type information. We should provide both, but document that the
descriptor-only information is preferred.

Change-Id: Ia0a098997fb1bd009994940ae8ea5257ccd87cae
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184578
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-12 23:43:21 +00:00
Joe Tsai
f8b855d768 runtime/protoiface: API adjustments
The following adjustments were made:
* The pragma.NoUnkeyedLiterals is moved to be the first field.
This is done to keep the options struct smaller. Even if the last
field is zero-length, Go GC implementation details forces the struct
to be padded at the end.
* Methods are documented as always treating AllowPartial as true.
* Added a support flag for UnmarshalOptions.DiscardUnknown.

Change-Id: I1f75d226542ab2bb0123d9cea143c7060df226d8
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185998
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-12 23:12:26 +00:00
Joe Tsai
0f81b38d61 runtime/protoiface: move and rename XXX_Methods
This CL moves and renames the protoreflect.ProtoMessage.XXX_Methods
to protoreflect.Message.ProtoMethods.

Since one needs to obtain a protoreflect.Message now to get at
the fast-path methods, we modify the method signatures to take
in a protoreflect.Message instead of protoreflect.ProtoMessage.
Doing so also avoids the wrapper hack that was formerly done on
impl.messageReflectWrapper.

After this change the new protoc-gen-go no longer generates
any XXX fields or methods. All internal fields and methods are truly
hidden from the end-user.

name                                     old time/op    new time/op    delta
Wire/Unmarshal/google_message1_proto2-4    1.50µs ±10%    1.50µs ± 2%    ~     (p=0.483 n=10+9)
Wire/Unmarshal/google_message1_proto3-4    1.06µs ± 6%    1.06µs ± 4%    ~     (p=0.814 n=9+9)
Wire/Unmarshal/google_message2-4            734µs ±22%     689µs ±13%    ~     (p=0.133 n=10+9)
Wire/Marshal/google_message1_proto2-4       790ns ±46%     652ns ± 8%    ~     (p=0.590 n=10+9)
Wire/Marshal/google_message1_proto3-4       872ns ± 4%     857ns ± 3%    ~     (p=0.168 n=9+9)
Wire/Marshal/google_message2-4              232µs ±16%     221µs ± 3%  -4.75%  (p=0.014 n=9+9)
Wire/Size/google_message1_proto2-4          164ns ± 2%     167ns ± 4%  +1.87%  (p=0.046 n=9+10)
Wire/Size/google_message1_proto3-4          240ns ± 9%     229ns ± 1%  -4.81%  (p=0.000 n=9+8)
Wire/Size/google_message2-4                58.9µs ± 9%    59.6µs ± 2%  +1.23%  (p=0.040 n=9+9)

name                                     old alloc/op   new alloc/op   delta
Wire/Unmarshal/google_message1_proto2-4      912B ± 0%      912B ± 0%    ~     (all equal)
Wire/Unmarshal/google_message1_proto3-4      688B ± 0%      688B ± 0%    ~     (all equal)
Wire/Unmarshal/google_message2-4            470kB ± 0%     470kB ± 0%    ~     (p=0.215 n=10+10)
Wire/Marshal/google_message1_proto2-4        240B ± 0%      240B ± 0%    ~     (all equal)
Wire/Marshal/google_message1_proto3-4        224B ± 0%      224B ± 0%    ~     (all equal)
Wire/Marshal/google_message2-4             90.1kB ± 0%    90.1kB ± 0%    ~     (all equal)
Wire/Size/google_message1_proto2-4          0.00B          0.00B         ~     (all equal)
Wire/Size/google_message1_proto3-4          0.00B          0.00B         ~     (all equal)
Wire/Size/google_message2-4                 0.00B          0.00B         ~     (all equal)

name                                     old allocs/op  new allocs/op  delta
Wire/Unmarshal/google_message1_proto2-4      24.0 ± 0%      24.0 ± 0%    ~     (all equal)
Wire/Unmarshal/google_message1_proto3-4      6.00 ± 0%      6.00 ± 0%    ~     (all equal)
Wire/Unmarshal/google_message2-4            8.49k ± 0%     8.49k ± 0%    ~     (all equal)
Wire/Marshal/google_message1_proto2-4        1.00 ± 0%      1.00 ± 0%    ~     (all equal)
Wire/Marshal/google_message1_proto3-4        1.00 ± 0%      1.00 ± 0%    ~     (all equal)
Wire/Marshal/google_message2-4               1.00 ± 0%      1.00 ± 0%    ~     (all equal)
Wire/Size/google_message1_proto2-4           0.00           0.00         ~     (all equal)
Wire/Size/google_message1_proto3-4           0.00           0.00         ~     (all equal)
Wire/Size/google_message2-4                  0.00           0.00         ~     (all equal)

Change-Id: Ibf3263ad0f293326695c22020a92a6b938ef4f65
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185697
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-12 19:31:58 +00:00
Damien Neil
a8a2cea3e7 proto: move T->*T wrappers from internal/scalar to proto
Usage of these is pervasive in code which works with proto2, and proto2
will be with us for a long, long time to come. Move them to the proto
package.

Change-Id: I1b2e57429fd5a8f107a848a4492d20c27f304bd7
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185543
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-07-12 17:35:01 +00:00
Joe Tsai
6c28674cea proto: fix merge semantics for oneof message
The proper semantics for a message field within a oneof
when unmarshaling is to merge into an existing message,
rather than replacing it.

Change-Id: I7c08f6e4fa958c6ee6241e9083f7311515a97e15
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185957
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-12 16:20:52 +00:00
Damien Neil
7492a09da9 internal/impl: support packed extensions
Change-Id: I5a9e22f1c98f5db9caae1681775017da5aa67394
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185541
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-07-11 18:07:16 +00:00