Skip to content

Backport GCC tests+fixes#12743

Merged
mkruskal-google merged 1 commit into23.xfrom
gcc-23
May 9, 2023
Merged

Backport GCC tests+fixes#12743
mkruskal-google merged 1 commit into23.xfrom
gcc-23

Conversation

@mkruskal-google
Copy link
Copy Markdown
Member

This also adds tests for C++17 and C++20 to prevent further regressions.

PiperOrigin-RevId: 530693318

This also adds tests for C++17 and C++20 to prevent further regressions.

PiperOrigin-RevId: 530693318
@mkruskal-google mkruskal-google added the back-port Cherrypick PRs to release branches label May 9, 2023
@mkruskal-google mkruskal-google requested a review from sbenzaquen May 9, 2023 20:54
@mkruskal-google mkruskal-google requested review from a team as code owners May 9, 2023 20:54
@mkruskal-google mkruskal-google requested review from deannagarcia and perezd and removed request for a team May 9, 2023 20:54
@mkruskal-google mkruskal-google merged commit 0ce78c6 into 23.x May 9, 2023
@mkruskal-google mkruskal-google deleted the gcc-23 branch May 9, 2023 23:22
@h-vetinari
Copy link
Copy Markdown
Contributor

Hey there. I backported this patch at the suggestion of @coryan in conda-forge/libprotobuf-feedstock#156, but things still failed with GCC 12.2 and C++17. Background is we need to compile everything with C++17 due to the ABI-impact of the standard version on abseil; For 4.22.x, this worked fine.

compilation error log
[...]
[342/391] Building CXX object CMakeFiles/tests.dir/src/google/protobuf/unittest.pb.cc.o
FAILED: CMakeFiles/tests.dir/src/google/protobuf/unittest.pb.cc.o 
$BUILD_PREFIX/bin/x86_64-conda-linux-gnu-c++ -DGOOGLE_PROTOBUF_CMAKE_BUILD -DGTEST_LINKED_AS_SHARED_LIBRARY=1 -DHAVE_ZLIB -DPROTOBUF_USE_DLLS -I$SRC_DIR/build-shared -I$SRC_DIR/src -I$SRC_DIR/third_party/utf8_range -fvisibility-inlines-hidden -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt  -ffunction-sections -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/libprotobuf-4.23.0 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix -DNDEBUG -O3 -DNDEBUG -std=gnu++17 -fPIE -DGOOGLE_PROTOBUF_TEST_PLUGIN_PATH=\"$SRC_DIR/build-shared/test_plugin\" -MD -MT CMakeFiles/tests.dir/src/google/protobuf/unittest.pb.cc.o -MF CMakeFiles/tests.dir/src/google/protobuf/unittest.pb.cc.o.d -o CMakeFiles/tests.dir/src/google/protobuf/unittest.pb.cc.o -c $SRC_DIR/src/google/protobuf/unittest.pb.cc
$SRC_DIR/src/google/protobuf/unittest.pb.cc: In constructor 'constexpr protobuf_unittest::TestCord::TestCord(google::protobuf::internal::ConstantInitialized)':
$SRC_DIR/src/google/protobuf/unittest.pb.cc:3084:73: error: temporary of non-literal type 'absl::lts_20230125::Cord' in a constant expression
 3084 |       TestCord::Impl_::_default_optional_bytes_cord_default_func_{})}} {}
      |                                                                         ^
In file included from $SRC_DIR/src/google/protobuf/io/coded_stream.h:134,
                 from $SRC_DIR/src/google/protobuf/unittest.pb.h:24,
                 from $SRC_DIR/src/google/protobuf/unittest.pb.cc:4:
$PREFIX/include/absl/strings/cord.h:150:7: note: 'absl::lts_20230125::Cord' is not literal because:
  150 | class Cord {
      |       ^~~~
$PREFIX/include/absl/strings/cord.h:150:7: note:   'absl::lts_20230125::Cord' has a non-trivial destructor
Build invocation, roughly
cmake -G "Ninja" \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_CXX_STANDARD=17 \
    -Dprotobuf_ABSL_PROVIDER="package" \
    -Dprotobuf_BUILD_SHARED_LIBS=ON \
    -Dprotobuf_JSONCPP_PROVIDER="package" \
    -Dprotobuf_USE_EXTERNAL_GTEST=ON \
    -Dprotobuf_WITH_ZLIB=ON \
    ..

I note that GCC 12.2 is just at the boundary of when PROTOBUF_GNUC_MIN(12, 2) kicks in, so I tried the following patch on top of this one:

@@ -683,7 +683,7 @@ static_assert(PROTOBUF_CPLUSPLUS_MIN(201402L), "Protobuf only supports C++14 and
 // GCC doesn't support constinit aggregate initialization of absl::Cord.
 # if PROTOBUF_GNUC_MIN(12, 2)
 #  define PROTOBUF_CONSTINIT
-#  define PROTOBUF_CONSTEXPR constexpr
+#  define PROTOBUF_CONSTEXPR
 # endif
 #else
 # if defined(__cpp_constinit) && !defined(__CYGWIN__)

And then the build succeeds (and ctest --progress --output-on-failure passes).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

back-port Cherrypick PRs to release branches

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants