[libcxx-commits] [libcxx] f97936f - [libc++] Cleanup and enable multiple warnings.
Hans Wennborg via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Feb 20 06:34:23 PST 2020
Pushed as 7a18790ae2f4b92dc26b2be963e588c8837d0076. Thanks!
On Wed, Feb 19, 2020 at 6:03 PM Eric Fiselier <eric at efcs.ca> wrote:
>
> I've pushed a fix + test upstream (not exactly a revert) in a829443cc7359ecf0f2de8f82519f511795675ec.
>
> Let's give it 12 hours to settle and then merge into the release branch.
>
> On Wed, Feb 19, 2020 at 3:28 AM Hans Wennborg <hans at chromium.org> wrote:
>>
>> 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