gmock-matchers.h patch could be upstreamed to googletest

[pogo59]

As part of the upgrade to googletest v1.14.0, there's a patch made some time ago to googlemock/include/gmock/gmock-matchers.h described as "support std::begin/std::end". In my own non-LLVM googletest work I haven't run into this, but it's not obviously in support of anything specific to LLVM either. There's a gist which has a lot more than this one patch in it, but if you look at just gmock-matchers.h in that gist, that's the sum total of the change.

This ticket has two goals:

1. Understand exactly what triggers the build error that prompted the patch in the first place.
2. If it's not something specific to LLVM, propose the change to upstream googletest.

[zeroomega]

When using the upstream v1.14.0 gmock-matcher.h, the LLVM unit test will fail to build. Example of the error message can be seen at: https://gist.github.com/zeroomega/1654cbdf714f265c05da8d86b6854971

[mikaelholmen]

Hi,

I'm seeing compilation failures with this patch on our downstream systems. I'm on RHEL7 and compiling this patch I get FAILED: compiler-rt/lib/memprof/tests/CMakeFiles/MemProfUnitTests.dir/driver.cpp.o /repo/uabelho/main-github/llvm/build-all-builtins/./bin/clang++ --target=x86_64-unknown-linux-gnu -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_LIBCPP_ENABLE_HARDENED_MODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/repo/uabelho/main-github/compiler-rt/lib/memprof/.. -I/repo/uabelho/main-github/compiler-rt/lib/memprof/../../include -I/repo/uabelho/main-github/compiler-rt/lib/memprof/tests/../../../include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -Wall -Werror -Wno-unused-parameter -O3 -DNDEBUG -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -g -Wno-covered-switch-default -Wno-suggest-override -DGTEST_NO_LLVM_SUPPORT=1 -DGTEST_HAS_RTTI=0 -I/repo/uabelho/main-github/runtimes/../third-party/unittest/googletest/include -I/repo/uabelho/main-github/runtimes/../third-party/unittest/googletest -I/repo/uabelho/main-github/runtimes/../third-party/unittest/googlemock/include -I/repo/uabelho/main-github/runtimes/../third-party/unittest/googlemock -I/repo/uabelho/main-github/compiler-rt/lib/ -DSANITIZER_COMMON_NO_REDEFINE_BUILTINS -O2 -fno-rtti -Wno-pedantic -fno-omit-frame-pointer -Wno-variadic-macros -std=c++17 -MD -MT compiler-rt/lib/memprof/tests/CMakeFiles/MemProfUnitTests.dir/driver.cpp.o -MF compiler-rt/lib/memprof/tests/CMakeFiles/MemProfUnitTests.dir/driver.cpp.o.d -o compiler-rt/lib/memprof/tests/CMakeFiles/MemProfUnitTests.dir/driver.cpp.o -c /repo/uabelho/main-github/compiler-rt/lib/memprof/tests/driver.cpp In file included from /repo/uabelho/main-github/compiler-rt/lib/memprof/tests/driver.cpp:9: In file included from /repo/uabelho/main-github/runtimes/../third-party/unittest/googletest/include/gtest/gtest.h:64: /repo/uabelho/main-github/runtimes/../third-party/unittest/googletest/include/gtest/gtest-assertion-result.h:207:48: error: no member named 'make_unique' in namespace 'std' 207 | if (message_ == nullptr) message_ = ::std::make_unique<::std::string>(); | ~~~~~~~^ /repo/uabelho/main-github/runtimes/../third-party/unittest/googletest/include/gtest/gtest-assertion-result.h:207:73: error: expected '(' for function-style cast or type construction 207 | if (message_ == nullptr) message_ = ::std::make_unique<::std::string>(); | ~~~~~~~~~~~~~^ /repo/uabelho/main-github/runtimes/../third-party/unittest/googletest/include/gtest/gtest-assertion-result.h:207:75: error: expected expression 207 | if (message_ == nullptr) message_ = ::std::make_unique<::std::string>(); | ^ and it follows with many more errors. Anyone knows anything about this?

[zeroomega]

Patch https://github.com/llvm/llvm-project/commit/a866ce789eb99da4d7a486eeb60a53be6c75f4fd updated the GoogleTest under third-party/unittest/googletest to v1.14.0.

This is the first time I saw an error like yours. From the error message, it seems that an header file is missing. @mikaelholmen Could you share the step (cmake command) to reproduce your build please?

