fix: disable PROTOBUF_CONSTINIT when building with libc++#26783
Open
linzhp wants to merge 1 commit intoprotocolbuffers:mainfrom
Open
fix: disable PROTOBUF_CONSTINIT when building with libc++#26783linzhp wants to merge 1 commit intoprotocolbuffers:mainfrom
linzhp wants to merge 1 commit intoprotocolbuffers:mainfrom
Conversation
Author
|
@esrauchg can you take a look? The test failed because the CI considered my fork as unsafe |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Building protobuf with Clang + libc++ fails to compile
port.cc:error: variable does not have a constant initializer
PROTOBUF_ATTRIBUTE_NO_DESTROY PROTOBUF_CONSTINIT
PROTOBUF_ATTRIBUTE_INIT_PRIORITY1 GlobalEmptyString
fixed_address_empty_string{};
note: pointer to subobject of heap-allocated object is not a constant expression
PROTOBUF_CONSTINITexpands toconstinit(or[[clang::require_constant_initialization]])on Clang. This requires
GlobalEmptyString's default constructor — which wrapsstd::string—to be evaluable at compile time. However, libc++'s
std::stringdefault constructor performs aheap allocation even for empty strings and is therefore not
constinit-compatible. libstdc++ andMSVC implement the Small String Optimization such that an empty
std::stringrequires noallocation, so
constinitworks there.This was previously reported in #22065 ("Unable to build v31.1 with zig-cc"), where a maintainer
noted:
The root cause is not zig-cc specifically but libc++, which zig-cc uses by default. The bug is
present on the current
mainbranch.Fix
Add
!defined(_LIBCPP_VERSION)to both Clang branches inport_def.incthat enablePROTOBUF_CONSTINIT._LIBCPP_VERSIONis the canonical macro defined by all libc++ versions.When using libc++, both branches are skipped and
PROTOBUF_CONSTINITfalls through to theexisting
#else, which already defines it as empty — so no new code path is introduced.fixed_address_empty_stringremains safely initialized ahead of other globals viaPROTOBUF_ATTRIBUTE_INIT_PRIORITY1, preserving the static initialization order guarantee withoutconstinit.Testing
Verified by building protobuf on
mainwith Clang 18 + libc++ via the Zig toolchain targetingx86_64-unknown-linux-gnu.References
Fixes #22065