[libc++] Make sure __bit_iterator does not use deprecated SMF generation

[• Quuxplusone]

Summary

This allows us to turn -Wdeprecated-copy back on, which a recent Clang
commit broke when the warning became more stringent.

[xbolva00]

Thanks for the fix

[• Quuxplusone]

You probably saw my Slack, but for the record, I think we want this:

public:
    _LIBCPP_INLINE_VISIBILITY __bit_iterator() _NOEXCEPT
#if _LIBCPP_STD_VER > 11
    : __seg_(nullptr), __ctz_(0)
#endif
    {}

    // When _IsConst=false, this is the copy constructor.
    // It is non-trivial. We maintain this for ABI purposes.
    _LIBCPP_INLINE_VISIBILITY
    __bit_iterator(const __bit_iterator<_Cp, false>& __it) _NOEXCEPT
        : __seg_(__it.__seg_), __ctz_(__it.__ctz_) {}

    // When _IsConst=false, we have a user-provided copy constructor,
    // so we must also provide a copy assignment operator because
    // the implicit generation of a defaulted one is deprecated.
    _LIBCPP_INLINE_VISIBILITY
    __bit_iterator& operator=(const _If<_IsConst, struct __private_nat, __bit_iterator>& __it) {
        __seg_ = __it.__seg_;
        __ctz_ = __it.__ctz_;
        return *this;
    }

    _LIBCPP_INLINE_VISIBILITY reference operator*() const _NOEXCEPT
        {return reference(__seg_, __storage_type(1) << __ctz_);}

[• Quuxplusone]

Previous (failing) buildkite run is in https://buildkite.com/llvm-project/libcxx-ci/builds/2759 . I'm about to update this PR with my suggested patch.

[• Quuxplusone]

Apply my suggested patch instead. Let's see what buildkite thinks of this. (I think it'll pass, but let's see.)

[curdeius]

Stupid question... but what's SMF?

[• Quuxplusone]

In D101183#2714519, @curdeius wrote:

Stupid question... but what's SMF?

https://quuxplusone.github.io/blog/2019/08/02/the-tough-guide-to-cpp-acronyms/#smf ;)

[ldionne]

libcxx/include/__bit_reference

1118

Maybe mention the obvious:

When _IsConst=true, the copy-constructor is the default compiler-generated one, which is trivial.

libcxx/include/__bit_reference

 
 public:
     _LIBCPP_INLINE_VISIBILITY __bit_iterator() _NOEXCEPT
 #if _LIBCPP_STD_VER > 11
     : __seg_(nullptr), __ctz_(0)
 #endif
     {}
 
-    // avoid re-declaring a copy constructor for the non-const version.
-    using __type_for_copy_to_const =
-      _If<_IsConst, __bit_iterator<_Cp, false>, struct __private_nat>;
-
+    // When _IsConst=false, this is the copy constructor.
+    // It is non-trivial. Making it trivial would break ABI.
+    // When _IsConst=true, this is a converting constructor;
+    // the copy and move constructors are implicitly generated
+    // and trivial.
     _LIBCPP_INLINE_VISIBILITY
-    __bit_iterator(const __type_for_copy_to_const& __it) _NOEXCEPT
+    __bit_iterator(const __bit_iterator<_Cp, false>& __it) _NOEXCEPT
         : __seg_(__it.__seg_), __ctz_(__it.__ctz_) {}
 
-    // The non-const __bit_iterator has historically had a non-trivial
-    // copy constructor (as a quirk of its construction). We need to maintain
-    // this for ABI purposes.
-    using __type_for_abi_non_trivial_copy_ctor =
-      _If<!_IsConst, __bit_iterator, struct __private_nat>;
-
+    // When _IsConst=false, we have a user-provided copy constructor,
+    // so we must also provide a copy assignment operator because
+    // the implicit generation of a defaulted one is deprecated.
+    // When _IsConst=true, the assignment operators are
+    // implicitly generated and trivial.
     _LIBCPP_INLINE_VISIBILITY
-    __bit_iterator(__type_for_abi_non_trivial_copy_ctor const& __it) _NOEXCEPT
-      : __seg_(__it.__seg_), __ctz_(__it.__ctz_) {}
-
-    // Always declare the copy assignment operator since the implicit declaration
-    // is deprecated.
-    _LIBCPP_INLINE_VISIBILITY
-    __bit_iterator& operator=(__bit_iterator const&) = default;
+    __bit_iterator& operator=(const _If<_IsConst, struct __private_nat, __bit_iterator>& __it) {
+        __seg_ = __it.__seg_;
+        __ctz_ = __it.__ctz_;
+        return *this;
+    }
 
     _LIBCPP_INLINE_VISIBILITY reference operator*() const _NOEXCEPT
         {return reference(__seg_, __storage_type(1) << __ctz_);}
 
     _LIBCPP_INLINE_VISIBILITY __bit_iterator& operator++()
     {
         if (__ctz_ != __bits_per_word-1)
             ++__ctz_;

libcxx/utils/libcxx/test/params.py

 #===----------------------------------------------------------------------===##
 #
 # Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 #===----------------------------------------------------------------------===##
 
 from libcxx.test.dsl import *
 from libcxx.test.features import _isMSVC
 
 _warningFlags = [
   '-Werror',
   '-Wall',
   '-Wextra',
-  '-Wno-deprecated-copy',
   '-Wshadow',
   '-Wundef',
   '-Wno-unused-command-line-argument',
   '-Wno-attributes',
   '-Wno-pessimizing-move',
   '-Wno-c++11-extensions',
   '-Wno-user-defined-literals',
   '-Wno-noexcept-type',