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