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

[Nico Weber via libcxx-commits]

See post-commit review thread for f97936f (which introduced it) for the abi
break discussion.

On Thu, Feb 20, 2020 at 4:47 PM Louis Dionne via libcxx-commits <
libcxx-commits at lists.llvm.org> wrote:

> 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
>
> _______________________________________________
> libcxx-commits mailing list
> libcxx-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20200220/774c69eb/attachment-0001.html>