[libcxx-commits] [libcxx] a829443 - [libc++] Fix ABI break in __bit_reference.

[Louis Dionne via libcxx-commits]

Can you please send out reviews for this sort of stuff? When was the ABI break introduced? I’m not sure I follow the commit message.

Louis

> On Feb 19, 2020, at 12:02, Eric Fiselier via libcxx-commits <libcxx-commits at lists.llvm.org> wrote:
> 
> 
> Author: Eric Fiselier
> Date: 2020-02-19T12:02:06-05:00
> New Revision: a829443cc7359ecf0f2de8f82519f511795675ec
> 
> URL: https://github.com/llvm/llvm-project/commit/a829443cc7359ecf0f2de8f82519f511795675ec
> DIFF: https://github.com/llvm/llvm-project/commit/a829443cc7359ecf0f2de8f82519f511795675ec.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.
> 
> 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;
> +}
> 
> 
> 
> _______________________________________________
> libcxx-commits mailing list
> libcxx-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits