[libcxx-commits] [libcxx] f97936f - [libc++] Cleanup and enable multiple warnings.

Eric Fiselier via libcxx-commits libcxx-commits at lists.llvm.org
Tue Feb 18 11:09:41 PST 2020


Yes, we should ensure it goes into the 10.0 release.

@Hans Wennborg <hans at chromium.org> it would be useful to know how you
spotted this, maybe we can design tests to catch similar 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
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_libcxx-2Dcommits&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=AjeYA_DyM418SS6GQzcbCydSuiosTXVPWZsgGadVQ_0&s=1kS_G3k3lZTrhD5rG3WDFYEepMkzQhxJvK0YUSOEIXM&e=>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20200218/9aa4b6be/attachment-0001.html>


More information about the libcxx-commits mailing list