[zeroomega]

The error is a bit weird as std::make_unique is defined in header and it is included in both gtest-assertion-result.h and gtest.h.

[mikaelholmen]

My cmake command looks like CC='/app/vbuild/RHEL7-x86_64/clang/15.0.5/bin/clang -march=corei7 --gcc-toolchain=/proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/1.9.3.0-1/crosscompiler' CXX='/app/vbuild/RHEL7-x86_64/clang/15.0.5/bin/clang++ -march=corei7 --gcc-toolchain=/proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/1.9.3.0-1/crosscompiler' LDFLAGS='-Wl,-R/proj/bbi_twh/wh_bbi/x86_64-Linux3/z3/4.8.8-1/lib64' PATH=/app/vbuild/RHEL7-x86_64/ninja/1.10.2/bin:$PATH /app/vbuild/RHEL7-x86_64/cmake/3.20.1/bin/cmake /repo/uabelho/main-github/llvm --debug-trycompile -G Ninja -DCMAKE_MAKE_PROGRAM=/app/vbuild/RHEL7-x86_64/ninja/1.10.2/bin/ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_FLAGS='-Wno-error=unused-command-line-argument' -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_INSTALL_PREFIX=/compiler-clang -DLLVM_APPEND_VC_REV=OFF -DCMAKE_CXX_COMPILER_LAUNCHER='CCACHE_DIR=/repo/uabelho/.ccache/;CCACHE_BASEDIR=/repo;CCACHE_CPP2=yes;/app/vbuild/RHEL7-x86_64/ccache/3.2.2/bin/ccache' -DCMAKE_C_COMPILER_LAUNCHER='CCACHE_DIR=/repo/uabelho/.ccache/;CCACHE_BASEDIR=/repo;CCACHE_CPP2=yes;/app/vbuild/RHEL7-x86_64/ccache/3.2.2/bin/ccache' -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_ENABLE_WERROR=ON -DLLVM_ENABLE_PROJECTS='clang;clang-tools-extra;lld' -DLLVM_BUILTIN_TARGETS='x86_64-unknown-linux-gnu' -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON -DLLVM_ENABLE_RUNTIMES='libcxx;libcxxabi;libunwind;compiler-rt' -DRUNTIMES_x86_64-unknown-linux-gnu_SANITIZER_USE_STATIC_LLVM_UNWINDER=ON -DRUNTIMES_x86_64-unknown-linux-gnu_SANITIZER_USE_STATIC_CXX_ABI=ON -DRUNTIMES_x86_64-unknown-linux-gnu_COMPILER_RT_USE_BUILTINS_LIBRARY=ON -DRUNTIMES_x86_64-unknown-linux-gnu_COMPILER_RT_CAN_EXECUTE_TESTS=OFF -DRUNTIMES_x86_64-unknown-linux-gnu_COMPILER_RT_BUILD_XRAY=OFF -DRUNTIMES_x86_64-unknown-linux-gnu_COMPILER_RT_BUILD_ORC=OFF -DLLVM_RUNTIME_TARGETS='x86_64-unknown-linux-gnu' -DLLVM_ENABLE_Z3_SOLVER=ON -DLLVM_Z3_INSTALL_DIR=/proj/bbi_twh/wh_bbi/x86_64-Linux3/z3/4.8.8-1 -DLLVM_ENABLE_LIBPFM=OFF -DLLVM_ENABLE_ZSTD=OFF -DLLVM_ENABLE_LIBXML2=OFF -DLLVM_ENABLE_TERMINFO=OFF -DLLVM_ENABLE_LIBEDIT=OFF -DLLVM_STATIC_LINK_CXX_STDLIB=ON -DCLANG_ENABLE_ARCMT=OFF -DCLANG_TOOLING_BUILD_AST_INTROSPECTION=OFF The gcc toolchain we have at /proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/1.9.3.0-1/crosscompiler contains gcc 9.3 with linux-3.10.108 and glibc-2.17

[zeroomega]

The failed command didn't have an explict sysroot directory, nor does it specify which stdlib to use. I suspect the C++ library (with its header) used in your build is too old and and it doesn't have C++14 features like make_unique.

Could you manually invoke the failed clang++ command with -v and examine the output please? It should list all the include paths. If it uses host c++ headers, it might be your host headers are too old and doesn't support C++14 features.

