This is just part of an effort at locking down (and understand well) how map
entries are serialized into textproto when they have empty fields.
Right now the C++ behaviour for DynamicMessages is that for implicit-presence
(i.e. "proto3") fields, the empty MapEntry fields are not serialized even if
explicitly set. For example, `value: ""` would not show up in textproto
serialization for proto3 MapEntry messages. This contrasts with C++ map
reflection behaviour, because C++ MapEntry messages are always generated with
hasbits (even if they are declared in a proto3 file and have implicit presence
according to their descriptors).
For this Python test, the DynamicMessage fallback path was triggered because
the implementation of the C++ proto was not found. When python is lacking in
implementation, it calls DynamicMessage which respects field presence, even for
map entries. If the corresponding `map_unittest_cc_proto` is linked, this test
actually fails, because the C++ MapEntry implementation, which is always
generated with hasbits even for implicit presence, is used.
PiperOrigin-RevId: 681944020
This should rename the self parameter name to "self_" if any field is named "self", not rename the corresponding keyword parameter name to "self_" if the first field is named "self".
PiperOrigin-RevId: 681872250
Previously, we didn't test for OutOfSpaceException, because all writes were buffered. The OutOfSpaceException wouldn't happen until flushing.
If we flush the stream, we can detect OutOfSpaceException.
Further, I saw some tests saying "If streaming, try different block sizes." These tests used block size (1, 2, 4, 8, 16), which are all less than the minimum block size of 20. So all these were rounded up to 20, and were not doing much useful: http://google3/third_party/java_src/protobuf/current/java/com/google/protobuf/CodedOutputStream.java;l=2280;rcl=679381814.
I've replaced such loops with a two OutputTypes:
- STREAM, which has a buffer of the size of the output
- STREAM_MINIMUM_BUFFER_SIZE, which has a buffer size 20
This allows deleting some extra duplicated code.
PiperOrigin-RevId: 681756512
This produces much better code for Android: https://godbolt.org/z/xE8T9xqrr
Down from 196 bytes to 140 bytes. Bounds checks get combined together.
This is a partial roll-forward of cl/673588324.
PiperOrigin-RevId: 681703327
This saves one ARM instruction (`mov x1, x4`) when the array is out of bounds:
https://godbolt.org/z/7Gb7so4Ez
Because the side effects of position++ have to happen even if the array overflows.
It's fairly minor. Probably won't make a big difference.
This is a partial roll forward of cl/673588324.
PiperOrigin-RevId: 681696805
Other methods are now updating position at the end of the method.
Also, it's good practice to limit the scope of try { } blocks.
This is a partial roll-forward of cl/673588324
PiperOrigin-RevId: 681673291
This has been replaced with a public cpp_string_type descriptor API, that supports both ctype and string_type smoothly between editions.
PiperOrigin-RevId: 681647787
When writing varints.
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.
PiperOrigin-RevId: 681644516
Empty strings are invalid numeric values. upb must fail to convert JSON objects that contain empty string values for numbers, but it currently does not.
PiperOrigin-RevId: 681623866
Adding java8 tests as a presubmit test to ensure no regression on java8 compatibility since the option will no longer apply to tests.
PiperOrigin-RevId: 681615348
This will allow usage of these variables from `.proto.h` files where the dependency types are incomplete.
Improve `TypeId::Get` by using the trait instead of calling through the default. This reduces binary size and runtime costs.
PiperOrigin-RevId: 681604581
Move hpb::internal::{Serialize, HasExtensionOrUnknown, GetOrPromoteExtension, DeepCopy, DeepClone} to message_lock.h as they all use the message lock
absl status helpers are now housed in status.h
PiperOrigin-RevId: 681432239
GCC aggressively emit warnings when comparing unsigned and signed integer types, which causes failures under protobuf's -Werror default. We can fix these often by switching to iterators, but sometimes it's easiest to add a cast or switch a variable type.
Closes#17212
COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/17212 from benjaminp:unsigned-size-comparison-warnings 4b3c9c2b4a
PiperOrigin-RevId: 681232361
Before, some encoders would not give any details about position/limit/length.
Now a few more places do.
Just found this while trying to add some tests for the exception message, and
found some encoders weren't setting it.
This doesn't fix all the places that OutOfSpaceException didn't have a useful message.
PiperOrigin-RevId: 681218740
In certain cases, it is useful to share submessages across multiple parent messages.
proto2::cpp has a mechanism for this, so we add the hpb equivalent.
For this initial impl, we stipulate that the arenas must be exactly the same. We may explore broadening the constraint to allow for all fused arenas.
PiperOrigin-RevId: 681169537
We reserved a range of 10 a couple of years ago but are nearing exhausting that block. We anticipate needing to define more custom options, so this allocates another block of numbers to Buf.
Closes#18548
COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/18548 from jhump:jh/more-option-numbers 898fdec6db
PiperOrigin-RevId: 680810934
We generate these constants to enable map operations, but this is no longer
necessary now that we can get the relevant size and alignment information for
each message through its vtable.
PiperOrigin-RevId: 680712939
mkruskal-google
Tony Liao
Samuel Freilich
Mark Hansen
Protobuf Team Bot
Joshua Haberman
Deanna Garcia
Protobuf Team Bot
Hong Shin
Derek Benson
Benjamin Peterson
Joshua Humphries
Mike Kruskal
Adam Cozzette