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
protocolbuffers · Oct 18, 2024 · fe53593 · fe53593
1 parent b93b8e5
commit fe53593
Show file tree

Hide file tree

Showing 10 changed files with 54 additions and 38 deletions.
diff --git a/.github/workflows/test_cpp.yml b/.github/workflows/test_cpp.yml
@@ -130,7 +130,7 @@ jobs:
             -c "set -ex;
             sccache -z;
             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 }};
             cmake --build . --parallel 20;
             ctest --parallel 20;
@@ -141,15 +141,13 @@ jobs:
       fail-fast: false   # Don't cancel all jobs if one fails.
       matrix:
         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
-            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
           - 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
-          - name: C++17
-            flags: -DCMAKE_CXX_STANDARD=17 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF
           - name: C++20
             flags: -DCMAKE_CXX_STANDARD=20 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF
           - name: Fetch
@@ -217,15 +215,15 @@ jobs:
           image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.16.9-f39fc8b4e244fe5cd4c7138d0b6959a52b46ca48
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           command: >-
-            /install.sh -DCMAKE_CXX_STANDARD=14 ${{ env.SCCACHE_CMAKE_FLAGS }}
+            /install.sh -DCMAKE_CXX_STANDARD=17 ${{ env.SCCACHE_CMAKE_FLAGS }}
             ${{ matrix.flags }}
             -Dprotobuf_BUILD_SHARED_LIBS=ON \&\&
             /test.sh
             ${{ env.SCCACHE_CMAKE_FLAGS }}
             -Dprotobuf_REMOVE_INSTALLED_HEADERS=ON
             -Dprotobuf_BUILD_PROTOBUF_BINARIES=OFF
             -Dprotobuf_BUILD_CONFORMANCE=ON
-            -DCMAKE_CXX_STANDARD=14
+            -DCMAKE_CXX_STANDARD=17
             ${{ matrix.flags }}
 
   # This test should always be skipped on presubmit
@@ -253,21 +251,19 @@ jobs:
           image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.16.9-f39fc8b4e244fe5cd4c7138d0b6959a52b46ca48
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           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_BUILD_EXAMPLES=OFF \&\&
             mkdir examples/build \&\&
             cd examples/build \&\&
-            cmake .. -DCMAKE_CXX_STANDARD=14 \&\&
+            cmake .. -DCMAKE_CXX_STANDARD=17 \&\&
             cmake --build .
 
   linux-cmake-gcc:
     strategy:
       fail-fast: false   # Don't cancel all jobs if one fails.
       matrix:
         include:
-          - name: C++14
-            flags: -DCMAKE_CXX_STANDARD=14
           - name: C++17
             flags: -DCMAKE_CXX_STANDARD=17
             continuous-only: true
@@ -331,7 +327,7 @@ jobs:
               /bin/bash -cex '
               cd /workspace;
               sccache -z;
-              cmake . -DCMAKE_CXX_STANDARD=14 ${{ env.SCCACHE_CMAKE_FLAGS }};
+              cmake . -DCMAKE_CXX_STANDARD=17 ${{ env.SCCACHE_CMAKE_FLAGS }};
               cmake --build . --parallel 20;
               ctest --verbose --parallel 20;
               sccache -s'
@@ -391,7 +387,6 @@ jobs:
         include:
           - name: MacOS CMake
             os: macos-13
-            flags: -DCMAKE_CXX_STANDARD=14
             cache-prefix: macos-cmake
             continuous-only: true
           - name: Windows CMake
@@ -430,7 +425,9 @@ jobs:
             cache-prefix: windows-2022-cmake
           - name: Windows CMake Install
             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: >-
               -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF
               -Dprotobuf_REMOVE_INSTALLED_HEADERS=ON
@@ -478,7 +475,9 @@ jobs:
         uses: protocolbuffers/protobuf-ci/bash@v3
         with:
           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
         if: ${{ matrix.install-flags && (!matrix.continuous-only || inputs.continuous-run) }}
         shell: bash
@@ -501,7 +500,9 @@ jobs:
         uses: protocolbuffers/protobuf-ci/bash@v3
         with:
           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
         if: ${{ !matrix.continuous-only || inputs.continuous-run }}

diff --git a/ci/Linux.bazelrc b/ci/Linux.bazelrc
@@ -1,5 +1,6 @@
 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 --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion" --copt="-Wno-deprecated-declarations"
+
diff --git a/ci/Windows.bazelrc b/ci/Windows.bazelrc
@@ -1,5 +1,7 @@
 import common.bazelrc
 
 # 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
