Skip to content
Permalink
Branch: master
Commits on Jul 10, 2019
  1. internal/impl: avoid preserving presence on proto3 bytes field

    dsnet committed Jul 10, 2019
    When setting a proto3 bytes field to an empty, non-nil bytes slice,
    just store a nil slice in the underderlying storage.
    This is done to avoid presenting the illusion to the user that
    presence is preserved for proto3.
    
    Updates golang/protobuf#896
    
    Change-Id: I1b97bedd547d336863c65d9418d8f07edf69ccd6
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185577
    Reviewed-by: Herbie Ong <herbie@google.com>
  2. internal/impl: add MessageState to every generated message

    dsnet committed Jun 20, 2019
    We define MessageState, which is essentially an atomically set *MessageInfo.
    By nesting this as the first field in every generated message, we can
    implement the reflective methods on a *MessageState when obtained by
    unsafe casting a concrete message pointer as a *MessageState.
    The MessageInfo held by MessageState provides additional Go type information
    to interpret the memory that comes after the contents of the MessageState.
    
    Since we are nesting a MessageState in every message,
    the memory use of every message instance grows by 8B.
    
    On average, the body of ProtoReflect grows from 133B to 202B (+50%).
    However, this is offset by XXX_Methods, which is 108B and
    will be removed in a future CL. Taking into account the eventual removal
    of XXX_Methods, this is a net reduction of 25%.
    
    name          old time/op    new time/op    delta
    Name/Value-4    70.3ns ± 2%    17.5ns ± 6%   -75.08%  (p=0.000 n=10+10)
    Name/Nil-4      70.6ns ± 3%    33.4ns ± 2%   -52.66%  (p=0.000 n=10+10)
    
    name          old alloc/op   new alloc/op   delta
    Name/Value-4     16.0B ± 0%      0.0B       -100.00%  (p=0.000 n=10+10)
    Name/Nil-4       16.0B ± 0%      0.0B       -100.00%  (p=0.000 n=10+10)
    
    name          old allocs/op  new allocs/op  delta
    Name/Value-4      1.00 ± 0%      0.00       -100.00%  (p=0.000 n=10+10)
    Name/Nil-4        1.00 ± 0%      0.00       -100.00%  (p=0.000 n=10+10)
    
    Change-Id: I92bd58dc681c57c92612fd5ba7fc066aea34e95a
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185460
    Reviewed-by: Damien Neil <dneil@google.com>
  3. encoding: use strs.UnsafeString to avoid duplicated code

    dsnet committed Jul 10, 2019
    The strs.UnsafeString casts a []byte as a string.
    This allows us to avoid duplicated functionality.
    
    Change-Id: I9930b94bae35eac0f98c0fa62963b300bc8d7e49
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185459
    Reviewed-by: Herbie Ong <herbie@google.com>
  4. encoding: cleanup some tests

    dsnet committed Jul 10, 2019
    Change-Id: I07b8cd5e9b71fea6a162ca423186b4df7fd49355
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185458
    Reviewed-by: Herbie Ong <herbie@google.com>
