[libc++][test] Add '-Wdeprecated-copy', '-Wdeprecated-copy-dtor' warnings to the test suite

[ldionne]

Summary

This is a follow up to https://reviews.llvm.org/D144694.
Fixes https://github.com/llvm/llvm-project/issues/60977.

[fsb4000]

fix tests

[fsb4000]

fix more tests

[CaseyCarter]

Guess I'll stop working on this ;)

libcxx/utils/libcxx/test/params.py

54

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.)

[fsb4000]

libcxx/utils/libcxx/test/params.py

54

Yes, GCC doesn't support -Wdeprecated-copy-with-user-provided-dtor

I changed the warning flag.

[Mordante]

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 ↗

(On Diff #500528)

Can these be deleted instead? I assume the id was intentionally const qualified.

libcxx/test/support/counting_predicates.h

24 ↗

(On Diff #500528)

Are they used? Copying the count_ seems wrong.

The same for copies of other stateful parts in the tests.

[fsb4000]

libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp

30 ↗

(On Diff #500528)

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 ↗

(On Diff #500528)

one more thing.
On Windows I can write whatever in that test.
It passes without my changes, it passes with =default, it passes with =delete

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 ↗

(On Diff #500528)

I saw the warnings but I will invistigate that.

[fsb4000]

libcxx/test/support/counting_predicates.h

24 ↗

(On Diff #500528)

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));
                     ^

https://github.com/llvm/llvm-project/blob/bbaa79470423e63d075abef869ab4d5a203c51d3/libcxx/test/std/utilities/function.objects/refwrap/refwrap.helpers/ref_2.pass.cpp#L25

https://github.com/llvm/llvm-project/blob/bbaa79470423e63d075abef869ab4d5a203c51d3/libcxx/test/std/utilities/function.objects/refwrap/refwrap.helpers/ref_2.pass.cpp#L60

[fsb4000]

libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp

30 ↗

(On Diff #500528)

I can add

Base(const Base&) = default;
Base& operator=(const Base&) = delete;

It seems it works: https://gcc.godbolt.org/z/6nEGfffEj

[fsb4000]

restore const char in test\libcxx\memory\trivial_abi\unique_ptr_destruction_order.pass.cpp

[fsb4000]

libcxx/test/support/counting_predicates.h

24 ↗

(On Diff #500528)

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

[philnik]

libcxx/test/support/nasty_containers.h

44–46 ↗

(On Diff #500370)

Maybe just remove the destructor instead? If it breaks a test ignore the suggestion.

[ldionne]

[Github PR transition cleanup]

Commandeering to pick up.

[ldionne]

Rebase.

[ldionne]

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.

[ldionne]

Clang-format.

[ldionne]

More missing format.

[ldionne]

Rebase and fix C++11 test.

libcxx/utils/libcxx/test/params.py

_warningFlags = [
   # These warnings should be enabled in order to support the MSVC
   # team using the test suite; They enable the warnings below and
   # expect the test suite to be clean.
   '-Wsign-compare',
   '-Wunused-variable',
   '-Wunused-parameter',
   '-Wunreachable-code',
   '-Wno-unused-local-typedef',
+  '-Wdeprecated-copy-with-user-provided-dtor',
 ]
 
 _allStandards = ['c++03', 'c++11', 'c++14', 'c++17', 'c++20', 'c++2b']
 def getStdFlag(cfg, std):
   fallbacks = {
     'c++11': 'c++0x',
     'c++14': 'c++1y',
     'c++17': 'c++1z',