If you look at the failed command, it uses the stage1 clang++ without any sysroot related flags, unlike your CC and CXX env args which uses clang/15.0.5/bin/clang++ with -gcc-toolchain flag. In this case, the stage1 clang++ by default will look for C and C++ headers from host system, judging your kernel version (first released in 2013), it is very likely host headers doesn't support C++14 or higher.

If my guess is correct, you will need to find a way to make your stage1 clang to use the correct (newer version) C++ standard library. In your CC env args, it looks like this library coming from your customized gcc 9.3 (by the -gcc-toolchain) flag. I am not very familiar with this part so you have to do your own research.

In Fuchsia's Clang toolchain, we uses libc++ by default so we didn't encounter this issue.

[mikaelholmen]

Yes I see from other compilation errors (I only pasted first one above) I see that it uses headers from the host, as in In file included from /repo/uabelho/dev-main/compiler-rt/lib/memprof/tests/driver.cpp:9: In file included from /repo/uabelho/dev-main/runtimes/../third-party/unittest/googletest/include/gtest/gtest.h:65: In file included from /repo/uabelho/dev-main/runtimes/../third-party/unittest/googletest/include/gtest/gtest-death-test.h:43: In file included from /repo/uabelho/dev-main/runtimes/../third-party/unittest/googletest/include/gtest/internal/gtest-death-test-internal.h:47: /repo/uabelho/dev-main/runtimes/../third-party/unittest/googletest/include/gtest/gtest-matchers.h:734:55: error: too few template arguments for class template 'not_equal_to' 734 | : public ComparisonBase<NeMatcher<Rhs>, Rhs, std::not_equal_to<>> { | ^ /usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/bits/stl_function.h:213:12: note: template is declared here 212 | template<typename _Tp> | ~~~~~~~~~~~~~~~~~~~~~~ 213 | struct not_equal_to : public binary_function<_Tp, _Tp, bool> | ^ Have we raised the minimum required version? What is it?

[zeroomega]

@mikaelholmen By the way, the build failure you encountered is unrelated to this github issue. This issue is trying to identify the reason why LLVM need a customized gmock-matchers.h instead of using the vanilla googletest version.

It might be better to discuss the build failure in PR https://github.com/llvm/llvm-project/pull/65823 or file a new issue.

LLVM requires C++17. From patch D130689

gcc 4.8.5 (shown in your error message) only supports C++11.

[mikaelholmen]

It might be better to discuss the build failure in PR #65823 or file a new issue.

Yes for sure, that's where I intended to post this since it's related to that change. Not sure why I ended up here :(

[pogo59]

When using the upstream v1.14.0 gmock-matcher.h, the LLVM unit test will fail to build.

I put back the original gmock-matchers.h code, and tried to reproduce the build failure. MSVC 2019, gcc 7.5, and a pretty recent clang using libstdc++ all compiled the unittests successfully. I see from the gist that you were using libc++ so perhaps there is a difference there we could track down.

[zeroomega]

When using the upstream v1.14.0 gmock-matcher.h, the LLVM unit test will fail to build.

I put back the original gmock-matchers.h code, and tried to reproduce the build failure. MSVC 2019, gcc 7.5, and a pretty recent clang using libstdc++ all compiled the unittests successfully. I see from the gist that you were using libc++ so perhaps there is a difference there we could track down.

Emmm, I try to build llvm-unit-test with my host gcc12 but still ran into similar issue: https://gist.github.com/zeroomega/db368654e32b7eeeff1e1773ed3cef21

My cmake command is

$ cmake \              
 -GNinja -DLLVM_GTEST_USE_SHARD=False \
 -DCMAKE_BUILD_TYPE=Release \
 -DLLVM_ENABLE_PROJECTS=llvm \
 ../llvm

$ ninja check-llvm-unit

Host gcc is "gcc (Debian 12.2.0-14) 12.2.0".

I checkout the LLVM 2a07f0fd40fe3c8f50706d91407d829300ea1fdf with only googletest/googlemock/include/gmock/gmock-matchers.h replaced by vanilla googletest 1.14.0

[pogo59]

Okay. My gcc is obviously kind of old. I'll try again when I get a newer one, although it might take a while.

[pogo59]

Note to self: the three compilation failures in the gist are all in llvm/unittests/ADT: EnumeratedArrayTest.cpp, SequenceTest.cpp, STLExtrasTest.cpp.