ListFields uses a slightly different codepath from HasField (both are
implemented in generated_message_reflection.cc), and it would be valuable to
add a test case for ListFields in addition to HasFields when reflection API are
tested.
ListFields is used, for example, in Python textproto serialization as a way to
iterate a message.
ListField behaves slightly differently for normal messages vs. MapEntry
messages. In a map, MapEntry essentially have "explicit presence" and empty
fields are considered present.
PiperOrigin-RevId: 682504379
This PR does the following:
* Special-case descriptor.proto to allow for codegen despite being proto2
* Fix a pre-existing bug that gets exercised now during descriptor builds
* Expand test coverage of conformance tests, and fix pre-existing issues
* Properly hook up gencode to staleness infrastructure for automated regen
Closes#18610
COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/18610 from protocolbuffers:php-regen 773a1bf01a
PiperOrigin-RevId: 682477438
These are supposed to control some code differences between open source and our
internal codebase, but some small errors or typos seem to have prevented a few
of them from working as intended.
I had to fix up some tests as well. Some test cases had to be tweaked to avoid
making assumptions about exact file paths in error messages, and there was also
a memory leak caused by `getcwd` allocating a buffer that was never freed.
Fixes#17507.
PiperOrigin-RevId: 682420154
Removes information on how to build Maven from source since that build path is no longer supported.
Also updates the documentation to cover how to use Kotlin protobufs.
PiperOrigin-RevId: 682382817
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
Protobuf Team Bot
Protobuf Team Bot
Tony Liao
Mike Kruskal
Adam Cozzette
Thomas Van Lenten
Deanna Garcia
Joshua Haberman
Eric Salo
Mark Hansen
Hong Shin
Samuel Freilich
Derek Benson
Benjamin Peterson