[llvm-branch-commits] [libcxx] 7a18790 - [libc++] Fix ABI break in __bit_reference.

[Hans Wennborg via llvm-branch-commits]

Author: Eric Fiselier
Date: 2020-02-20T15:32:05+01:00
New Revision: 7a18790ae2f4b92dc26b2be963e588c8837d0076

URL: https://github.com/llvm/llvm-project/commit/7a18790ae2f4b92dc26b2be963e588c8837d0076
DIFF: https://github.com/llvm/llvm-project/commit/7a18790ae2f4b92dc26b2be963e588c8837d0076.diff

LOG: [libc++] Fix ABI break in __bit_reference.

The libc++ __bit_iterator type has weird ABI calling conventions as a
quirk
of the implementation. The const bit iterator is trivial, but the
non-const
bit iterator is not because it declares a user-defined copy constructor.

Changing this now is an ABI break, so this test ensures that each type
is trivial/non-trivial as expected.

The definition of 'non-trivial for the purposes of calls':
  A type is considered non-trivial for the purposes of calls if:
      * it has a non-trivial copy constructor, move constructor, or
            destructor, or
	        * all of its copy and move constructors are deleted.

(cherry picked from commit a829443cc7359ecf0f2de8f82519f511795675ec)

Added: 
    libcxx/test/libcxx/containers/sequences/vector.bool/trivial_for_purposes_of_call.pass.cpp

Modified: 
    libcxx/include/__bit_reference

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__bit_reference b/libcxx/include/__bit_reference
index 3d4da1cbb68a..4a2b82064b3c 100644
--- a/libcxx/include/__bit_reference
+++ b/libcxx/include/__bit_reference
@@ -1122,6 +1122,21 @@ public:
     __bit_iterator(const __type_for_copy_to_const& __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>;
+
+    _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;
+
     _LIBCPP_INLINE_VISIBILITY reference operator*() const _NOEXCEPT
         {return reference(__seg_, __storage_type(1) << __ctz_);}
 

diff  --git a/libcxx/test/libcxx/containers/sequences/vector.bool/trivial_for_purposes_of_call.pass.cpp b/libcxx/test/libcxx/containers/sequences/vector.bool/trivial_for_purposes_of_call.pass.cpp
new file mode 100644
index 000000000000..7b0b5c427c6f
--- /dev/null
+++ b/libcxx/test/libcxx/containers/sequences/vector.bool/trivial_for_purposes_of_call.pass.cpp
@@ -0,0 +1,57 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <vector>
+
+// typedef ... iterator;
+// typedef ... const_iterator;
+
+// The libc++ __bit_iterator type has weird ABI calling conventions as a quirk
+// of the implementation. The const bit iterator is trivial, but the non-const
+// bit iterator is not because it declares a user-defined copy constructor.
+//
+// Changing this now is an ABI break, so this test ensures that each type
+// is trivial/non-trivial as expected.
+
+// The definition of 'non-trivial for the purposes of calls':
+//   A type is considered non-trivial for the purposes of calls if:
+//     * it has a non-trivial copy constructor, move constructor, or
+//       destructor, or
+//     * all of its copy and move constructors are deleted.
+
+// UNSUPPORTED: c++98, c++03
+
+#include <vector>
+#include <cassert>
+
+#include "test_macros.h"
+
+template <class T>
+using IsTrivialForCall = std::integral_constant<bool,
+  std::is_trivially_copy_constructible<T>::value &&
+  std::is_trivially_move_constructible<T>::value &&
+  std::is_trivially_destructible<T>::value
+  // Ignore the all-deleted case, it shouldn't occur here.
+  >;
+
+void test_const_iterator() {
+  using It = std::vector<bool>::const_iterator;
+  static_assert(IsTrivialForCall<It>::value, "");
+}
+
+void test_non_const_iterator() {
+  using It = std::vector<bool>::iterator;
+  static_assert(!IsTrivialForCall<It>::value, "");
+}
+
+int main(int, char**) {
+  test_const_iterator();
+  test_non_const_iterator();
+
+  return 0;
+}