Commits on Jul 9, 2019
  1. cmd/protoc-gen-go: add more flags to control generation of deprecated…

    dsnet committed Apr 17, 2019
    … features
    
    Change-Id: I57f5927c1d3a834de067a5e0a96d957a7af9aafe
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/172657
    Reviewed-by: Herbie Ong <herbie@google.com>
  2. internal/encoding/tag: support marshaling weak fields

    dsnet committed Jul 9, 2019
    We already support unmarshaling weak fields, support the other direction.
    
    Change-Id: I514e6b7b18cf9a3567fb1775a1e389fe64f22d22
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185340
    Reviewed-by: Herbie Ong <herbie@google.com>
  3. internal/encoding/tag: fix handling of JSON name

    dsnet committed Jul 9, 2019
    When decoding, only treat the name as being explicitly set if the
    name differs from what the JSON name would have been had it been
    automatically derived from the protobuf field name.
    
    Change-Id: Ida9256eafe7af20c7a06be2d4fb298be44276104
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185398
    Reviewed-by: Herbie Ong <herbie@google.com>
  4. internal/strs: unify string manipulation functionality

    dsnet committed Jul 7, 2019
    Create a new internal/strs package that unifies common functionality:
    * Since protobuf itself pseudo-specifies at least 4 different camel-case
    and snake-case conversion functions, we define all variants in one place.
    * We move the internal/filedesc.nameBuilder function to this package.
    We simplify its implementation to not depend on a strings.Builder fork
    under the hood since the semantics we desire is simpler than what
    strings.Builder provides.
    * We use strs.Builder in reflect/protodesc in its construction of all
    the full names. This is perfect use case of strs.Builder since all
    full names within a file descriptor share the same lifetime.
    * Add an UnsafeString and UnsafeBytes cast function that will be useful
    in the near future for optimizing encoding/prototext and encoding/protojson.
    
    Change-Id: I2cf07cbaf6f72e5f9fd6ae3d37b0d46f6af2ad59
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185198
    Reviewed-by: Damien Neil <dneil@google.com>
Commits on Jul 8, 2019
  1. cmd/protoc-gen-go: disable generation of XXX_WellKnownType

    dsnet committed Jul 8, 2019
    The logic for this will be removed entirely in a future CL.
    
    Change-Id: I3a20fb2a207fc5bedc550b3f6deeab7f63eaa2c4
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185243
    Reviewed-by: Herbie Ong <herbie@google.com>
  2. cmd/protoc-gen-go: remove MessageSet hackery

    dsnet committed Jul 8, 2019
    The encoding/prototext and encoding/protojson are implemented entirely
    in terms of protobuf reflection, which side-steps this information.
    Remove the hacks in the generator to special-case MessageSet.
    
    Change-Id: I708c4636b77672545a103b7ab686f103b9dfc514
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185240
    Reviewed-by: Herbie Ong <herbie@google.com>
  3. cmd/protoc-gen-go: unexport implementation-specific XXX fields

    dsnet committed Jul 6, 2019
    We modify protoc-gen-go to stop generating exported XXX fields.
    The unsafe implementation is unaffected by this change since unsafe
    can access fields regardless of visibility. However, for the purego
    implementation, we need to respect Go visibility rules as enforced
    by the reflect package.
    
    We work around this by generating a exporter function that given
    a reference to the message and the field to export, returns a reference
    to the unexported field value. This exporter function is protected by
    a constant such that it is not linked into the final binary in non-purego
    build environment.
    
    Updates golang/protobuf#276
    
    Change-Id: Idf5c1f158973fa1c61187ff41440acb21c5dac94
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185141
    Reviewed-by: Damien Neil <dneil@google.com>
  4. cmd/protoc-gen-go: remove generation of XXX_OneofWrappers

    dsnet committed Jul 8, 2019
    Associate the oneof wrapper types with a message by conveying that
    information to the associated MessageInfo.
    
    Change-Id: Iabfca593850e1d6a89498a37eacbf22dbb73bd20
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185239
    Reviewed-by: Damien Neil <dneil@google.com>
  5. cmd/protoc-gen-go/testdata: add registry test

    dsnet committed Jun 29, 2019
    This test verifies that the inialization logic does not accidentally
    execute the lazy initialization logic that is intended to only evaluate
    upon first use.
    
    Change-Id: I5e9dea17ce88081c78195af0e86dd38223689f63
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184259
    Reviewed-by: Damien Neil <dneil@google.com>
  6. internal/encoding/wire: fix trivial misspelling

    dsnet committed Jun 29, 2019
    Change-Id: I71f1715145ebc58aadec1d5ecb5f1900998f65a0
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184277
    Reviewed-by: Damien Neil <dneil@google.com>
  7. cmd/protoc-gen-go: use Merge instead of Marshal/Unmarshal

    dsnet committed Jul 8, 2019
    Change-Id: I689dee8417944bb0fbe19c75b7ca1accb49afb33
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185144
    Reviewed-by: Herbie Ong <herbie@google.com>
  8. all: fix stale comments

    dsnet committed Jul 7, 2019
    Change-Id: I4bfef75bc74c8d876a3926635bea12bbbaf4993e
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185238
    Reviewed-by: Damien Neil <dneil@google.com>
  9. internal/filedesc: avoid deep-copying the options

    dsnet committed Jul 7, 2019
    The protoreflect.Descriptor.Options method is currently documented as
    returning a reference to the options, where the user must not mutate
    the returned message. This changes internal/filedesc to avoid returning
    a copy of the options by caching the first unmarshal.
    
    See golang/protobuf#877
    
    Change-Id: I15701d33fbda7535b21b2add72628b02992c373f
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185197
    Reviewed-by: Benny Siegert <bsiegert@gmail.com>
  10. internal/impl: centralize the logic for finding XXX fields

    dsnet committed Jul 6, 2019
    Change-Id: I177fbeeb474eeb86e4a47229ac5c48cb7191e95b
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185140
    Reviewed-by: Damien Neil <dneil@google.com>
Commits on Jul 3, 2019
  1. reflect/protodesc: enforce strict validation

    dsnet committed Jul 2, 2019
    Hyrum's Law dictates that if we do not prevent naughty behavior,
    people will rely on it. If we do not validate that the provided
    file descriptor is correct today, it will be near impossible
    to add proper validation checks later on.
    
    The logic added validates that the provided file descriptor is
    correct according to the same semantics as protoc,
    which was reversed engineered to derive the set of rules implemented here.
    The rules are unfortunately complicated because protobuf is a language
    full of many non-orthogonal features. While our logic is complicated,
    it is still 1/7th the size of the equivalent C++ code!
    
    Change-Id: I6acc5dc3bd2e4c6bea6cd9e81214f8104402602a
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184837
    Reviewed-by: Damien Neil <dneil@google.com>
  2. reflect/protodesc: robustify dependency resolution implementation

    dsnet committed Jun 29, 2019
    Overview of changes:
    * Add an option that specifies whether to replace unresolvable references
    with a placeholder instead of producing an error. Since the prior behavior
    produced placeholders (not always), we default to that behavior for now,
    but will enable strict resolving in a future CL.
    * The option is not yet exported because there is concern about what the
    public API should look like. This will be exposed in a future CL.
    * Unlike before, we now permit placeholders for unresolvable enum values.
    * We implement relative name resolution logic.
    * We handle the case where the type is unknown, but type_name is specified.
    In such a case, we populate both FieldDescriptor.{Enum,Message} and leave
    the FieldDescriptor.Kind with the zero value. If the type_name happened
    to resolve, we use that to determine the type.
    * If a placeholder is used to represent a relative name,
    the FullName reports an invalid full name with a "*." prefix.
    
    Change-Id: Ifa8c750423c488fb9324eec4d033a2f251505fda
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184317
    Reviewed-by: Damien Neil <dneil@google.com>
  3. reflect/protodesc: return deep copies of the input

    dsnet committed Jul 3, 2019
    There is little performance benefit to aliasing the input since we copy
    every field except the options. Thus, just go all the way and copy the
    options as well and document this as such.
    
    Change-Id: If6ca5ce0ee03c9f76e528023b6056ad99d3ca209
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184879
    Reviewed-by: Damien Neil <dneil@google.com>
  4. encoding: avoid direct access to XXX_unrecognized

    dsnet committed Jun 22, 2019
    Use the SetUnknown API instead.
    
    Change-Id: Id9dc81581084ccf8ef5453a80c75fcfd63289636
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184877
    Reviewed-by: Herbie Ong <herbie@google.com>
  5. all: remove dependency on proto v1

    dsnet committed May 16, 2019
    This does not remove all dependencies,
    but all of the cases where it can now be implemented in terms of v2.
    
    Change-Id: Idc5b0273f0d35c284bf2141eb9cce998692ceb15
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184878
    Reviewed-by: Herbie Ong <herbie@google.com>
