This unifies proto2 and proto3 (and later editions) implementations to incorporate performance optimizations directly referencing unsafe and caching reflection presence field (and offset) to all syntaxes. These optimizations were originally added in cl/187404278 for proto2 only.
These optimizations do not seem to be meaningfully proto2/3-specific. The UnsafeUtil methods simply wrap the corresponding Unsafe methods. Presence bit fields are used in the same way for proto3, but this optimization seems to predate proto3 optional.
While in there, better document how the buffer encoding and mask/offsets work.
PiperOrigin-RevId: 540320373
Our automation can't currently update release branches, so we need to enable staleness tests as presubmits to force manual regeneration.
PiperOrigin-RevId: 540094169
This implements the associated type change suggested by dmitrig@.
I don't foresee use cases that would make `ViewFor` allowing multiple `T` to be
useful, rather than confusing.
This also implements some suggested wording changes that I agree with.
PiperOrigin-RevId: 540024883
This treats clear similarly to has and get to avoids issues from missing clear method for escaped synthetic oneofs (e.g. field _underscore -> oneof X_underscore). Previously, `clear` was using the clear method of the field (which has the same camel-cased name outside of the underscore case).
We also remove synthetic oneof camelCase names from the gencode for FieldAccesorTable since these should not be used / exposed.
Fixes#12880
PiperOrigin-RevId: 539069001
The previous way of defining them is not compatible with unsigned integer overflow sanitizer, which complains about the conversion from uint64_t to int64_t in the macro LL().
PiperOrigin-RevId: 538418425
Hi,
It seems that until last year, the logic behind `PROTOBUF_USE_DLLS` was for Windows (MSCV) only. It was changed to all platforms here in 5a0887fc65
Last month, the generated pkg config files were updated to reflect the protobuf build-time value of `PROTOBUF_USE_DLLS` as it was indeed noted that it changes the ABI. This was done in https://github.com/protocolbuffers/protobuf/pull/12700 In the commit message it is mentionned that most likely we shall rather have a stable ABI.
Finally in https://github.com/protocolbuffers/protobuf/issues/12746 which at some point mentions https://issuetracker.google.com/issues/283987730#comment7 where a Google employee hits the linker issue:
```
undefined reference to `google::protobuf::internal::ThreadSafeArena::thread_cache_'
```
which denotes a mix of some .o or libs built `PROTOBUF_USE_DLLS` defined and some others build with `PROTOBUF_USE_DLLS` undefined, resulting in ABI incompatibilities.
I also hit this issue while trying to include protobuf in a corporate environment using it's own proprietary build system in which it is expected that .a and .so use a compatible ABI.
From my own understanding, ideally we should always use `thread_local` variables, but experience has shown that:
- old iOS (iOS < 9) didn't seem to accept `thread_local`, leading to the `GOOGLE_PROTOBUF_NO_THREADLOCAL` macro later renamed `PROTOBUF_NO_THREADLOCAL` which allowed to disable this, but it is not set anywhere in the protobuf code base. Also I doubt you still want to support such old iOS now, so maybe you should consider removing all `PROTOBUF_NO_THREADLOCAL` related code paths (this pull request doesn't do this).
- MSVC's DLL interface doesn't seem to accept exporting thread local variables (at least from what I understood, I know absolutely nothing about the Windows ecosystem), yet we can "hide" a thread local variable in a static function using a thread local variable. However in that case the access to TLS variable is not inlined, leading to worse performances, this hack shall be done only for Windows (actually when using MSVC) *AND* we build a shared library.
- In all other cases, a classical `thread_local` shall be used, no matter if we build a static or a shared library. In particular on Linux which I guess is the target Google cares the more about for its own production. This pull request achieves this.
Am I right in my conclusion ?
Closes#12983
COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/12983 from Romain-Geissler-1A:stable-abi-use-dll-non-windows dc23ff50f6
PiperOrigin-RevId: 538230923
This additional if is necessary as of abseil 20230125.3 when abseil is consumed via add_subdirectory,
the abseil_dll target is named abseil_dll, while if abseil is consumed via find_package, the target is called `absl::abseil_dll` .
Once https://github.com/abseil/abseil-cpp/pull/1466 is merged and released in the minimum version of abseil required by protobuf, it is possible to always link `absl::abseil_dll` and `absl::abseil_test_dll` and remove the if.
You may wonder how linking worked at all before when `protobuf_ABSL_PROVIDER STREQUAL "package"`, as `abseil_dll` was not an imported target defined by `find_package(absl)`. The reason behind this is that if a name that is not an imported target is passed to `target_link_libraries`, it is just regarded as a C++ library name. So, in the end the `abseil_dll` library was correctly linked, simply all the transitive usage requirements defined by the `absl::abseil_dll` target were not propagated, that could lead to compilation errors if abseil was compiled with the `ABSL_PROPAGATE_CXX_STD` CMake option enabled.
Closes#12978
COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/12978 from traversaro:patch-1 39dd074281
PiperOrigin-RevId: 537990391
Matt Kulukundis
Sandy Zhang
Protobuf Team Bot
Protobuf Team Bot
Mike Kruskal
Marcel Hlopko
Dmitri Gribenko
Tomo Suzuki
Jie Luo
Dave MacLachlan
Thomas Van Lenten
Chad Wilson
Krzysztof Kosiński
Romain Geissler @ Amadeus
Silvio Traversaro