<div dir="ltr">See post-commit review thread for f97936f (which introduced it) for the abi break discussion.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Feb 20, 2020 at 4:47 PM Louis Dionne via libcxx-commits <<a href="mailto:libcxx-commits@lists.llvm.org">libcxx-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">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.<br>
<br>
Louis<br>
<br>
> On Feb 19, 2020, at 12:02, Eric Fiselier via libcxx-commits <<a href="mailto:libcxx-commits@lists.llvm.org" target="_blank">libcxx-commits@lists.llvm.org</a>> wrote:<br>
> <br>
> <br>
> Author: Eric Fiselier<br>
> Date: 2020-02-19T12:02:06-05:00<br>
> New Revision: a829443cc7359ecf0f2de8f82519f511795675ec<br>
> <br>
> URL: <a href="https://github.com/llvm/llvm-project/commit/a829443cc7359ecf0f2de8f82519f511795675ec" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/a829443cc7359ecf0f2de8f82519f511795675ec</a><br>
> DIFF: <a href="https://github.com/llvm/llvm-project/commit/a829443cc7359ecf0f2de8f82519f511795675ec.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/a829443cc7359ecf0f2de8f82519f511795675ec.diff</a><br>
> <br>
> LOG: [libc++] Fix ABI break in __bit_reference.<br>
> <br>
> The libc++ __bit_iterator type has weird ABI calling conventions as a<br>
> quirk<br>
> of the implementation. The const bit iterator is trivial, but the<br>
> non-const<br>
> bit iterator is not because it declares a user-defined copy constructor.<br>
> <br>
> Changing this now is an ABI break, so this test ensures that each type<br>
> is trivial/non-trivial as expected.<br>
> <br>
> The definition of 'non-trivial for the purposes of calls':<br>
> A type is considered non-trivial for the purposes of calls if:<br>
> * it has a non-trivial copy constructor, move constructor, or<br>
> destructor, or<br>
> * all of its copy and move constructors are deleted.<br>
> <br>
> Added: <br>
> libcxx/test/libcxx/containers/sequences/vector.bool/trivial_for_purposes_of_call.pass.cpp<br>
> <br>
> Modified: <br>
> libcxx/include/__bit_reference<br>
> <br>
> Removed: <br>
> <br>
> <br>
> <br>
> ################################################################################<br>
> diff --git a/libcxx/include/__bit_reference b/libcxx/include/__bit_reference<br>
> index 3d4da1cbb68a..4a2b82064b3c 100644<br>
> --- a/libcxx/include/__bit_reference<br>
> +++ b/libcxx/include/__bit_reference<br>
> @@ -1122,6 +1122,21 @@ public:<br>
> __bit_iterator(const __type_for_copy_to_const& __it) _NOEXCEPT<br>
> : __seg_(__it.__seg_), __ctz_(__it.__ctz_) {}<br>
> <br>
> + // The non-const __bit_iterator has historically had a non-trivial<br>
> + // copy constructor (as a quirk of its construction). We need to maintain<br>
> + // this for ABI purposes.<br>
> + using __type_for_abi_non_trivial_copy_ctor =<br>
> + _If<!_IsConst, __bit_iterator, struct __private_nat>;<br>
> +<br>
> + _LIBCPP_INLINE_VISIBILITY<br>
> + __bit_iterator(__type_for_abi_non_trivial_copy_ctor const& __it) _NOEXCEPT<br>
> + : __seg_(__it.__seg_), __ctz_(__it.__ctz_) {}<br>
> +<br>
> + // Always declare the copy assignment operator since the implicit declaration<br>
> + // is deprecated.<br>
> + _LIBCPP_INLINE_VISIBILITY<br>
> + __bit_iterator& operator=(__bit_iterator const&) = default;<br>
> +<br>
> _LIBCPP_INLINE_VISIBILITY reference operator*() const _NOEXCEPT<br>
> {return reference(__seg_, __storage_type(1) << __ctz_);}<br>
> <br>
> <br>
> 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<br>
> new file mode 100644<br>
> index 000000000000..7b0b5c427c6f<br>
> --- /dev/null<br>
> +++ b/libcxx/test/libcxx/containers/sequences/vector.bool/trivial_for_purposes_of_call.pass.cpp<br>
> @@ -0,0 +1,57 @@<br>
> +//===----------------------------------------------------------------------===//<br>
> +//<br>
> +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.<br>
> +// See <a href="https://llvm.org/LICENSE.txt" rel="noreferrer" target="_blank">https://llvm.org/LICENSE.txt</a> for license information.<br>
> +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception<br>
> +//<br>
> +//===----------------------------------------------------------------------===//<br>
> +<br>
> +// <vector><br>
> +<br>
> +// typedef ... iterator;<br>
> +// typedef ... const_iterator;<br>
> +<br>
> +// The libc++ __bit_iterator type has weird ABI calling conventions as a quirk<br>
> +// of the implementation. The const bit iterator is trivial, but the non-const<br>
> +// bit iterator is not because it declares a user-defined copy constructor.<br>
> +//<br>
> +// Changing this now is an ABI break, so this test ensures that each type<br>
> +// is trivial/non-trivial as expected.<br>
> +<br>
> +// The definition of 'non-trivial for the purposes of calls':<br>
> +// A type is considered non-trivial for the purposes of calls if:<br>
> +// * it has a non-trivial copy constructor, move constructor, or<br>
> +// destructor, or<br>
> +// * all of its copy and move constructors are deleted.<br>
> +<br>
> +// UNSUPPORTED: c++98, c++03<br>
> +<br>
> +#include <vector><br>
> +#include <cassert><br>
> +<br>
> +#include "test_macros.h"<br>
> +<br>
> +template <class T><br>
> +using IsTrivialForCall = std::integral_constant<bool,<br>
> + std::is_trivially_copy_constructible<T>::value &&<br>
> + std::is_trivially_move_constructible<T>::value &&<br>
> + std::is_trivially_destructible<T>::value<br>
> + // Ignore the all-deleted case, it shouldn't occur here.<br>
> + >;<br>
> +<br>
> +void test_const_iterator() {<br>
> + using It = std::vector<bool>::const_iterator;<br>
> + static_assert(IsTrivialForCall<It>::value, "");<br>
> +}<br>
> +<br>
> +void test_non_const_iterator() {<br>
> + using It = std::vector<bool>::iterator;<br>
> + static_assert(!IsTrivialForCall<It>::value, "");<br>
> +}<br>
> +<br>
> +int main(int, char**) {<br>
> + test_const_iterator();<br>
> + test_non_const_iterator();<br>
> +<br>
> + return 0;<br>
> +}<br>
> <br>
> <br>
> <br>
> _______________________________________________<br>
> libcxx-commits mailing list<br>
> <a href="mailto:libcxx-commits@lists.llvm.org" target="_blank">libcxx-commits@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits</a><br>
<br>
_______________________________________________<br>
libcxx-commits mailing list<br>
<a href="mailto:libcxx-commits@lists.llvm.org" target="_blank">libcxx-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits</a><br>
</blockquote></div>