Commits on Jul 2, 2019
  1. internal/impl: fix race in aberrant message logic

    dsnet committed Jul 2, 2019
    Previously, when aberrantLoadMessageDesc returned it was guaranteed
    to have initialized the current message through the use of the done signal.
    However, this does not guarantee that the descriptor for a cylic reference
    has also finished initialization.
    
    Rather than add more complicated logic to wait until all cyclic references
    have finished initializing, just add a global lock for the entire
    aberrantLoadMessageDesc function.
    
    This slows down performance, but is easier to reason about.
    
    Change-Id: I4cdae8b955f71ee40fa6979f5a8d548d9749042c
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184657
    Reviewed-by: Damien Neil <dneil@google.com>
Commits on Jul 1, 2019
  1. reflect/protodesc: fix initialization bug

    dsnet committed Jun 29, 2019
    This fixes a bug introduced by CL/182360.
    
    Overview of the problem:
    * CL/182360 removes the internal/prototype package, such that
    protodesc was re-implemented using internal/filedesc.
    * As a result of that change, resolving internal dependencies became
    the responsibility of protodesc.
    * Dependency resolution used the following two-pass algorithm:
    	1) first pass derives the full name of all declarations
    	2) second pass fully initializes each descriptor declaration,
    	now being able to resolve local dependencies from the previous step.
    * When the second pass looks up a local dependency, it is guaranteed to
    find it, but it is not guaranteed that the dependency has been initialized
    (since it may appear later on). This is problematic for default enum values
    since it implies that the enum dependency may not be sufficiently
    initialized to be able to query its set of values, leading to panics.
    * CL/182360 recognized the problem and attempted to enforce an initialization
    ordering where nested enums were always initialized before the body of the
    message declaration itself.
    * However, that ordering fails to enforce that that enum declarations outside
    the parent tree are initialized beforehand. For example, referring to an
    enum value that is declared within a sibling of the parent message.
    * This CL fixes the problem with a three-pass algorithm:
    	1) first pass derives the full name *and* fully initialize the
    	entire descriptor *except* for dependency references (i.e., type_name).
    	2) second pass only resolves dependency references,
    	where we do not need to worry about initialization ordering.
    	3) third pass validates the descriptors are well-formed.
    	This can now depend on all information being fully initialized.
    * While a lot of code moves, this change is actually very mechanical.
    Other than split things apart, no new logic is introduced nor removed.
    
    Change-Id: Ia91d4aade8f6187c19d704d43ae96b3b9d276792
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184297
    Reviewed-by: Damien Neil <dneil@google.com>
  2. internal/impl: drop XXX_MessageName aberrant support

    dsnet committed Jul 1, 2019
    The aberrant support logic only has access to the Go type
    information, and not a concrete value. However, the XXX_MessageName
    method exists on some hacky dynamic proto implementations where
    it is only valid to call on a concrete value, not just newly created
    instance of the given type.
    
    However, from the perspective of the support logic, it is impossible
    to distinguish between dynamic messages and hand-crafted custom messages.
    Thus, just drop support for XXX_MessageName. We won't get the full name
    of the message right, but oh well, what can we do.
    
    Change-Id: Icc272861e11a355639fb82a991ca2854a9edc0c7
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184557
    Reviewed-by: Damien Neil <dneil@google.com>
  3. internal/impl: support aberrant enums and messages

    dsnet committed Jul 1, 2019
    Aberrant messages are hand-crafted messages that happen to work because
    they use the same struct tags that generated code emits.
    This happens to work in v1, but is unspecified behavior and entirely outside
    the compatibility promise.
    
    Support for this was added early on in the history of the v2 implementation,
    but entirely untested. It was removed in CL/182360 to reduce the
    technical debt of the legacy implementation. Unfortunately, sufficient number
    of targets do rely on this aberrant support, so it is being added back.
    
    The logic being added is essentially the same thing as the previous logic,
    but ported to use internal/filedesc instead of the now deleted
    internal/prototype package.
    
    Change-Id: Ib5cab3e90480825b9615db358044ce05a14b05bd
    Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184517
    Reviewed-by: Damien Neil <dneil@google.com>
You can’t perform that action at this time.