Fix uses of __cpp_lib_constexpr_string #18890
Closed
Conversation
This file contains 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
zhangskz
added
c++
🅰️ safe for tests
github-actions
bot
removed
the
🅰️ safe for tests
|
Looks like there's a conflict -- can you rebase / resolve? |
|
looks like the commit that introduced this was rolled back f58849b maybe a googler can comment on if that's going to come back out, or if this patch is irrelevant |
|
The change was submitted again. |
sbenzaquen
approved these changes
Nov 5, 2024
The intent of these checks was that if the C++20 version of this feature are enabled to execute the top codepath. libstdc++ 12 defines this feature as: ```cpp // in: /usr/include/c++/12/bits/basic_string.h #ifdef __cpp_lib_is_constant_evaluated // Support P0980R1 in C++20. # define __cpp_lib_constexpr_string 201907L #elif __cplusplus >= 201703L && _GLIBCXX_HAVE_IS_CONSTANT_EVALUATED // Support P0426R1 changes to char_traits in C++17. # define __cpp_lib_constexpr_string 201611L #endif ``` So this codepath was always being hit even with -std=c++17 and then resulted in a failure because the string wasn't actually constinit. This matches the other use of this feature check in `inlined_string_field.h`
keith
force-pushed
the
ks/fix-uses-of-__cpp_lib_constexpr_string
branch
from
91bb279 to
edb4f5f
Compare
|
reabsed! |
tonyliaoss
added
the
🅰️ safe for tests
github-actions
bot
removed
the
🅰️ safe for tests
copybara-service bot
pushed a commit
that referenced
this pull request
Nov 14, 2024
The intent of these checks was that if the C++20 version of this feature are enabled to execute the top codepath. libstdc++ 12 defines this feature as: ```cpp // in: /usr/include/c++/12/bits/basic_string.h #ifdef __cpp_lib_is_constant_evaluated // Support P0980R1 in C++20. # define __cpp_lib_constexpr_string 201907L #elif __cplusplus >= 201703L && _GLIBCXX_HAVE_IS_CONSTANT_EVALUATED // Support P0426R1 changes to char_traits in C++17. # define __cpp_lib_constexpr_string 201611L #endif ``` So this codepath was always being hit even with -std=c++17 and then resulted in a failure because the string wasn't actually constinit. This matches the other use of this feature check in `inlined_string_field.h` Closes #18890 COPYBARA_INTEGRATE_REVIEW=#18890 from keith:ks/fix-uses-of-__cpp_lib_constexpr_string edb4f5f FUTURE_COPYBARA_INTEGRATE_REVIEW=#18890 from keith:ks/fix-uses-of-__cpp_lib_constexpr_string edb4f5f PiperOrigin-RevId: 696299666
copybara-service bot
pushed a commit
that referenced
this pull request
Nov 14, 2024
The intent of these checks was that if the C++20 version of this feature are enabled to execute the top codepath. libstdc++ 12 defines this feature as: ```cpp // in: /usr/include/c++/12/bits/basic_string.h #ifdef __cpp_lib_is_constant_evaluated // Support P0980R1 in C++20. # define __cpp_lib_constexpr_string 201907L #elif __cplusplus >= 201703L && _GLIBCXX_HAVE_IS_CONSTANT_EVALUATED // Support P0426R1 changes to char_traits in C++17. # define __cpp_lib_constexpr_string 201611L #endif ``` So this codepath was always being hit even with -std=c++17 and then resulted in a failure because the string wasn't actually constinit. This matches the other use of this feature check in `inlined_string_field.h` Closes #18890 COPYBARA_INTEGRATE_REVIEW=#18890 from keith:ks/fix-uses-of-__cpp_lib_constexpr_string edb4f5f FUTURE_COPYBARA_INTEGRATE_REVIEW=#18890 from keith:ks/fix-uses-of-__cpp_lib_constexpr_string edb4f5f PiperOrigin-RevId: 696299666
protobuf-team-bot
added a commit
that referenced
this pull request
Nov 14, 2024
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.
The intent of these checks was that if the C++20 version of this feature
are enabled to execute the top codepath. libstdc++ 12 defines this
feature as:
So this codepath was always being hit even with -std=c++17 and then
resulted in a failure because the string wasn't actually constinit. This
matches the other use of this feature check in
inlined_string_field.h