I'm just about to edit this code, and I want these tests for my peace of mind.
Also update the other callers to use assertThrows for consistency.
PiperOrigin-RevId: 673590917
This has twofold goals:
1. Correctness: if position overruns the array, checking space left may return a negative number. I'm not sure how bad that is, but let's avoid it.
2. Performance. This generates more optimal assembly code which can combine bounds checks, particularly on Android (I haven't looked at the generated assembly on the server JVM; it's possible the server JVM can already performance this hoist).
The `position` field is stored on the object, so Android ART generates assembly codes for `this.position++` like "load, add, store":
```
ldr w3, [x1, #12]
add w4, w3, #0x1 (1)
str w4, [x1, #12]
```
There can be a lot of these loads/stores executed each step of a loop (e.g. writeFixed64NoTag updates position 8 times, and varint encoding could do it even more). It's faster if we can hoist these so we load once at the start of the function, and store once at the end of the function. This also has the nice benefit that it won't store if we've thrown an exception.
See before/after in Compiler Explorer: https://godbolt.org/z/bWWYqsxK4. I'm not an assembly expert, but it seems clear that the increment instructions like `add w4, w0, #0x1 (1)` are no longer always surrounded by loads and stores in the new version.
And in Compiler Explorer, you also see `bufferFixed64NoTag` has reduced from 98 lines of assembly to 57 lines of assembly in the hoisted version. This is because we don't need to re-check the array bounds each time we reload `position`. I imagine this also makes any other method with a fixed number of increments like `writeFixed32NoTag` faster too.
PiperOrigin-RevId: 673588324
This reduces the total number of instantiations and reduces binary size costs.
It puts an hard upper bound on the total number of instantiations of the template.
PiperOrigin-RevId: 673452657
In the worst case, the source object is a constant (eg a global default instance) and modifying them has undefined behavior.
PiperOrigin-RevId: 673402981
The `retention` and `target` options have been functional for a long time, so
we can delete these stale comments saying that they do not work yet.
PiperOrigin-RevId: 673401292
- Add overloads that take `absl::Cord` and `std::string&&` as inputs, putting
the burden in the implementation instead of users.
- Hide the APIs that return `std::string*` when the breaking change is
enabled (via `-DPROTOBUF_TEMPORARY_ENABLE_STRING_VIEW_RETURN_TYPE`)....
PiperOrigin-RevId: 673397581
`ClassData`. This reduces code size bloat and runtime costs when using the
custom vtable. It has no significant change for normal mode.
PiperOrigin-RevId: 673010897
This enables enforcement of lifetime specifications on individual enum values for features. It will allow us to add new values to existing features, as well as deprecate/and remove existing values. By default, each value will be scoped to the lifetime spec of its corresponding feature field. However, individual lifetime boundaries can be overridden at the value-level for finer grained control.
In the near-term, this will allow us to deprecate/remove required field presence, and add a stricter utf8 validation feature.
PiperOrigin-RevId: 672710484
* `GenericTypeHandler` used to be forward-declared in message_lite.h and unncessarily friended by `MessageLite`, which was its only use in that source. repeated_ptr_field.h relied on that forward declaration being there, and fully defined `GenericTypeHandler` below the first use.
* `StringPieceField` was forward-declared, and now IWYU'ed.
PiperOrigin-RevId: 672686570
This appears to be causing a bunch of errors during our release process,
but I don't think it's critical to have this flag in `.bazelrc`. We
should still be able to get about the same test coverage since we also
set `-Werror` in the `bazelrc` files in the `ci/` directory.
PiperOrigin-RevId: 672673187
* s/Handler/TypeHandler/g.
* Consistently use aliased `Type` throughout the `GenericTypeHandler` specializations. This makes visual diffing between them easier.
PiperOrigin-RevId: 672665545
Introduce a upb_MessageValue_Zero() function to use for the cases we do want a zero'd union (typically a zero MessageValue union is not needed)
PiperOrigin-RevId: 672592274
Normal mode keeps the existing member function path.
In the future we might change this to a static member function instead to avoid the bloat of the member pointer, but that currently affects normal mode and we want to avoid it for now.
PiperOrigin-RevId: 672552340
The goal of the `names.h` convention is to have a single canonical place where a code generator can define the set of symbols it exports to other code generators, and a canonical place where the name mangling logic is implemented.
Each upb code generator now has its own `names.h` file defining the symbols that it owns & exports:
* `third_party/upb/upb_generator/c/names.h` (for `foo.upb.h` files)
* `third_party/upb/upb_generator/minitable/names.h` (for `foo.upb_minitable.h` files)
* `third_party/upb/upb_generator/reflection/names.h` (for `foo.upbdefs.h` files)
This is a significant improvement over the previous situation where the name mangling functions were co-mingled in `common.h`/`mangle.h`, or sprinkled throughout the generators, with no clear structure for which code generator owns which symbols.
With this structure in place, the visibility lists for the various `names.h` files provide a clear dependency graph for how different generators depend on each other. In general, we want to keep dependencies on the "C" code generator to a minimum, since it is the largest and most complicated of upb's generated APIs, and is also the most prone to symbol name clashes.
Note that upb's `names.h` headers are somewhat unusual, in that we do not want them to depend on C++'s reflection or upb's reflection. Most `names.h` headers in protobuf would use types like `proto2::Descriptor`, but we don't want upb to depend on C++ reflection, especially during its bootstrapping process. We also don't want to force users to build upb defs just to use these name mangling functions. So we use only plain string types like `absl::string_view` and `std::string`.
PiperOrigin-RevId: 672397247
This yields several benefits:
1. The code no longer needs to be bootstrapped (since it no longer depends on upb reflection).
2. The upb code generator no longer depends on libprotobuf at all (except for `code_generator_lite.{h,cc}`, which is just one .cc file and has no deps).
PiperOrigin-RevId: 672280579
Mark Hansen
Protobuf Team Bot
Mike Kruskal
Protobuf Team Bot
Adam Cozzette
Hong Shin
Derek Benson
liqipeng
Joshua Haberman