This is a follow up to https://reviews.llvm.org/D144694.
Fixes https://github.com/llvm/llvm-project/issues/60977.
Details
- Reviewers
philnik Mordante fsb4000 ldionne - Group Reviewers
Restricted Project Restricted Project - Commits
- rG70248920fcd8: [libc++][test] Add '-Wdeprecated-copy', '-Wdeprecated-copy-dtor' warnings to…
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Guess I'll stop working on this ;)
libcxx/utils/libcxx/test/params.py | ||
---|---|---|
15 | I suggest using both -Wdeprecated-copy and -Wdeprecated-copy-dtor here (I'm not sure if GCC supports the longer -Wdeprecated-copy-with-user-provided-dtor, but the manual does include -Wdeprecated-copy-dtor.) |
libcxx/utils/libcxx/test/params.py | ||
---|---|---|
15 | Yes, GCC doesn't support -Wdeprecated-copy-with-user-provided-dtor I changed the warning flag. |
Thanks for working on this!
I assume this fixes https://github.com/llvm/llvm-project/issues/60977. If so can you mention it in the commit message?
libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp | ||
---|---|---|
30 | Can these be deleted instead? I assume the id was intentionally const qualified. | |
libcxx/test/support/counting_predicates.h | ||
24 | Are they used? Copying the count_ seems wrong. The same for copies of other stateful parts in the tests. |
libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp | ||
---|---|---|
30 | Without my changes: /home/libcxx-builder/.buildkite-agent/builds/99763c758770-1/llvm-project/libcxx-ci/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp:29:3: error: definition of implicit copy constructor for 'Base' is deprecated because it has a user-provided destructor [-Werror,-Wdeprecated-copy-with-user-provided-dtor] ~Base() { shared_buff[(*cur_idx)++] = id; } ^ /home/libcxx-builder/.buildkite-agent/builds/99763c758770-1/llvm-project/libcxx-ci/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp:32:8: note: in implicit copy constructor for 'Base' first required here struct A : Base { ^ /home/libcxx-builder/.buildkite-agent/builds/99763c758770-1/llvm-project/libcxx-ci/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp:53:8: note: in implicit move constructor for 'A' first required here func(A(shared_buf, &cur_idx), std::unique_ptr<B>(new B(shared_buf, &cur_idx)), If I add =default: /home/libcxx-builder/.buildkite-agent/builds/c072959aa3b7-1/llvm-project/libcxx-ci/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp:30:9: error: explicitly defaulted copy assignment operator is implicitly deleted [-Werror,-Wdefaulted-function-deleted] Base& operator=(const Base&) = default; ^ /home/libcxx-builder/.buildkite-agent/builds/c072959aa3b7-1/llvm-project/libcxx-ci/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp:25:14: note: copy assignment operator of 'Base' is implicitly deleted because field 'id' is of const-qualified type 'const char' const char id; ^ /home/libcxx-builder/.buildkite-agent/builds/c072959aa3b7-1/llvm-project/libcxx-ci/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp:30:34: note: replace 'default' with 'delete' Base& operator=(const Base&) = default; ^~~~~~~ delete 1 error generated. If I add =delete /home/libcxx-builder/.buildkite-agent/builds/ab7607b38669-1/llvm-project/libcxx-ci/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp:34:8: error: call to deleted constructor of 'Base' struct A : Base { ^ /home/libcxx-builder/.buildkite-agent/builds/ab7607b38669-1/llvm-project/libcxx-ci/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp:55:8: note: in implicit copy constructor for 'A' first required here func(A(shared_buf, &cur_idx), std::unique_ptr<B>(new B(shared_buf, &cur_idx)), ^ /home/libcxx-builder/.buildkite-agent/builds/ab7607b38669-1/llvm-project/libcxx-ci/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp:29:3: note: 'Base' has been explicitly marked deleted here Base(const Base&) = delete; ^ /home/libcxx-builder/.buildkite-agent/builds/ab7607b38669-1/llvm-project/libcxx-ci/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp:42:8: error: call to deleted constructor of 'Base' struct C : Base { ^ /home/libcxx-builder/.buildkite-agent/builds/ab7607b38669-1/llvm-project/libcxx-ci/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp:56:8: note: in implicit copy constructor for 'C' first required here C(shared_buf, &cur_idx)); ^ /home/libcxx-builder/.buildkite-agent/builds/ab7607b38669-1/llvm-project/libcxx-ci/libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp:29:3: note: 'Base' has been explicitly marked deleted here Base(const Base&) = delete; ^ 2 errors generated. | |
30 | one more thing. I run the test this way: C:\Dev\STL\llvm-project\build>python bin\llvm-lit.py ..\libcxx\test\libcxx\memory\trivial_abi\unique_ptr_destruction_order.pass.cpp -v llvm-lit.py: C:\Dev\STL\llvm-project\libcxx\utils\libcxx\test\config.py:19: note: (llvm-libc++-shared-clangcl.cfg.in) Using %{cxx} substitution: ''C:/Program Files/Microsoft Visual Studio/2022/Preview/VC/Tools/Llvm/x64/bin/clang-cl.exe'' llvm-lit.py: C:\Dev\STL\llvm-project\libcxx\utils\libcxx\test\config.py:19: note: (llvm-libc++-shared-clangcl.cfg.in) Using %{flags} substitution: '--driver-mode=g++ --target=x86_64-pc-windows-msvc' llvm-lit.py: C:\Dev\STL\llvm-project\libcxx\utils\libcxx\test\config.py:19: note: (llvm-libc++-shared-clangcl.cfg.in) Using %{compile_flags} substitution: '-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS -DNOMINMAX -std=c++2b -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Wdeprecated-copy -Wdeprecated-copy-dtor -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -DTEST_WINDOWS_DLL' llvm-lit.py: C:\Dev\STL\llvm-project\libcxx\utils\libcxx\test\config.py:19: note: (llvm-libc++-shared-clangcl.cfg.in) Using %{link_flags} substitution: '-llibc++experimental -nostdlib -L %{lib} -lc++ -lmsvcrt -lmsvcprt -loldnames' llvm-lit.py: C:\Dev\STL\llvm-project\libcxx\utils\libcxx\test\config.py:19: note: (llvm-libc++-shared-clangcl.cfg.in) Using %{exec} substitution: '%{executor} --execdir %T --env PATH=%{lib} -- ' llvm-lit.py: C:\Dev\STL\llvm-project\libcxx\utils\libcxx\test\config.py:19: note: (llvm-libc++-shared-clangcl.cfg.in) All available features: no-filesystem, buildhost=win32, clang-15.0, target=x86_64-pc-windows-msvc, stdlib=libc++, stdlib=llvm-libc++, locale.fr_CA.ISO8859-1, -faligned-allocation, locale.zh_CN.UTF-8, locale.fr_FR.UTF-8, buildhost=windows, clang-15, clang-15.0.1, -fsized-deallocation, locale.en_US.UTF-8, clang, executor-has-no-bash, libcpp-abi-version=1, msvc-19.36, win32-broken-utf8-wchar-ctype, diagnose-if-support, windows, fdelayed-template-parsing, c++2b, thread-safety, objective-c++, c++experimental, windows-dll, long_tests, msvc-19, has-fblocks, locale.ja_JP.UTF-8, verify-support, locale.ru_RU.UTF-8, has-fconstexpr-steps, msvc -- Testing: 1 tests, 1 workers -- PASS: llvm-libc++-shared-clangcl.cfg.in :: libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp (1 of 1) Testing Time: 5.96s Passed: 1 | |
libcxx/test/support/counting_predicates.h | ||
24 | I saw the warnings but I will invistigate that. |
libcxx/test/support/counting_predicates.h | ||
---|---|---|
24 | Yes, they called. C:/Dev/STL/llvm-project/libcxx/test/support\counting_predicates.h:25:5: error: definition of implicit copy constructor for 'unary_counting_predicate<bool (*)(int), int>' is deprecated because it has a user-provided destructor [-Werror,-Wdeprecated-copy-with-user-provided-dtor] ~unary_counting_predicate() {} ^ C:\Dev\STL\llvm-project\libcxx\test\std\utilities\function.objects\refwrap\refwrap.helpers\ref_2.pass.cpp:60:22: note: in implicit copy constructor for 'unary_counting_predicate<bool (*)(int), int>' first required here assert(call_pred(cp)); ^ |
libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp | ||
---|---|---|
30 | I can add Base(const Base&) = default; Base& operator=(const Base&) = delete; It seems it works: https://gcc.godbolt.org/z/6nEGfffEj |
restore const char in test\libcxx\memory\trivial_abi\unique_ptr_destruction_order.pass.cpp
libcxx/test/support/counting_predicates.h | ||
---|---|---|
24 | About CDeleter: In file included from C:\Dev\STL\llvm-project\libcxx\test\std\utilities\smartptr\unique.ptr\unique.ptr.class\unique.ptr.asgn\move.pass.cpp:28: C:/Dev/STL/llvm-project/libcxx/test/support/deleter_types.h:169:24: error: definition of implicit copy assignment operator for 'CDeleter<A>' is deprecated because it has a user-provided destructor [-Werror,-Wdeprecated-copy-with-user-provided-dtor] TEST_CONSTEXPR_CXX23 ~CDeleter() { ^ C:/Dev/STL/llvm-project/build/include/c++/v1\__memory/unique_ptr.h:232:21: note: in implicit copy assignment operator for 'CDeleter<A>' first required here __ptr_.second() = _VSTD::forward<deleter_type>(__u.get_deleter()); ^ C:\Dev\STL\llvm-project\libcxx\test\std\utilities\smartptr\unique.ptr\unique.ptr.class\unique.ptr.asgn\move.pass.cpp:76:8: note: in instantiation of member function 'std::unique_ptr<A, CDeleter<A> &>::operator=' requested here s2 = std::move(s1); ^ C:\Dev\STL\llvm-project\libcxx\test\std\utilities\smartptr\unique.ptr\unique.ptr.class\unique.ptr.asgn\move.pass.cpp:133:5: note: in instantiation of function template specialization 'test_basic<false>' requested here test_basic</*IsArray*/ false>(); ^ In file included from C:\Dev\STL\llvm-project\libcxx\test\std\utilities\smartptr\unique.ptr\unique.ptr.class\unique.ptr.asgn\move.pass.cpp:28: C:/Dev/STL/llvm-project/libcxx/test/support/deleter_types.h:194:24: error: definition of implicit copy assignment operator for 'CDeleter<A[]>' is deprecated because it has a user-provided destructor [-Werror,-Wdeprecated-copy-with-user-provided-dtor] TEST_CONSTEXPR_CXX23 ~CDeleter() { ^ C:/Dev/STL/llvm-project/build/include/c++/v1\__memory/unique_ptr.h:434:21: note: in implicit copy assignment operator for 'CDeleter<A[]>' first required here __ptr_.second() = _VSTD::forward<deleter_type>(__u.get_deleter()); ^ C:\Dev\STL\llvm-project\libcxx\test\std\utilities\smartptr\unique.ptr\unique.ptr.class\unique.ptr.asgn\move.pass.cpp:76:8: note: in instantiation of member function 'std::unique_ptr<A[], CDeleter<A[]> &>::operator=' requested here s2 = std::move(s1); ^ C:\Dev\STL\llvm-project\libcxx\test\std\utilities\smartptr\unique.ptr\unique.ptr.class\unique.ptr.asgn\move.pass.cpp:137:5: note: in instantiation of function template specialization 'test_basic<true>' requested here test_basic</*IsArray*/ true>(); ^ 2 errors generated. error: command failed with exit status: 1 |
libcxx/test/support/nasty_containers.h | ||
---|---|---|
46–48 | Maybe just remove the destructor instead? If it breaks a test ignore the suggestion. |
Based on the discussion, I don't think any of the comments are blocking. I'll ship this if CI is green, everything in this patch seems reasonable to me and it fixes a real issue for MSVC's STL usage of the test suite.
Can these be deleted instead? I assume the id was intentionally const qualified.