[clang] Update GoogleTest to v1.14.0 (PR #65823)
aganea wrote: I managed to repro. It is actually this, still open, issue: https://github.com/microsoft/STL/issues/1066 https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
zmodem wrote: I believe we're on the 10.0.19041.0 (2020-04) SDK and VS 16.6.1, running on Windows 10. The rpmalloc version is bc1923f which is probably old by now. I'll try with the latest rpmalloc version, and will also see if I can dig into the stl headers a bit, but I won't be able to get to this today. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
aganea wrote: @zmodem I haven’t been able to repro with the recipe posted on https://crbug.com/1487548. I am using latest rpmalloc (main branch), latest VS 2022, latest WinSDK. I am running on Win11, on a AMD Ryzen9 CPU. Are you running on WinServer2022? Are able to give more precision on the conditions for reproducing? https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
petrhosek wrote: We have seen occasional segfaults when using rpmalloc in other tools such as GN and Ninja which we haven't been able to get to the bottom of so we eventually switched to another allocator. It's possible that it's the same issue here, in which case perhaps we should reconsider our support for rpmalloc. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
zmodem wrote: Just tried it, but I didn't hit any asserts. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
aganea wrote: @zmodem Can you add `ENABLE_ASSERTS` on [this line](https://github.com/llvm/llvm-project/blob/720e3bacbd9fdba05645a4d621d43ad712c44df3/llvm/lib/Support/CMakeLists.txt#L107) and run the test again, see if that gives something interesting? https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
zmodem wrote: We're seeing unit test crashes on Windows when building with rpmalloc (via `LLVM_INTEGRATED_CRT_ALLOC`). It's crashing here: https://github.com/google/googletest/blob/v1.14.0/googletest/include/gtest/internal/gtest-port.h#L2092 + @aganea have you also hit this? (Our bug: https://crbug.com/1487548) https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
bjope wrote:
> > Ok, seems like the only problem is the MemProf unit test
> > compiler-rt/lib/memprof/tests/
> > So if I manually disable that I don't see any other failures with this
> > patch.
>
> I think any components that uses freshly built clang (stage1) could
> potentially run into this problem again, the correct way to solve it is to
> provide a compatible C++ Library to the freshly built clang when it built
> compiler-rt and related tests. For example, use -D with
> `RUNTIMES_${target}_CMAKE_CXX_FLAGS` and `RUNTIMES_${target}_CMAKE_CXX_FLAGS`
> to pass `--gcc-toolchain` (it might be deprecated and there is an updated
> flag for it) to the runtimes build. This will probably allow stage1 clang to
> use gcc's stdlib like your bootstrap clang.
Thanks! Yes, I think we hadn't considered that some things we set up for the
regular build wasn't forwarded to the compiler-rt test.
When setting
`-DRUNTIMES_${target}_COMPILER_RT_TEST_COMPILER_CFLAGS=--gcc-toolchain=`
the problem @mikaelholmen mentioned disappeared. I haven't tried adding it to
CMAKE_CXX_FLAGS, but maybe that is a better option.
https://github.com/llvm/llvm-project/pull/65823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
zeroomega wrote:
> Ok, seems like the only problem is the MemProf unit test
> compiler-rt/lib/memprof/tests/
>
> So if I manually disable that I don't see any other failures with this patch.
I think any components that uses freshly built clang (stage1) could potentially
run into this problem again, the correct way to solve it is to provide a
compatible C++ Library to the freshly built clang when it built compiler-rt and
related tests. For example, use -D with `RUNTIMES_${target}_CMAKE_CXX_FLAGS`
and `RUNTIMES_${target}_CMAKE_CXX_FLAGS` to pass `--gcc-toolchain` (it might be
deprecated and there is an updated flag for it) to the runtimes build. This
will probably allow stage1 clang to use gcc's stdlib like your bootstrap clang.
https://github.com/llvm/llvm-project/pull/65823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
mikaelholmen wrote: Ok, seems like the only problem is the MemProf unit test compiler-rt/lib/memprof/tests/ So if I manually disable that I don't see any other failures with this patch. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
mikaelholmen wrote:
I see compilation problems with this patch.
By mistake I originally posted about the problems in
https://github.com/llvm/llvm-project/issues/66268#issuecomment-1718791073
So the problem: On a RHEL7 server we see stuff like:
`
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:728:50:
error: too few template arguments for class template 'equal_to'
728 | : ComparisonBase, Rhs, std::equal_to<>>(rhs) {}
| ^
/usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/bits/stl_function.h:204:12:
note: template is declared here
203 | template
| ~~
204 | struct equal_to : public binary_function<_Tp, _Tp, bool>
|^
`
so it looks like it's when it's using the newly built clang to compile
compiler-rt/lib/memprof/tests/driver.cpp
it's using header files from the host and those headers are too old for the
updated GoogleTest.
If we don't build builtins (LLVM_BUILTIN_TARGETS) I don't see any problems.
https://github.com/llvm/llvm-project/pull/65823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
zeroomega wrote: Relanded as a866ce789eb99da4d7a486eeb60a53be6c75f4fd. I will monitor the bots and manually trigger clean build if linker error appears again. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
zeroomega wrote: I look through all the bot failures from my mail box and I think so far there are two issues: * `error: unused function 'operator<<'` issue, this is caused by a type matching bug I introduced in the roll and it will be addressed by: https://github.com/llvm/llvm-project/commit/c7e1a49754b69f548b9839a3fc1eac2d50c6b49d * Linker error and SEGV in runtimes bots (e.g. https://lab.llvm.org/buildbot/#/builders/240/builds/15001 andhttps://lab.llvm.org/buildbot/#/builders/239/builds/3686). These are caused by an incremental build bug in runtimes build. I can reproduce these errors locally on my machine if I do an incremental build after the roll and the issue is cleared if I did a clean build. I filed bug: https://github.com/llvm/llvm-project/issues/66272 I plan to reland the roll after it passes more local tests. For the incremental build bug, I plan to manually trigger a clean build once these runtimes bots pick up my change. Please let me know if you have questions and comments. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
zeroomega wrote: I looked into the "unused functions" issue shown up in bots like: https://lab.llvm.org/buildbot/#/builders/57/builds/29853 . The root cause is that GoogleTest changed the way it converts an arbitrary object into string. In the past, any object that cannot be printed to the string will eventually matched to function at https://github.com/llvm/llvm-project/blob/58d50b0cadafe118faf2e7d7bf738d2daa38bb73/third-party/unittest/googletest/include/gtest/gtest-printers.h#L264 . Then the call to the llvm_gtest::printable will use `OS << V.V;` to print the value to the stream, if the object overrides its `<<` operator, like https://github.com/llvm/llvm-project/blob/58d50b0cadafe118faf2e7d7bf738d2daa38bb73/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp#L457 , it will be used to convert the object to string. If not, a build error will happen. In GoogleTest v1.14.0, the object to string logic is re-written and for an arbitrary object, it will eventually matched to `RawBytesPrinter` defined at https://github.com/llvm/llvm-project/blob/54c1a9b20d89e85cd60d002c77b34c00f36520f4/third-party/unittest/googletest/include/gtest/gtest-printers.h#L290 . Therefore, the `operator<<` override won't be used and a unused function warnig(error) will be thrown. So deleting these `llvm::raw_ostream <<` functions won't cause any test failure but it will degrade the error message output since now the objects were printed as raw bytes. @bogner FYI. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
zeroomega wrote: > I'm seeing some unused functions in files that haven't been changed in years, > which I'm guessing come from this update - did the new googletest change how > custom output works? > > ``` > llvm/unittests/Support/JSONTest.cpp:490:27: error: unused function > 'operator<<' [-Werror,-Wunused-function] > inline llvm::raw_ostream <<(llvm::raw_ostream , > ^ > 1 error generated. > ``` > > I guess we can just delete this function, but I'm not sure if it means we've > degraded the error messages for these tests or anything like that. Yes new gtest changes how customized function work. These functions either need to be deleted or revised. I plan to revert this roll as I am seeing some linker error in bots: ``` FAILED: compiler-rt/lib/fuzzer/tests/FuzzerUtils-aarch64-Test /b/sanitizer-aarch64-linux-fuzzer/build/llvm_build0/runtimes/runtimes-bins/compiler-rt/lib/fuzzer/tests/FuzzerUtils-aarch64-Test cd /b/sanitizer-aarch64-linux-fuzzer/build/llvm_build0/runtimes/runtimes-bins/compiler-rt/lib/fuzzer/tests && /b/sanitizer-aarch64-linux-fuzzer/build/llvm_build0/./bin/clang++ FuzzedDataProviderTestObjects.FuzzedDataProviderUnittest.cpp.aarch64.o FuzzedDataProviderTestObjects.gtest-all.cc.aarch64.o -o /b/sanitizer-aarch64-linux-fuzzer/build/llvm_build0/runtimes/runtimes-bins/compiler-rt/lib/fuzzer/tests/./FuzzerUtils-aarch64-Test -fuse-ld=lld -Wl,--color-diagnostics -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta --driver-mode=g++ -lpthread -nostdlib++ -fno-exceptions /b/sanitizer-aarch64-linux-fuzzer/build/llvm_build0/runtimes/runtimes-bins/compiler-rt/lib/fuzzer/libcxx_fuzzer_aarch64/lib/libc++.a -march=armv8-a ld.lld: error: undefined symbol: testing::internal2::PrintBytesInObjectTo(unsigned char const*, unsigned long, std::__Fuzzer::basic_ostream>*) >>> referenced by gtest-printers.h:158 >>> (/b/sanitizer-aarch64-linux-fuzzer/build/llvm-project/runtimes/../third-party/unittest/googletest/include/gtest/gtest-printers.h:158) >>> >>> FuzzedDataProviderTestObjects.FuzzedDataProviderUnittest.cpp.aarch64.o:(std::__Fuzzer::basic_string>> std::__Fuzzer::char_traits, std::__Fuzzer::allocator> >>> testing::internal::FormatForComparisonFailureMessage>> >>> FuzzedDataProvider_ConsumeEnum_Test::TestBody()::Enum>(FuzzedDataProvider_ConsumeEnum_Test::TestBody()::Enum >>> const&, FuzzedDataProvider_ConsumeEnum_Test::TestBody()::Enum const&)) clang++: error: linker command failed with exit code 1 (use -v to see invocation) ``` It will need some time to debug. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
bogner wrote: I'm seeing some unused functions in files that haven't been changed in years, which I'm guessing come from this update - did the new googletest change how custom output works? ``` llvm/unittests/Support/JSONTest.cpp:490:27: error: unused function 'operator<<' [-Werror,-Wunused-function] inline llvm::raw_ostream <<(llvm::raw_ostream , ^ 1 error generated. ``` I guess we can just delete this function, but I'm not sure if it means we've degraded the error messages for these tests or anything like that. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
zeroomega wrote: First bot failure: https://lab.llvm.org/buildbot/#/builders/127/builds/55068 ``` [18/126] Generating SANITIZER_TEST_OBJECTS.sanitizer_lzw_test.cpp.x86_64.o FAILED: projects/compiler-rt/lib/sanitizer_common/tests/SANITIZER_TEST_OBJECTS.sanitizer_lzw_test.cpp.x86_64.o C:/b/slave/sanitizer-windows/build/stage1/projects/compiler-rt/lib/sanitizer_common/tests/SANITIZER_TEST_OBJECTS.sanitizer_lzw_test.cpp.x86_64.o cmd.exe /C "cd /D C:\b\slave\sanitizer-windows\build\stage1\projects\compiler-rt\lib\sanitizer_common\tests && C:\b\slave\sanitizer-windows\build\stage1\.\bin\clang.exe -DGTEST_NO_LLVM_SUPPORT=1 -DGTEST_HAS_RTTI=0 -IC:/b/slave/sanitizer-windows/llvm-project/llvm/../third-party/unittest/googletest/include -IC:/b/slave/sanitizer-windows/llvm-project/llvm/../third-party/unittest/googletest -Wno-deprecated-declarations -DGTEST_NO_LLVM_SUPPORT=1 -DGTEST_HAS_RTTI=0 -IC:/b/slave/sanitizer-windows/llvm-project/llvm/../third-party/unittest/googlemock/include -IC:/b/slave/sanitizer-windows/llvm-project/llvm/../third-party/unittest/googlemock -IC:/b/slave/sanitizer-windows/llvm-project/compiler-rt/include -IC:/b/slave/sanitizer-windows/llvm-project/compiler-rt/lib -IC:/b/slave/sanitizer-windows/llvm-project/compiler-rt/lib/sanitizer_common -DSANITIZER_COMMON_NO_REDEFINE_BUILTINS -fno-rtti -O2 -Werror=sign-compare -Wno-gnu-zero-variadic-macro-arguments -gline-tables-only -gcodeview -c -o SANITIZER_TEST_OBJECTS.sanitizer_lzw_test.cpp.x86_64.o C:/b/slave/sanitizer-windows/llvm-project/compiler-rt/lib/sanitizer_common/tests/sanitizer_lzw_test.cpp" C:/b/slave/sanitizer-windows/llvm-project/compiler-rt/lib/sanitizer_common/tests/sanitizer_lzw_test.cpp:22:10: error: no member named 'generate' in namespace 'std' 22 | std::generate(data.begin(), data.end(), gen); | ~^ ``` Probably caused by missing include header. Looking into it. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
https://github.com/zeroomega closed https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
pogo59 wrote: I'm happy with this, the bots will catch any environmental issues that you haven't found yet. LGTM. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
zeroomega wrote: I have locally tested this PR using gcc 12.2.0 on Linux x64 and MSVC from VS2019, both passed without issues. @pogo59 Do you have any concerns or suggestions before we merge this? https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
https://github.com/petrhosek approved this pull request. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
@@ -10,14 +10,10 @@ Cleaned up as follows: # Remove all the unnecessary files and directories $ rm -f CMakeLists.txt configure* Makefile* CHANGES CONTRIBUTORS README README.md .gitignore $ rm -rf build-aux make msvc scripts test docs -$ rm -f `find . -name \*\.pump` $ rm -f src/gmock_main.cc # Put the license in the consistent place for LLVM. $ mv LICENSE LICENSE.TXT Modified as follows: * Support for std::begin/std::end in gmock-matchers.h pogo59 wrote: I looked at that specific commit, seems fine. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
zeroomega wrote: I have addressed all review comments in the the latest amend commits. Please take a look. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
@@ -10,14 +10,10 @@ Cleaned up as follows: # Remove all the unnecessary files and directories $ rm -f CMakeLists.txt configure* Makefile* CHANGES CONTRIBUTORS README README.md .gitignore $ rm -rf build-aux make msvc scripts test docs -$ rm -f `find . -name \*\.pump` $ rm -f src/gmock_main.cc # Put the license in the consistent place for LLVM. $ mv LICENSE LICENSE.TXT Modified as follows: * Support for std::begin/std::end in gmock-matchers.h zeroomega wrote: I put the local changes into commented blocks in change in amend commit: 2fe51a37bad9fc6aa08f09b4b6f137034e2c8140 in this PR, please take a look. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
https://github.com/zeroomega resolved https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
@@ -1,22 +1,21 @@ LLVM notes -- -This directory contains Google Test 1.10.0, with all elements removed except for -the actual source code, to minimize the addition to the LLVM distribution. +This directory contains Google Test 1.14.0, +revision `f8d7d77c06936315286eb55f8de22cd23c188571`, with all elements removed +except for the actual source code, to minimize the addition to the LLVM +distribution. Cleaned up as follows: # Remove all the unnecessary files and directories $ rm -f CMakeLists.txt configure* Makefile* CHANGES CONTRIBUTORS README README.md .gitignore $ rm -rf build-aux cmake codegear m4 make msvc samples scripts test xcode docs -$ rm -f `find . -name \*\.pump` $ rm -f src/gtest_main.cc # Put the license in the consistent place for LLVM. $ mv LICENSE LICENSE.TXT Modified as follows: -* Added support for NetBSD, Minix and Haiku. * Added raw_os_ostream support to include/gtest/internal/custom/gtest-printers.h. * Added StringRef support to include/gtest/internal/custom/gtest-printers.h. zeroomega wrote: I added a description for the these two headers. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
@@ -10,14 +10,10 @@ Cleaned up as follows: # Remove all the unnecessary files and directories $ rm -f CMakeLists.txt configure* Makefile* CHANGES CONTRIBUTORS README README.md .gitignore $ rm -rf build-aux make msvc scripts test docs -$ rm -f `find . -name \*\.pump` $ rm -f src/gmock_main.cc # Put the license in the consistent place for LLVM. $ mv LICENSE LICENSE.TXT Modified as follows: * Support for std::begin/std::end in gmock-matchers.h pogo59 wrote: That seems odd, and worth following up with upstream googletest. In the meantime, I think it would be clearer to future maintainers if we did something to clearly identify the code blocks that were changed, and why. For example, `// LLVM local change to support std::begin/std::end` followed by the new code, and then the old code commented-out. That way diffs will provide clues, and merges/commits will have conflicts that someone will need to sort out properly. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
@@ -77,18 +77,18 @@ endif () target_include_directories(llvm_gtest PUBLIC $ $ - $ - $ + $ + $ zeroomega wrote: My checkout just stales. Thanks for pointing it out. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
@@ -77,18 +77,18 @@ endif ()
target_include_directories(llvm_gtest
PUBLIC $
$
- $
- $
+ $
+ $
PRIVATE googletest googlemock
)
add_subdirectory(UnitTestMain)
if (LLVM_INSTALL_GTEST)
install(TARGETS llvm_gtest llvm_gtest_main LLVMTestingSupport
LLVMTestingAnnotations
- ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}" COMPONENT llvm_gtest)
- install(DIRECTORY googletest/include/gtest/ DESTINATION
"${CMAKE_INSTALL_INCLUDEDIR}/llvm-gtest/gtest/" COMPONENT llvm_gtest)
- install(DIRECTORY googlemock/include/gmock/ DESTINATION
"${CMAKE_INSTALL_INCLUDEDIR}/llvm-gmock/gmock/" COMPONENT llvm_gtest)
+ ARCHIVE DESTINATION "lib${LLVM_LIBDIR_SUFFIX}" COMPONENT llvm_gtest)
+ install(DIRECTORY googletest/include/gtest/ DESTINATION
include/llvm-gtest/gtest/ COMPONENT llvm_gtest)
+ install(DIRECTORY googlemock/include/gmock/ DESTINATION
include/llvm-gmock/gmock/ COMPONENT llvm_gtest)
zeroomega wrote:
My checkout just stales. Thanks for pointing it out.
https://github.com/llvm/llvm-project/pull/65823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
@@ -1,22 +1,21 @@ LLVM notes -- -This directory contains Google Test 1.10.0, with all elements removed except for -the actual source code, to minimize the addition to the LLVM distribution. +This directory contains Google Test 1.14.0, +revision `f8d7d77c06936315286eb55f8de22cd23c188571`, with all elements removed zeroomega wrote: v1.14.0 is a valid tag and `f8d7d77c06936315286eb55f8de22cd23c188571` is the revision that it resolves to. The content matches the 1.14.0 source tar ball. The reason that I include a hash here is because the previous roll v1.10.0 doesn't have a valid tag and I had a bit hard time to find the correct revision for doing a diff to see what LLVM have patched. Since v1.14.0 is a valid tag, I think it is appropriate to remove the hash and just leave v1.14.0 in the doc. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
@@ -10,14 +10,10 @@ Cleaned up as follows: # Remove all the unnecessary files and directories $ rm -f CMakeLists.txt configure* Makefile* CHANGES CONTRIBUTORS README README.md .gitignore $ rm -rf build-aux make msvc scripts test docs -$ rm -f `find . -name \*\.pump` $ rm -f src/gmock_main.cc # Put the license in the consistent place for LLVM. $ mv LICENSE LICENSE.TXT Modified as follows: * Support for std::begin/std::end in gmock-matchers.h zeroomega wrote: Yes, gmock-matchers.h still have this part patched, see https://gist.github.com/zeroomega/fb24b1d1c4252b852200e15fda29384d#file-gtest_llvm_modification-patch-L251 . When locally test it, without the patch, LLVM unit tests will have build errors. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
https://github.com/pogo59 commented: Nice to see the LLVM intrusion into googletest proper is very minimized! IME, testing on Linux with both gcc and clang as build compilers is very helpful. With Windows/MSVC that covers the three main cases. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
https://github.com/pogo59 edited https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
@@ -1,22 +1,21 @@ LLVM notes -- -This directory contains Google Test 1.10.0, with all elements removed except for -the actual source code, to minimize the addition to the LLVM distribution. +This directory contains Google Test 1.14.0, +revision `f8d7d77c06936315286eb55f8de22cd23c188571`, with all elements removed pogo59 wrote: Parts of the googletest documentation claim to follow a "live at HEAD" philosophy, so a hash reference (here and in the other README) seems appropriate. Although using a tag, if there is one, would be preferable. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
@@ -1,22 +1,21 @@ LLVM notes -- -This directory contains Google Test 1.10.0, with all elements removed except for -the actual source code, to minimize the addition to the LLVM distribution. +This directory contains Google Test 1.14.0, +revision `f8d7d77c06936315286eb55f8de22cd23c188571`, with all elements removed +except for the actual source code, to minimize the addition to the LLVM +distribution. Cleaned up as follows: # Remove all the unnecessary files and directories $ rm -f CMakeLists.txt configure* Makefile* CHANGES CONTRIBUTORS README README.md .gitignore $ rm -rf build-aux cmake codegear m4 make msvc samples scripts test xcode docs -$ rm -f `find . -name \*\.pump` $ rm -f src/gtest_main.cc # Put the license in the consistent place for LLVM. $ mv LICENSE LICENSE.TXT Modified as follows: -* Added support for NetBSD, Minix and Haiku. * Added raw_os_ostream support to include/gtest/internal/custom/gtest-printers.h. * Added StringRef support to include/gtest/internal/custom/gtest-printers.h. pogo59 wrote: Also include/gtest/gtest-message.h and include/gtest/gtest-printers.h. Probably worth saying something like "(see uses of llvm_gtest)" to help the next person doing a roll. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
@@ -10,14 +10,10 @@ Cleaned up as follows: # Remove all the unnecessary files and directories $ rm -f CMakeLists.txt configure* Makefile* CHANGES CONTRIBUTORS README README.md .gitignore $ rm -rf build-aux make msvc scripts test docs -$ rm -f `find . -name \*\.pump` $ rm -f src/gmock_main.cc # Put the license in the consistent place for LLVM. $ mv LICENSE LICENSE.TXT Modified as follows: * Support for std::begin/std::end in gmock-matchers.h pogo59 wrote: Is this still true? If it is, it sounds like the kind of patch that ought to be made in upstream googletest. If not, please remove this line. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
@@ -1,22 +1,21 @@ LLVM notes -- -This directory contains Google Test 1.10.0, with all elements removed except for -the actual source code, to minimize the addition to the LLVM distribution. +This directory contains Google Test 1.14.0, +revision `f8d7d77c06936315286eb55f8de22cd23c188571`, with all elements removed petrhosek wrote: We don't include the revision in the `googlemock` `README.LLVM` file so I'd avoid it here also for consistency (or include it in the other one as well). https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
@@ -77,18 +77,18 @@ endif ()
target_include_directories(llvm_gtest
PUBLIC $
$
- $
- $
+ $
+ $
PRIVATE googletest googlemock
)
add_subdirectory(UnitTestMain)
if (LLVM_INSTALL_GTEST)
install(TARGETS llvm_gtest llvm_gtest_main LLVMTestingSupport
LLVMTestingAnnotations
- ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}" COMPONENT llvm_gtest)
- install(DIRECTORY googletest/include/gtest/ DESTINATION
"${CMAKE_INSTALL_INCLUDEDIR}/llvm-gtest/gtest/" COMPONENT llvm_gtest)
- install(DIRECTORY googlemock/include/gmock/ DESTINATION
"${CMAKE_INSTALL_INCLUDEDIR}/llvm-gmock/gmock/" COMPONENT llvm_gtest)
+ ARCHIVE DESTINATION "lib${LLVM_LIBDIR_SUFFIX}" COMPONENT llvm_gtest)
+ install(DIRECTORY googletest/include/gtest/ DESTINATION
include/llvm-gtest/gtest/ COMPONENT llvm_gtest)
+ install(DIRECTORY googlemock/include/gmock/ DESTINATION
include/llvm-gmock/gmock/ COMPONENT llvm_gtest)
petrhosek wrote:
This is undoing a change made recently in
https://github.com/llvm/llvm-project/commit/91b3ca39667b6341a8c1983a1467fae14b58318b.
Is that intentional or was your checkout just stale?
https://github.com/llvm/llvm-project/pull/65823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
@@ -77,18 +77,18 @@ endif () target_include_directories(llvm_gtest PUBLIC $ $ - $ - $ + $ + $ petrhosek wrote: This is undoing a change made recently in https://github.com/llvm/llvm-project/commit/91b3ca39667b6341a8c1983a1467fae14b58318b. Is that intentional or was your checkout just stale? https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
https://github.com/joker-eph approved this pull request. Agree with @MaskRay for the style: we should minimize the diff with upstream as much as possible! https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
https://github.com/MaskRay approved this pull request. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
MaskRay wrote: Thank you for the change! `ninja check-llvm check-clang check-clang-tools check-flang check-mlir check-lld check-polly` parses. > The GoogleTest code also uses a different code style (Google C++ style > instead of LLVM style). Shall I reformat the entire GoogleTest code base? I think we should just use the upstream style. https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
https://github.com/github-actions[bot] labeled https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
https://github.com/zeroomega review_requested https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
https://github.com/zeroomega review_requested https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
https://github.com/zeroomega review_requested https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update GoogleTest to v1.14.0 (PR #65823)
https://github.com/zeroomega review_requested https://github.com/llvm/llvm-project/pull/65823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