-common --enable_runfiles
+common --enable_runfiles
+
diff --git a/ci/macOS.bazelrc b/ci/macOS.bazelrc
@@ -1,7 +1,8 @@
 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 --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 --xcode_version_config=@com_google_protobuf//.github:host_xcodes
+
diff --git a/examples/.bazelrc b/examples/.bazelrc
@@ -1,9 +1,9 @@
 common --enable_platform_specific_config
 
-build:linux --cxxopt=-std=c++14 --host_cxxopt=-std=c++14
-build:macos --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++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 --remote_download_outputs=all
diff --git a/src/google/protobuf/arena_unittest.cc b/src/google/protobuf/arena_unittest.cc
@@ -1541,11 +1541,7 @@ TEST(ArenaTest, ClearOneofMessageOnArena) {
 
 #ifndef PROTOBUF_ASAN
   EXPECT_NE(child->moo_int(), 100);
-#else
-#if GTEST_HAS_DEATH_TEST && defined(__cpp_if_constexpr)
-  EXPECT_DEATH(EXPECT_EQ(child->moo_int(), 0), "use-after-poison");
-#endif
-#endif
+#endif  // !PROTOBUF_ASAN
 }
 
 TEST(ArenaTest, CopyValuesWithinOneof) {

diff --git a/src/google/protobuf/port_def.inc b/src/google/protobuf/port_def.inc
@@ -116,7 +116,8 @@ static_assert(PROTOBUF_GNUC_MIN(7, 3), "Protobuf only supports GCC 7.3 and newer
 #elif defined(_MSVC_LANG)
 static_assert(PROTOBUF_MSC_VER_MIN(1910), "Protobuf only supports MSVC 2017 and newer.");
 #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.
 #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
 #error PROTOBUF_NODISCARD was previously defined
 #endif
-#if ABSL_HAVE_CPP_ATTRIBUTE(nodiscard) && PROTOBUF_CPLUSPLUS_MIN(201703L)
+#if ABSL_HAVE_CPP_ATTRIBUTE(nodiscard)
 #define PROTOBUF_NODISCARD [[nodiscard]]
 #elif ABSL_HAVE_ATTRIBUTE(warn_unused_result) || defined(__GNUC__)
 #define PROTOBUF_NODISCARD __attribute__((warn_unused_result))
@@ -615,8 +616,7 @@ static_assert(PROTOBUF_ABSL_MIN(20230125, 3),
 #ifdef PROTOBUF_UNUSED
 #error PROTOBUF_UNUSED was previously defined
 #endif
-#if ABSL_HAVE_CPP_ATTRIBUTE(maybe_unused) || \
-    (PROTOBUF_MSC_VER_MIN(1911) && PROTOBUF_CPLUSPLUS_MIN(201703L))
+#if ABSL_HAVE_CPP_ATTRIBUTE(maybe_unused) || (PROTOBUF_MSC_VER_MIN(1911))
 #define PROTOBUF_UNUSED [[maybe_unused]]
 #elif ABSL_HAVE_ATTRIBUTE(unused) || defined(__GNUC__)
 #define PROTOBUF_UNUSED __attribute__((__unused__))

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

diff --git a/src/google/protobuf/reflection_visit_field_info.h b/src/google/protobuf/reflection_visit_field_info.h
@@ -1137,6 +1137,13 @@ struct RepeatedGroupDynamicExtensionInfo
 // users from a similar dispatch without creating KeyInfo or ValueInfo per type.
 template <FieldDescriptor::CppType cpp_type, typename T>
 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) {
     static_assert(std::is_same_v<T, int32_t>, "type mismatch");
     switch (type) {

diff --git a/src/google/protobuf/reflection_visit_fields_test.cc b/src/google/protobuf/reflection_visit_fields_test.cc
@@ -172,7 +172,7 @@ void MutateNothingByVisit(Message& message) {
         }
       } else {
         for (auto& it : info.Mutable()) {
-          it = it;
+          it = *&it;  // Avoid -Wself-assign.
         }
       }
     } else {
@@ -207,6 +207,12 @@ TEST_P(VisitFieldsTest, MutateNothingByVisitIdempotent) {
 
 template <typename InfoT>
 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) {
     return WireFormatLite::StringSize(info.Get());
   } else {
@@ -216,6 +222,12 @@ inline size_t MapKeyByteSizeLong(FieldDescriptor::Type type, InfoT info) {
 
 template <typename InfoT>
 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) {
     return WireFormatLite::StringSize(info.Get());
   } else if constexpr (info.cpp_type == FieldDescriptor::CPPTYPE_MESSAGE) {