Bump minimum C++ version to C++17 after branch cut for v29.

Branch cut for v29 is done on 2024-09-30:
https://github.com/protocolbuffers/protobuf/releases/tag/v29.0-rc1

The next version v30 will be a breaking release. The release date is scheduled
after the EOL of C++14 support on 2024-12-15 for Google open source projects
generally:
https://github.com/google/oss-policies-info/blob/main/foundational-cxx-support-matrix.md

This commit allows us to start taking advantage of C++17 features now.

Some issues I ran into while upgrading:

Two GCC 9.5 bugs related to -Wunused-but-set-parameter:
- https://godbolt.org/z/qo51cKe7b
- https://godbolt.org/z/65qW3vGhP

Another GCC warning related to -Wself-assign in a template.

There is a custom ASAN check that is not yet open sourced. I'll see if I can
open source them in a subsequent commit.

#test-continuous

PiperOrigin-RevId: 687435042
pull/18773/head
Tony Liao 5 months ago committed by Copybara-Service
parent b93b8e5f64
commit fe535930d3
  1. 35
      .github/workflows/test_cpp.yml
  2. 3
      ci/Linux.bazelrc
  3. 2
      ci/Windows.bazelrc
  4. 3
      ci/macOS.bazelrc
  5. 6
      examples/.bazelrc
  6. 6
      src/google/protobuf/arena_unittest.cc
  7. 8
      src/google/protobuf/port_def.inc
  8. 6
      src/google/protobuf/proto3_arena_unittest.cc
  9. 7
      src/google/protobuf/reflection_visit_field_info.h
  10. 14
      src/google/protobuf/reflection_visit_fields_test.cc

