<div dir="ltr">I've pushed a fix + test upstream (not exactly a revert) in a829443cc7359ecf0f2de8f82519f511795675ec.<div><br></div><div>Let's give it 12 hours to settle and then merge into the release branch. </div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Feb 19, 2020 at 3:28 AM Hans Wennborg <<a href="mailto:hans@chromium.org">hans@chromium.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">On Tue, Feb 18, 2020 at 8:09 PM Eric Fiselier <<a href="mailto:eric@efcs.ca" target="_blank">eric@efcs.ca</a>> wrote:<br>
><br>
> Yes, we should ensure it goes into the 10.0 release.<br>
<br>
It's on my radar for 10.0. Just let me know when there's a revert I<br>
can back-port.<br>
<br>
><br>
> @Hans Wennborg it would be useful to know how you spotted this, maybe we can design tests to catch similar bugs?<br>
<br>
It turned out that in one part of Chromium, we were accidentally using<br>
two versions of libc++ at once, one that had your change, and one that<br>
didn't, causing it to crash when the caller of<br>
std::vector<bool>::begin() was using one version, and the callee using<br>
the other version.<br>
<br>
We've since fixed this, and I don't think it's a good approach for<br>
finding bugs :-)<br>
<br>
<br>
> On Tue., Feb. 18, 2020, 2:01 p.m. Shoaib Meenai, <<a href="mailto:smeenai@fb.com" target="_blank">smeenai@fb.com</a>> wrote:<br>
>><br>
>> I’m assuming that revert should be picked to 10.0 as well?<br>
>><br>
>><br>
>><br>
>> From: libcxx-commits <<a href="mailto:libcxx-commits-bounces@lists.llvm.org" target="_blank">libcxx-commits-bounces@lists.llvm.org</a>> on behalf of Eric Fiselier via libcxx-commits <<a href="mailto:libcxx-commits@lists.llvm.org" target="_blank">libcxx-commits@lists.llvm.org</a>><br>
>> Reply-To: Eric Fiselier <<a href="mailto:eric@efcs.ca" target="_blank">eric@efcs.ca</a>><br>
>> Date: Tuesday, February 18, 2020 at 10:51 AM<br>
>> To: Hans Wennborg <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>><br>
>> Cc: Eric Fiselier <<a href="mailto:llvmlistbot@llvm.org" target="_blank">llvmlistbot@llvm.org</a>>, "<a href="mailto:libcxx-commits@lists.llvm.org" target="_blank">libcxx-commits@lists.llvm.org</a>" <<a href="mailto:libcxx-commits@lists.llvm.org" target="_blank">libcxx-commits@lists.llvm.org</a>><br>
>> Subject: Re: [libcxx-commits] [libcxx] f97936f - [libc++] Cleanup and enable multiple warnings.<br>
>><br>
>><br>
>><br>
>> That certainly wasn't intentional, and I don't quite understand exactly why that happened.<br>
>><br>
>> I'll craft a revert for just the section responsible later today.<br>
>><br>
>><br>
>><br>
>> /Eric<br>
>><br>
>><br>
>><br>
>> On Tue., Feb. 18, 2020, 8:14 a.m. Hans Wennborg, <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>> wrote:<br>
>><br>
>> We noticed (don't ask how :p) that this seems to have changed the<br>
>> return type of std::__1::vector<bool, std::__1::allocator<bool><br>
>> >::begin()<br>
>><br>
>> At f97936fabd263e3b311e3b8e9ffca4920e4fcff0^1<br>
>><br>
>> $ cat /tmp/a.cc<br>
>> #include <vector><br>
>> void foobar(std::vector<bool>& v) {<br>
>> v.begin();<br>
>> }<br>
>> $ bin/clang -stdlib=libc++ -c /tmp/a.cc -S -emit-llvm -o - | grep<br>
>> define.*_ZNSt3__16vectorIbNS_9allocatorIbEEE5beginEv<br>
>> define linkonce_odr hidden void<br>
>> @_ZNSt3__16vectorIbNS_9allocatorIbEEE5beginEv(%"class.std::__1::__bit_iterator"*<br>
>> noalias sret %agg.result, %"class.std::__1::vector"* %this) #0 comdat<br>
>> align 2 {<br>
>><br>
>> At f97936fabd263e3b311e3b8e9ffca4920e4fcff0<br>
>><br>
>> define linkonce_odr hidden { i64*, i32 }<br>
>> @_ZNSt3__16vectorIbNS_9allocatorIbEEE5beginEv(%"class.std::__1::vector"*<br>
>> %this) #0 comdat align 2 {<br>
>><br>
>> I don't know whether there are any ABI guarantees around this but I<br>
>> thought it might be worth pointing out in any case.<br>
>><br>
>> On Fri, Dec 13, 2019 at 3:09 AM Eric Fiselier via libcxx-commits<br>
>> <<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: 2019-12-12T21:09:08-05:00<br>
>> > New Revision: f97936fabd263e3b311e3b8e9ffca4920e4fcff0<br>
>> ><br>
>> > URL: <a href="https://github.com/llvm/llvm-project/commit/f97936fabd263e3b311e3b8e9ffca4920e4fcff0" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/f97936fabd263e3b311e3b8e9ffca4920e4fcff0</a><br>
>> > DIFF: <a href="https://github.com/llvm/llvm-project/commit/f97936fabd263e3b311e3b8e9ffca4920e4fcff0.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/f97936fabd263e3b311e3b8e9ffca4920e4fcff0.diff</a><br>
>> ><br>
>> > LOG: [libc++] Cleanup and enable multiple warnings.<br>
>> ><br>
>> > Too many warnings are being disabled too quickly. Warnings are<br>
>> > important to keeping libc++ correct. This patch re-enables two<br>
>> > warnings: -Wconstant-evaluated and -Wdeprecated-copy.<br>
>> ><br>
>> > In future, all warnings disabled for the test suite should require<br>
>> > an attached bug. The bug should state the plan for re-enabling that<br>
>> > warning, or a strong case why it should remain disabled.<br>
>> ><br>
>> > Added:<br>
>> ><br>
>> ><br>
>> > Modified:<br>
>> > libcxx/include/__bit_reference<br>
>> > libcxx/include/__hash_table<br>
>> > libcxx/include/__tree<br>
>> > libcxx/include/ext/hash_map<br>
>> > libcxx/include/random<br>
>> > libcxx/include/valarray<br>
>> > libcxx/test/libcxx/containers/sequences/vector/asan_throw.pass.cpp<br>
>> > libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.fail.cpp<br>
>> > libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.pass.cpp<br>
>> > libcxx/test/std/utilities/optional/optional.object/optional.object.assign/copy.pass.cpp<br>
>> > libcxx/test/support/test_macros.h<br>
>> > libcxx/utils/libcxx/test/config.py<br>
>> ><br>
>> > Removed:<br>
>> ><br>
>> ><br>
>> ><br>
>> > ################################################################################<br>
>> > diff --git a/libcxx/include/__bit_reference b/libcxx/include/__bit_reference<br>
>> > index bbc159834242..3d4da1cbb68a 100644<br>
>> > --- a/libcxx/include/__bit_reference<br>
>> > +++ b/libcxx/include/__bit_reference<br>
>> > @@ -1114,8 +1114,12 @@ public:<br>
>> > #endif<br>
>> > {}<br>
>> ><br>
>> > + // avoid re-declaring a copy constructor for the non-const version.<br>
>> > + using __type_for_copy_to_const =<br>
>> > + _If<_IsConst, __bit_iterator<_Cp, false>, struct __private_nat>;<br>
>> > +<br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > - __bit_iterator(const __bit_iterator<_Cp, false>& __it) _NOEXCEPT<br>
>> > + __bit_iterator(const __type_for_copy_to_const& __it) _NOEXCEPT<br>
>> > : __seg_(__it.__seg_), __ctz_(__it.__ctz_) {}<br>
>> ><br>
>> > _LIBCPP_INLINE_VISIBILITY reference operator*() const _NOEXCEPT<br>
>> ><br>
>> > diff --git a/libcxx/include/__hash_table b/libcxx/include/__hash_table<br>
>> > index 0b953f58e99e..7ba3cebd0941 100644<br>
>> > --- a/libcxx/include/__hash_table<br>
>> > +++ b/libcxx/include/__hash_table<br>
>> > @@ -825,11 +825,13 @@ private:<br>
>> ><br>
>> > allocator_type& __na_;<br>
>> ><br>
>> > - __hash_node_destructor& operator=(const __hash_node_destructor&);<br>
>> > -<br>
>> > public:<br>
>> > bool __value_constructed;<br>
>> ><br>
>> > + __hash_node_destructor(__hash_node_destructor const&) = default;<br>
>> > + __hash_node_destructor& operator=(const __hash_node_destructor&) = delete;<br>
>> > +<br>
>> > +<br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > explicit __hash_node_destructor(allocator_type& __na,<br>
>> > bool __constructed = false) _NOEXCEPT<br>
>> ><br>
>> > diff --git a/libcxx/include/__tree b/libcxx/include/__tree<br>
>> > index 8421299c9bc8..8b6509cf91c0 100644<br>
>> > --- a/libcxx/include/__tree<br>
>> > +++ b/libcxx/include/__tree<br>
>> > @@ -775,11 +775,14 @@ private:<br>
>> > typedef __tree_node_types<pointer> _NodeTypes;<br>
>> > allocator_type& __na_;<br>
>> ><br>
>> > - __tree_node_destructor& operator=(const __tree_node_destructor&);<br>
>> ><br>
>> > public:<br>
>> > bool __value_constructed;<br>
>> ><br>
>> > +<br>
>> > + __tree_node_destructor(const __tree_node_destructor &) = default;<br>
>> > + __tree_node_destructor& operator=(const __tree_node_destructor&) = delete;<br>
>> > +<br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > explicit __tree_node_destructor(allocator_type& __na, bool __val = false) _NOEXCEPT<br>
>> > : __na_(__na),<br>
>> ><br>
>> > diff --git a/libcxx/include/ext/hash_map b/libcxx/include/ext/hash_map<br>
>> > index 09beaa607d83..7478d7410064 100644<br>
>> > --- a/libcxx/include/ext/hash_map<br>
>> > +++ b/libcxx/include/ext/hash_map<br>
>> > @@ -318,12 +318,13 @@ private:<br>
>> ><br>
>> > allocator_type& __na_;<br>
>> ><br>
>> > - __hash_map_node_destructor& operator=(const __hash_map_node_destructor&);<br>
>> > -<br>
>> > public:<br>
>> > bool __first_constructed;<br>
>> > bool __second_constructed;<br>
>> ><br>
>> > + __hash_map_node_destructor(__hash_map_node_destructor const&) = default;<br>
>> > + __hash_map_node_destructor& operator=(const __hash_map_node_destructor&) = delete;<br>
>> > +<br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > explicit __hash_map_node_destructor(allocator_type& __na)<br>
>> > : __na_(__na),<br>
>> ><br>
>> > diff --git a/libcxx/include/random b/libcxx/include/random<br>
>> > index 0cbc72dc1cfc..7c4054f7eea8 100644<br>
>> > --- a/libcxx/include/random<br>
>> > +++ b/libcxx/include/random<br>
>> > @@ -6105,6 +6105,7 @@ public:<br>
>> > template<class _UnaryOperation><br>
>> > param_type(size_t __nw, result_type __xmin, result_type __xmax,<br>
>> > _UnaryOperation __fw);<br>
>> > + param_type(param_type const&) = default;<br>
>> > param_type & operator=(const param_type& __rhs);<br>
>> ><br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > @@ -6428,6 +6429,7 @@ public:<br>
>> > template<class _UnaryOperation><br>
>> > param_type(size_t __nw, result_type __xmin, result_type __xmax,<br>
>> > _UnaryOperation __fw);<br>
>> > + param_type(param_type const&) = default;<br>
>> > param_type & operator=(const param_type& __rhs);<br>
>> ><br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> ><br>
>> > diff --git a/libcxx/include/valarray b/libcxx/include/valarray<br>
>> > index 8f6221ab3f39..c048a6d7e498 100644<br>
>> > --- a/libcxx/include/valarray<br>
>> > +++ b/libcxx/include/valarray<br>
>> > @@ -1256,6 +1256,8 @@ public:<br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > operator>>=(const _Expr& __v) const;<br>
>> ><br>
>> > + slice_array(slice_array const&) = default;<br>
>> > +<br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > const slice_array& operator=(const slice_array& __sa) const;<br>
>> ><br>
>> > @@ -1505,11 +1507,6 @@ public:<br>
>> ><br>
>> > #endif // _LIBCPP_CXX03_LANG<br>
>> ><br>
>> > -// gslice(const gslice&) = default;<br>
>> > -// gslice(gslice&&) = default;<br>
>> > -// gslice& operator=(const gslice&) = default;<br>
>> > -// gslice& operator=(gslice&&) = default;<br>
>> > -<br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > size_t start() const {return __1d_.size() ? __1d_[0] : 0;}<br>
>> ><br>
>> > @@ -1645,10 +1642,7 @@ public:<br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > void operator=(const value_type& __x) const;<br>
>> ><br>
>> > -// gslice_array(const gslice_array&) = default;<br>
>> > -// gslice_array(gslice_array&&) = default;<br>
>> > -// gslice_array& operator=(const gslice_array&) = default;<br>
>> > -// gslice_array& operator=(gslice_array&&) = default;<br>
>> > + gslice_array(const gslice_array&) = default;<br>
>> ><br>
>> > private:<br>
>> > gslice_array(const gslice& __gs, const valarray<value_type>& __v)<br>
>> > @@ -1977,17 +1971,14 @@ public:<br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > operator>>=(const _Expr& __v) const;<br>
>> ><br>
>> > + mask_array(const mask_array&) = default;<br>
>> > +<br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > const mask_array& operator=(const mask_array& __ma) const;<br>
>> ><br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > void operator=(const value_type& __x) const;<br>
>> ><br>
>> > -// mask_array(const mask_array&) = default;<br>
>> > -// mask_array(mask_array&&) = default;<br>
>> > -// mask_array& operator=(const mask_array&) = default;<br>
>> > -// mask_array& operator=(mask_array&&) = default;<br>
>> > -<br>
>> > private:<br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > mask_array(const valarray<bool>& __vb, const valarray<value_type>& __v)<br>
>> > @@ -2336,17 +2327,14 @@ public:<br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > operator>>=(const _Expr& __v) const;<br>
>> ><br>
>> > + indirect_array(const indirect_array&) = default;<br>
>> > +<br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > const indirect_array& operator=(const indirect_array& __ia) const;<br>
>> ><br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > void operator=(const value_type& __x) const;<br>
>> ><br>
>> > -// indirect_array(const indirect_array&) = default;<br>
>> > -// indirect_array(indirect_array&&) = default;<br>
>> > -// indirect_array& operator=(const indirect_array&) = default;<br>
>> > -// indirect_array& operator=(indirect_array&&) = default;<br>
>> > -<br>
>> > private:<br>
>> > _LIBCPP_INLINE_VISIBILITY<br>
>> > indirect_array(const valarray<size_t>& __ia, const valarray<value_type>& __v)<br>
>> ><br>
>> > diff --git a/libcxx/test/libcxx/containers/sequences/vector/asan_throw.pass.cpp b/libcxx/test/libcxx/containers/sequences/vector/asan_throw.pass.cpp<br>
>> > index 443a6f2e9243..235a3ffae14e 100644<br>
>> > --- a/libcxx/test/libcxx/containers/sequences/vector/asan_throw.pass.cpp<br>
>> > +++ b/libcxx/test/libcxx/containers/sequences/vector/asan_throw.pass.cpp<br>
>> > @@ -50,6 +50,7 @@ class ThrowOnCopy {<br>
>> > throw 0;<br>
>> > }<br>
>> > }<br>
>> > + ThrowOnCopy& operator=(ThrowOnCopy const&) = default;<br>
>> ><br>
>> > bool should_throw;<br>
>> > };<br>
>> ><br>
>> > diff --git a/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.fail.cpp b/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.fail.cpp<br>
>> > index c139233b8dae..2d2e07157624 100644<br>
>> > --- a/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.fail.cpp<br>
>> > +++ b/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.fail.cpp<br>
>> > @@ -24,6 +24,7 @@ int main(int, char**)<br>
>> > #else<br>
>> > // expected-error@+1 {{static_assert failed}}<br>
>> > static_assert(!std::is_constant_evaluated(), "");<br>
>> > + // expected-error@-1 0-1 {{'std::is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}<br>
>> > #endif<br>
>> > return 0;<br>
>> > }<br>
>> ><br>
>> > diff --git a/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.pass.cpp b/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.pass.cpp<br>
>> > index 0f9f98e91768..e4b8a7575b0b 100644<br>
>> > --- a/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.pass.cpp<br>
>> > +++ b/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.pass.cpp<br>
>> > @@ -23,6 +23,12 @@<br>
>> > #endif<br>
>> > #endif<br>
>> ><br>
>> > +// Disable the tautological constant evaluation warnings for this test,<br>
>> > +// because it's explicitly testing those cases.<br>
>> > +#if TEST_HAS_WARNING("-Wconstant-evaluated") && defined(__clang__)<br>
>> > +#pragma clang diagnostic ignored "-Wconstant-evaluated"<br>
>> > +#endif<br>
>> > +<br>
>> > template <bool> struct InTemplate {};<br>
>> ><br>
>> > int main(int, char**)<br>
>> ><br>
>> > diff --git a/libcxx/test/std/utilities/optional/optional.object/optional.object.assign/copy.pass.cpp b/libcxx/test/std/utilities/optional/optional.object/optional.object.assign/copy.pass.cpp<br>
>> > index a5ff10201c92..f14e20657d7b 100644<br>
>> > --- a/libcxx/test/std/utilities/optional/optional.object/optional.object.assign/copy.pass.cpp<br>
>> > +++ b/libcxx/test/std/utilities/optional/optional.object/optional.object.assign/copy.pass.cpp<br>
>> > @@ -30,6 +30,7 @@ struct X<br>
>> > if (throw_now)<br>
>> > TEST_THROW(6);<br>
>> > }<br>
>> > + X& operator=(X const&) = default;<br>
>> > };<br>
>> ><br>
>> > bool X::throw_now = false;<br>
>> ><br>
>> > diff --git a/libcxx/test/support/test_macros.h b/libcxx/test/support/test_macros.h<br>
>> > index f5fbedb55552..443165516b25 100644<br>
>> > --- a/libcxx/test/support/test_macros.h<br>
>> > +++ b/libcxx/test/support/test_macros.h<br>
>> > @@ -47,6 +47,12 @@<br>
>> > #define TEST_HAS_EXTENSION(X) 0<br>
>> > #endif<br>
>> ><br>
>> > +#ifdef __has_warning<br>
>> > +#define TEST_HAS_WARNING(X) __has_warning(X)<br>
>> > +#else<br>
>> > +#define TEST_HAS_WARNING(X) 0<br>
>> > +#endif<br>
>> > +<br>
>> > #ifdef __has_builtin<br>
>> > #define TEST_HAS_BUILTIN(X) __has_builtin(X)<br>
>> > #else<br>
>> ><br>
>> > diff --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py<br>
>> > index 403abe9ddfb8..e81571a7f1f0 100644<br>
>> > --- a/libcxx/utils/libcxx/test/config.py<br>
>> > +++ b/libcxx/utils/libcxx/test/config.py<br>
>> > @@ -917,8 +917,6 @@ def configure_warnings(self):<br>
>> > self.cxx.addWarningFlagIfSupported('-Wshadow')<br>
>> > self.cxx.addWarningFlagIfSupported('-Wno-unused-command-line-argument')<br>
>> > self.cxx.addWarningFlagIfSupported('-Wno-attributes')<br>
>> > - self.cxx.addWarningFlagIfSupported('-Wno-deprecated-copy')<br>
>> > - self.cxx.addWarningFlagIfSupported('-Wno-constant-evaluated')<br>
>> > self.cxx.addWarningFlagIfSupported('-Wno-pessimizing-move')<br>
>> > self.cxx.addWarningFlagIfSupported('-Wno-c++11-extensions')<br>
>> > self.cxx.addWarningFlagIfSupported('-Wno-user-defined-literals')<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>
</blockquote></div>