@ -130,7 +130,7 @@ jobs:
-c "set -ex; -c "set -ex;
sccache -z; sccache -z;
cmake . -DWITH_PROTOC=/workspace/${{ steps.cross-compile.outputs.protoc }} cmake . -DWITH_PROTOC=/workspace/${{ steps.cross-compile.outputs.protoc }}
-Dprotobuf_BUILD_LIBUPB=OFF -Dprotobuf_BUILD_CONFORMANCE=ON -DCMAKE_CXX_STANDARD=14 -Dprotobuf_BUILD_LIBUPB=OFF -Dprotobuf_BUILD_CONFORMANCE=ON -DCMAKE_CXX_STANDARD=17
-Dprotobuf_WITH_ZLIB=OFF ${{ env.SCCACHE_CMAKE_FLAGS }}; -Dprotobuf_WITH_ZLIB=OFF ${{ env.SCCACHE_CMAKE_FLAGS }};
cmake --build . --parallel 20; cmake --build . --parallel 20;
ctest --parallel 20; ctest --parallel 20;
@ -141,15 +141,13 @@ jobs:
fail-fast: false # Don't cancel all jobs if one fails. fail-fast: false # Don't cancel all jobs if one fails.
matrix: matrix:
include: include:
- flags: -Dprotobuf_BUILD_EXAMPLES=ON -DCMAKE_CXX_STANDARD=14 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF - flags: -Dprotobuf_BUILD_EXAMPLES=ON -DCMAKE_CXX_STANDARD=17 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF
- name: Ninja - name: Ninja
flags: -G Ninja -DCMAKE_CXX_STANDARD=14 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF flags: -G Ninja -DCMAKE_CXX_STANDARD=17 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF
continuous-only: true continuous-only: true
- name: Shared - name: Shared
flags: -Dprotobuf_BUILD_SHARED_LIBS=ON -Dprotobuf_BUILD_EXAMPLES=ON -DCMAKE_CXX_STANDARD=14 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF flags: -Dprotobuf_BUILD_SHARED_LIBS=ON -Dprotobuf_BUILD_EXAMPLES=ON -DCMAKE_CXX_STANDARD=17 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF
continuous-only: true continuous-only: true
- name: C++17
flags: -DCMAKE_CXX_STANDARD=17 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF
- name: C++20 - name: C++20
flags: -DCMAKE_CXX_STANDARD=20 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF flags: -DCMAKE_CXX_STANDARD=20 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF
- name: Fetch - name: Fetch
@ -217,7 +215,7 @@ jobs:
image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.16.9-f39fc8b4e244fe5cd4c7138d0b6959a52b46ca48 image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.16.9-f39fc8b4e244fe5cd4c7138d0b6959a52b46ca48
credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
command: >- command: >-
/install.sh -DCMAKE_CXX_STANDARD=14 ${{ env.SCCACHE_CMAKE_FLAGS }} /install.sh -DCMAKE_CXX_STANDARD=17 ${{ env.SCCACHE_CMAKE_FLAGS }}
${{ matrix.flags }} ${{ matrix.flags }}
-Dprotobuf_BUILD_SHARED_LIBS=ON \&\& -Dprotobuf_BUILD_SHARED_LIBS=ON \&\&
/test.sh /test.sh
@ -225,7 +223,7 @@ jobs:
-Dprotobuf_REMOVE_INSTALLED_HEADERS=ON -Dprotobuf_REMOVE_INSTALLED_HEADERS=ON
-Dprotobuf_BUILD_PROTOBUF_BINARIES=OFF -Dprotobuf_BUILD_PROTOBUF_BINARIES=OFF
-Dprotobuf_BUILD_CONFORMANCE=ON -Dprotobuf_BUILD_CONFORMANCE=ON
-DCMAKE_CXX_STANDARD=14 -DCMAKE_CXX_STANDARD=17
${{ matrix.flags }} ${{ matrix.flags }}
# This test should always be skipped on presubmit # This test should always be skipped on presubmit
@ -253,12 +251,12 @@ jobs:
image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.16.9-f39fc8b4e244fe5cd4c7138d0b6959a52b46ca48 image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.16.9-f39fc8b4e244fe5cd4c7138d0b6959a52b46ca48
credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
command: >- command: >-
/install.sh -DCMAKE_CXX_STANDARD=14 ${{ env.SCCACHE_CMAKE_FLAGS }} /install.sh -DCMAKE_CXX_STANDARD=17 ${{ env.SCCACHE_CMAKE_FLAGS }}
-Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF
-Dprotobuf_BUILD_EXAMPLES=OFF \&\& -Dprotobuf_BUILD_EXAMPLES=OFF \&\&
mkdir examples/build \&\& mkdir examples/build \&\&
cd examples/build \&\& cd examples/build \&\&
cmake .. -DCMAKE_CXX_STANDARD=14 \&\& cmake .. -DCMAKE_CXX_STANDARD=17 \&\&
cmake --build . cmake --build .
linux-cmake-gcc: linux-cmake-gcc:
@ -266,8 +264,6 @@ jobs:
fail-fast: false # Don't cancel all jobs if one fails. fail-fast: false # Don't cancel all jobs if one fails.
matrix: matrix:
include: include:
- name: C++14
flags: -DCMAKE_CXX_STANDARD=14
- name: C++17 - name: C++17
flags: -DCMAKE_CXX_STANDARD=17 flags: -DCMAKE_CXX_STANDARD=17
continuous-only: true continuous-only: true
@ -331,7 +327,7 @@ jobs:
/bin/bash -cex ' /bin/bash -cex '
cd /workspace; cd /workspace;
sccache -z; sccache -z;
cmake . -DCMAKE_CXX_STANDARD=14 ${{ env.SCCACHE_CMAKE_FLAGS }}; cmake . -DCMAKE_CXX_STANDARD=17 ${{ env.SCCACHE_CMAKE_FLAGS }};
cmake --build . --parallel 20; cmake --build . --parallel 20;
ctest --verbose --parallel 20; ctest --verbose --parallel 20;
sccache -s' sccache -s'
@ -391,7 +387,6 @@ jobs:
include: include:
- name: MacOS CMake - name: MacOS CMake
os: macos-13 os: macos-13
flags: -DCMAKE_CXX_STANDARD=14
cache-prefix: macos-cmake cache-prefix: macos-cmake
continuous-only: true continuous-only: true
- name: Windows CMake - name: Windows CMake
@ -430,7 +425,9 @@ jobs:
cache-prefix: windows-2022-cmake cache-prefix: windows-2022-cmake
- name: Windows CMake Install - name: Windows CMake Install
os: windows-2022 os: windows-2022
install-flags: -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF -Dprotobuf_BUILD_TESTS=OFF install-flags: >-
-G Ninja -Dprotobuf_WITH_ZLIB=OFF
-Dprotobuf_BUILD_CONFORMANCE=OFF -Dprotobuf_BUILD_TESTS=OFF
flags: >- flags: >-
-G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF
-Dprotobuf_REMOVE_INSTALLED_HEADERS=ON -Dprotobuf_REMOVE_INSTALLED_HEADERS=ON
@ -478,7 +475,9 @@ jobs:
uses: protocolbuffers/protobuf-ci/bash@v3 uses: protocolbuffers/protobuf-ci/bash@v3
with: with:
credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
command: cmake . ${{ matrix.install-flags }} ${{ env.SCCACHE_CMAKE_FLAGS }} -Dprotobuf_ALLOW_CCACHE=ON command: >-
cmake . -DCMAKE_CXX_STANDARD=17 ${{ matrix.install-flags }}
${{ env.SCCACHE_CMAKE_FLAGS }} -Dprotobuf_ALLOW_CCACHE=ON
- name: Build for install - name: Build for install
if: ${{ matrix.install-flags && (!matrix.continuous-only || inputs.continuous-run) }} if: ${{ matrix.install-flags && (!matrix.continuous-only || inputs.continuous-run) }}
shell: bash shell: bash
@ -501,7 +500,9 @@ jobs:
uses: protocolbuffers/protobuf-ci/bash@v3 uses: protocolbuffers/protobuf-ci/bash@v3
with: with:
credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
command: cmake . ${{ matrix.flags }} ${{ env.SCCACHE_CMAKE_FLAGS }} -Dprotobuf_ALLOW_CCACHE=ON command: >-
cmake . -DCMAKE_CXX_STANDARD=17 ${{ matrix.flags }}
${{ env.SCCACHE_CMAKE_FLAGS }} -Dprotobuf_ALLOW_CCACHE=ON
- name: Build - name: Build
if: ${{ !matrix.continuous-only || inputs.continuous-run }} if: ${{ !matrix.continuous-only || inputs.continuous-run }}

@ -1,5 +1,6 @@
import common.bazelrc import common.bazelrc
build --cxxopt=-std=c++14 --host_cxxopt=-std=c++14 build --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
build --cxxopt="-Woverloaded-virtual" build --cxxopt="-Woverloaded-virtual"
build --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion" --copt="-Wno-deprecated-declarations" build --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion" --copt="-Wno-deprecated-declarations"

@ -1,5 +1,7 @@
import common.bazelrc import common.bazelrc
# Workaround for maximum path length issues # Workaround for maximum path length issues
build --cxxopt=/std:c++17 --host_cxxopt=/std:c++17
startup --output_user_root=C:/tmp --windows_enable_symlinks startup --output_user_root=C:/tmp --windows_enable_symlinks
common --enable_runfiles common --enable_runfiles

@ -1,7 +1,8 @@
import common.bazelrc import common.bazelrc
build --cxxopt=-std=c++14 --host_cxxopt=-std=c++14 build --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
build --cxxopt="-Woverloaded-virtual" build --cxxopt="-Woverloaded-virtual"
build --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion" --copt="-Wno-deprecated-declarations" build --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion" --copt="-Wno-deprecated-declarations"
common --repo_env=BAZEL_NO_APPLE_CPP_TOOLCHAIN=1 common --repo_env=BAZEL_NO_APPLE_CPP_TOOLCHAIN=1
common --xcode_version_config=@com_google_protobuf//.github:host_xcodes common --xcode_version_config=@com_google_protobuf//.github:host_xcodes

@ -1,9 +1,9 @@
common --enable_platform_specific_config common --enable_platform_specific_config
build:linux --cxxopt=-std=c++14 --host_cxxopt=-std=c++14 build:linux --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
build:macos --cxxopt=-std=c++14 --host_cxxopt=-std=c++14 build:macos --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
common:windows --enable_runfiles common:windows --cxxopt=/std:c++17 --host_cxxopt=/std:c++17 --enable_runfiles
build --experimental_remote_cache_eviction_retries=5 build --experimental_remote_cache_eviction_retries=5
build --remote_download_outputs=all build --remote_download_outputs=all

@ -1541,11 +1541,7 @@ TEST(ArenaTest, ClearOneofMessageOnArena) {
#ifndef PROTOBUF_ASAN #ifndef PROTOBUF_ASAN
EXPECT_NE(child->moo_int(), 100); EXPECT_NE(child->moo_int(), 100);
#else #endif // !PROTOBUF_ASAN
#if GTEST_HAS_DEATH_TEST && defined(__cpp_if_constexpr)
EXPECT_DEATH(EXPECT_EQ(child->moo_int(), 0), "use-after-poison");
#endif
#endif
} }
TEST(ArenaTest, CopyValuesWithinOneof) { TEST(ArenaTest, CopyValuesWithinOneof) {

@ -116,7 +116,8 @@ static_assert(PROTOBUF_GNUC_MIN(7, 3), "Protobuf only supports GCC 7.3 and newer
#elif defined(_MSVC_LANG) #elif defined(_MSVC_LANG)
static_assert(PROTOBUF_MSC_VER_MIN(1910), "Protobuf only supports MSVC 2017 and newer."); static_assert(PROTOBUF_MSC_VER_MIN(1910), "Protobuf only supports MSVC 2017 and newer.");
#endif #endif
static_assert(PROTOBUF_CPLUSPLUS_MIN(201402L), "Protobuf only supports C++14 and newer."); static_assert(PROTOBUF_CPLUSPLUS_MIN(201703L),
"Protobuf only supports C++17 and newer.");
// Check minimum Abseil version. // Check minimum Abseil version.
#if defined(ABSL_LTS_RELEASE_VERSION) && defined(ABSL_LTS_RELEASE_PATCH_LEVEL) #if defined(ABSL_LTS_RELEASE_VERSION) && defined(ABSL_LTS_RELEASE_PATCH_LEVEL)
@ -371,7 +372,7 @@ static_assert(PROTOBUF_ABSL_MIN(20230125, 3),
#ifdef PROTOBUF_NODISCARD #ifdef PROTOBUF_NODISCARD
#error PROTOBUF_NODISCARD was previously defined #error PROTOBUF_NODISCARD was previously defined
#endif #endif
#if ABSL_HAVE_CPP_ATTRIBUTE(nodiscard) && PROTOBUF_CPLUSPLUS_MIN(201703L) #if ABSL_HAVE_CPP_ATTRIBUTE(nodiscard)
#define PROTOBUF_NODISCARD [[nodiscard]] #define PROTOBUF_NODISCARD [[nodiscard]]
#elif ABSL_HAVE_ATTRIBUTE(warn_unused_result) || defined(__GNUC__) #elif ABSL_HAVE_ATTRIBUTE(warn_unused_result) || defined(__GNUC__)
#define PROTOBUF_NODISCARD __attribute__((warn_unused_result)) #define PROTOBUF_NODISCARD __attribute__((warn_unused_result))
@ -615,8 +616,7 @@ static_assert(PROTOBUF_ABSL_MIN(20230125, 3),
#ifdef PROTOBUF_UNUSED #ifdef PROTOBUF_UNUSED
#error PROTOBUF_UNUSED was previously defined #error PROTOBUF_UNUSED was previously defined
#endif #endif
#if ABSL_HAVE_CPP_ATTRIBUTE(maybe_unused) || \ #if ABSL_HAVE_CPP_ATTRIBUTE(maybe_unused) || (PROTOBUF_MSC_VER_MIN(1911))
(PROTOBUF_MSC_VER_MIN(1911) && PROTOBUF_CPLUSPLUS_MIN(201703L))
#define PROTOBUF_UNUSED [[maybe_unused]] #define PROTOBUF_UNUSED [[maybe_unused]]
#elif ABSL_HAVE_ATTRIBUTE(unused) || defined(__GNUC__) #elif ABSL_HAVE_ATTRIBUTE(unused) || defined(__GNUC__)
#define PROTOBUF_UNUSED __attribute__((__unused__)) #define PROTOBUF_UNUSED __attribute__((__unused__))

@ -289,11 +289,7 @@ TEST(Proto3ArenaTest, CheckOneofMessageFieldIsCleared) {
#ifndef PROTOBUF_ASAN #ifndef PROTOBUF_ASAN
EXPECT_EQ(child->bb(), 0); EXPECT_EQ(child->bb(), 0);
#else #endif // !PROTOBUF_ASAN
#if GTEST_HAS_DEATH_TEST && defined(__cpp_if_constexpr)
EXPECT_DEATH(EXPECT_EQ(child->bb(), 100), "use-after-poison");
#endif
#endif
} }
TEST(Proto3OptionalTest, OptionalFieldDescriptor) { TEST(Proto3OptionalTest, OptionalFieldDescriptor) {

@ -1137,6 +1137,13 @@ struct RepeatedGroupDynamicExtensionInfo
// users from a similar dispatch without creating KeyInfo or ValueInfo per type. // users from a similar dispatch without creating KeyInfo or ValueInfo per type.
template <FieldDescriptor::CppType cpp_type, typename T> template <FieldDescriptor::CppType cpp_type, typename T>
inline size_t MapPrimitiveFieldByteSize(FieldDescriptor::Type type, T value) { inline size_t MapPrimitiveFieldByteSize(FieldDescriptor::Type type, T value) {
// There is a bug in GCC 9.5 where if-constexpr arguments are not understood
// if encased in a switch statement. A reproduction of the bug can be found
// at: https://godbolt.org/z/qo51cKe7b
// This is fixed in GCC 10.1+.
(void)type; // Suppress -Wunused-but-set-parameter
(void)value; // Suppress -Wunused-but-set-parameter
if constexpr (cpp_type == FieldDescriptor::CPPTYPE_INT32) { if constexpr (cpp_type == FieldDescriptor::CPPTYPE_INT32) {
static_assert(std::is_same_v<T, int32_t>, "type mismatch"); static_assert(std::is_same_v<T, int32_t>, "type mismatch");
switch (type) { switch (type) {

@ -172,7 +172,7 @@ void MutateNothingByVisit(Message& message) {
} }
} else { } else {
for (auto& it : info.Mutable()) { for (auto& it : info.Mutable()) {
it = it; it = *&it; // Avoid -Wself-assign.
} }
} }
} else { } else {
@ -207,6 +207,12 @@ TEST_P(VisitFieldsTest, MutateNothingByVisitIdempotent) {
template <typename InfoT> template <typename InfoT>
inline size_t MapKeyByteSizeLong(FieldDescriptor::Type type, InfoT info) { inline size_t MapKeyByteSizeLong(FieldDescriptor::Type type, InfoT info) {
// There is a bug in GCC 9.5 where if-constexpr arguments are not understood
// if passed into a helper function. A reproduction of the bug can be found
// at: https://godbolt.org/z/65qW3vGhP
// This is fixed in GCC 10.1+.
(void)type; // Suppress -Wunused-but-set-parameter
if constexpr (info.cpp_type == FieldDescriptor::CPPTYPE_STRING) { if constexpr (info.cpp_type == FieldDescriptor::CPPTYPE_STRING) {
return WireFormatLite::StringSize(info.Get()); return WireFormatLite::StringSize(info.Get());
} else { } else {
@ -216,6 +222,12 @@ inline size_t MapKeyByteSizeLong(FieldDescriptor::Type type, InfoT info) {
template <typename InfoT> template <typename InfoT>
inline size_t MapValueByteSizeLong(FieldDescriptor::Type type, InfoT info) { inline size_t MapValueByteSizeLong(FieldDescriptor::Type type, InfoT info) {
// There is a bug in GCC 9.5 where if-constexpr arguments are not understood
// if passed into a helper function. A reproduction of the bug can be found
// at: https://godbolt.org/z/65qW3vGhP
// This is fixed in GCC 10.1+.
(void)type; // Suppress -Wunused-but-set-parameter
if constexpr (info.cpp_type == FieldDescriptor::CPPTYPE_STRING) { if constexpr (info.cpp_type == FieldDescriptor::CPPTYPE_STRING) {
return WireFormatLite::StringSize(info.Get()); return WireFormatLite::StringSize(info.Get());
} else if constexpr (info.cpp_type == FieldDescriptor::CPPTYPE_MESSAGE) { } else if constexpr (info.cpp_type == FieldDescriptor::CPPTYPE_MESSAGE) {

Loading…
Cancel
Save