Message ID | CABKRkggMfkUGNkTagojwUKK79P9q+9oC4LhDEEwz0wD11TxT5w@mail.gmail.com |
---|---|
State | New |
Headers | show |
Series | [libstd++,PR92156] | expand |
Fixes all this. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92156 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91630 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90415 On Fri, Apr 17, 2020 at 10:45 PM kamlesh kumar <kamleshbhalui@gmail.com> wrote: > > This patch corrects the requirement of 4,5 and 6th constructor > As per https://en.cppreference.com/w/cpp/utility/any/any. > > ChangeLog: > 2020-04-17 Kamlesh Kumar <kamleshbhalui@gmail.com> > > PR libstdc++/92156 > * include/std/any (ans::any(_ValueType &&):: Remove is_constructible. > (any::any(in_place_type_t<_ValueType>, _Args&&...)): Use decay_t. > (any::any(in_place_type_t<_ValueType>,initializer_list<_Up>, _Args&&...)): > Use decay_t. > * testsuite/20_util/any/misc/92156.cc: New Test. > > diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any > index 6b7e68f0e63..fb212eb2231 100644 > --- a/libstdc++-v3/include/std/any > +++ b/libstdc++-v3/include/std/any > @@ -178,30 +178,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > /// Construct with a copy of @p __value as the contained object. > template <typename _ValueType, typename _Tp = _Decay<_ValueType>, > typename _Mgr = _Manager<_Tp>, > - __any_constructible_t<_Tp, _ValueType&&> = true, > - enable_if_t<!__is_in_place_type<_Tp>::value, bool> = true> > + enable_if_t<is_copy_constructible<_Tp>::value && > + !__is_in_place_type<_Tp>::value, bool> = true> > any(_ValueType&& __value) > : _M_manager(&_Mgr::_S_manage) > { > _Mgr::_S_create(_M_storage, std::forward<_ValueType>(__value)); > } > > - /// Construct with a copy of @p __value as the contained object. > - template <typename _ValueType, typename _Tp = _Decay<_ValueType>, > - typename _Mgr = _Manager<_Tp>, > - enable_if_t<__and_v<is_copy_constructible<_Tp>, > - __not_<is_constructible<_Tp, _ValueType&&>>, > - __not_<__is_in_place_type<_Tp>>>, > - bool> = false> > - any(_ValueType&& __value) > - : _M_manager(&_Mgr::_S_manage) > - { > - _Mgr::_S_create(_M_storage, __value); > - } > - > /// Construct with an object created from @p __args as the contained object. > template <typename _ValueType, typename... _Args, > - typename _Tp = _Decay<_ValueType>, > + typename _Tp = decay_t<_ValueType>, > typename _Mgr = _Manager<_Tp>, > __any_constructible_t<_Tp, _Args&&...> = false> > explicit > @@ -214,7 +201,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > /// Construct with an object created from @p __il and @p __args as > /// the contained object. > template <typename _ValueType, typename _Up, typename... _Args, > - typename _Tp = _Decay<_ValueType>, > + typename _Tp = decay_t<_ValueType>, > typename _Mgr = _Manager<_Tp>, > __any_constructible_t<_Tp, initializer_list<_Up>, > _Args&&...> = false> > diff --git a/libstdc++-v3/testsuite/20_util/any/misc/92156.cc b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc > new file mode 100644 > index 00000000000..c4f1ed55aee > --- /dev/null > +++ b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc > @@ -0,0 +1,34 @@ > +// { dg-options "-std=gnu++17" } > +// { dg-do compile } > + > +// Copyright (C) 2014-2020 Free Software Foundation, Inc. > +// > +// This file is part of the GNU ISO C++ Library. This library is free > +// software; you can redistribute it and/or modify it under the > +// terms of the GNU General Public License as published by the > +// Free Software Foundation; either version 3, or (at your option) > +// any later version. > + > +// This library is distributed in the hope that it will be useful, > +// but WITHOUT ANY WARRANTY; without even the implied warranty of > +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +// GNU General Public License for more details. > + > +// You should have received a copy of the GNU General Public License along > +// with this library; see the file COPYING3. If not see > +// <http://www.gnu.org/licenses/>. > + > +#include <any> > +#include <utility> > +#include <tuple> > + > +int main() { > + auto a = std::any(std::in_place_type<std::any>, 5); > + auto b = std::any(std::in_place_type<std::any>, {1}); > + std::any p = std::pair<std::any, std::any>(1, 1); > + (void)p; > + std::any t = std::tuple<std::any>(1); > + (void)t; > + return 0; > +} > + > > Regtested on X86_64-linux. > > Thanks, >
On Fri, Apr 17, 2020, 10:59 PM kamlesh kumar <kamleshbhalui@gmail.com> wrote: > Fixes all this. > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92156 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91630 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90415 > > On Fri, Apr 17, 2020 at 10:45 PM kamlesh kumar <kamleshbhalui@gmail.com> > wrote: > > > > This patch corrects the requirement of 4,5 and 6th constructor > > As per https://en.cppreference.com/w/cpp/utility/any/any. > > > > ChangeLog: > > 2020-04-17 Kamlesh Kumar <kamleshbhalui@gmail.com> > > > > PR libstdc++/92156 > > * include/std/any (ans::any(_ValueType &&):: Remove > is_constructible. > > (any::any(in_place_type_t<_ValueType>, _Args&&...)): Use decay_t. > > (any::any(in_place_type_t<_ValueType>,initializer_list<_Up>, > _Args&&...)): > > Use decay_t. > > * testsuite/20_util/any/misc/92156.cc: New Test. > > > > diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any > > index 6b7e68f0e63..fb212eb2231 100644 > > --- a/libstdc++-v3/include/std/any > > +++ b/libstdc++-v3/include/std/any > > @@ -178,30 +178,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > /// Construct with a copy of @p __value as the contained object. > > template <typename _ValueType, typename _Tp = _Decay<_ValueType>, > > typename _Mgr = _Manager<_Tp>, > > - __any_constructible_t<_Tp, _ValueType&&> = true, > > - enable_if_t<!__is_in_place_type<_Tp>::value, bool> = true> > > + enable_if_t<is_copy_constructible<_Tp>::value && > > + !__is_in_place_type<_Tp>::value, bool> = true> > > any(_ValueType&& __value) > > : _M_manager(&_Mgr::_S_manage) > > { > > _Mgr::_S_create(_M_storage, std::forward<_ValueType>(__value)); > > } > > > > - /// Construct with a copy of @p __value as the contained object. > > - template <typename _ValueType, typename _Tp = _Decay<_ValueType>, > > - typename _Mgr = _Manager<_Tp>, > > - enable_if_t<__and_v<is_copy_constructible<_Tp>, > > - __not_<is_constructible<_Tp, _ValueType&&>>, > > - __not_<__is_in_place_type<_Tp>>>, > > - bool> = false> > > - any(_ValueType&& __value) > > - : _M_manager(&_Mgr::_S_manage) > > - { > > - _Mgr::_S_create(_M_storage, __value); > > - } > > - > > /// Construct with an object created from @p __args as the > contained object. > > template <typename _ValueType, typename... _Args, > > - typename _Tp = _Decay<_ValueType>, > > + typename _Tp = decay_t<_ValueType>, > > typename _Mgr = _Manager<_Tp>, > > __any_constructible_t<_Tp, _Args&&...> = false> > > explicit > > @@ -214,7 +201,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > /// Construct with an object created from @p __il and @p __args as > > /// the contained object. > > template <typename _ValueType, typename _Up, typename... _Args, > > - typename _Tp = _Decay<_ValueType>, > > + typename _Tp = decay_t<_ValueType>, > > typename _Mgr = _Manager<_Tp>, > > __any_constructible_t<_Tp, initializer_list<_Up>, > > _Args&&...> = false> > > diff --git a/libstdc++-v3/testsuite/20_util/any/misc/92156.cc > b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc > > new file mode 100644 > > index 00000000000..c4f1ed55aee > > --- /dev/null > > +++ b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc > > @@ -0,0 +1,34 @@ > > +// { dg-options "-std=gnu++17" } > > +// { dg-do compile } > > + > > +// Copyright (C) 2014-2020 Free Software Foundation, Inc. > > +// > > +// This file is part of the GNU ISO C++ Library. This library is free > > +// software; you can redistribute it and/or modify it under the > > +// terms of the GNU General Public License as published by the > > +// Free Software Foundation; either version 3, or (at your option) > > +// any later version. > > + > > +// This library is distributed in the hope that it will be useful, > > +// but WITHOUT ANY WARRANTY; without even the implied warranty of > > +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +// GNU General Public License for more details. > > + > > +// You should have received a copy of the GNU General Public License > along > > +// with this library; see the file COPYING3. If not see > > +// <http://www.gnu.org/licenses/>. > > + > > +#include <any> > > +#include <utility> > > +#include <tuple> > > + > > +int main() { > > + auto a = std::any(std::in_place_type<std::any>, 5); > > + auto b = std::any(std::in_place_type<std::any>, {1}); > > + std::any p = std::pair<std::any, std::any>(1, 1); > > + (void)p; > > + std::any t = std::tuple<std::any>(1); > > + (void)t; > > + return 0; > > +} > > + > > > > Regtested on X86_64-linux. > > > > Thanks, > > >
On Sat, 18 Apr 2020 at 03:35, kamlesh kumar via Libstdc++ <libstdc++@gcc.gnu.org> wrote: > > On Fri, Apr 17, 2020, 10:59 PM kamlesh kumar <kamleshbhalui@gmail.com> > wrote: > > > Fixes all this. > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92156 > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91630 > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90415 > > > > On Fri, Apr 17, 2020 at 10:45 PM kamlesh kumar <kamleshbhalui@gmail.com> > > wrote: > > > > > > This patch corrects the requirement of 4,5 and 6th constructor > > > As per https://en.cppreference.com/w/cpp/utility/any/any. The patch looks correct to me. We have some old cruft there, like the overload your patch removes, it was there to support copy-only types, but LWG issues axed that bit. This constructor indeed should not check is_constructible, because it'll end up instantiating this constructor itself, and compute its constraints, and instantiate itself. The in_place constructor doesn't have that problem, because it won't instantiate itself.
On Mon, 20 Apr 2020 at 21:09, Ville Voutilainen <ville.voutilainen@gmail.com> wrote: > > On Sat, 18 Apr 2020 at 03:35, kamlesh kumar via Libstdc++ > <libstdc++@gcc.gnu.org> wrote: > > > > On Fri, Apr 17, 2020, 10:59 PM kamlesh kumar <kamleshbhalui@gmail.com> > > wrote: > > > > > Fixes all this. > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92156 > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91630 > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90415 > > > > > > On Fri, Apr 17, 2020 at 10:45 PM kamlesh kumar <kamleshbhalui@gmail.com> > > > wrote: > > > > > > > > This patch corrects the requirement of 4,5 and 6th constructor > > > > As per https://en.cppreference.com/w/cpp/utility/any/any. > > The patch looks correct to me. We have some old cruft there, like the > overload your patch removes, it was > there to support copy-only types, but LWG issues axed that bit. This > constructor indeed should not check is_constructible, > because it'll end up instantiating this constructor itself, and > compute its constraints, and instantiate itself. > The in_place constructor doesn't have that problem, because it won't > instantiate itself. ..except the change from _Decay to decay_t looks wrong. _Decay also checks the non-sameness with any. That change shouldn't be made.
Thank you for reviewing. without _Decay to decay_t in the constructor which takes inplace_type_t, cases like this fails auto a = std::any(std::in_place_type<std::any>, 5); for these constructors, standard does not say anything about not-sameness checks with any. https://en.cppreference.com/w/cpp/utility/any/any. ./kamlesh On Mon, Apr 20, 2020 at 11:54 PM Ville Voutilainen <ville.voutilainen@gmail.com> wrote: > > On Mon, 20 Apr 2020 at 21:09, Ville Voutilainen > <ville.voutilainen@gmail.com> wrote: > > > > On Sat, 18 Apr 2020 at 03:35, kamlesh kumar via Libstdc++ > > <libstdc++@gcc.gnu.org> wrote: > > > > > > On Fri, Apr 17, 2020, 10:59 PM kamlesh kumar <kamleshbhalui@gmail.com> > > > wrote: > > > > > > > Fixes all this. > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92156 > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91630 > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90415 > > > > > > > > On Fri, Apr 17, 2020 at 10:45 PM kamlesh kumar <kamleshbhalui@gmail.com> > > > > wrote: > > > > > > > > > > This patch corrects the requirement of 4,5 and 6th constructor > > > > > As per https://en.cppreference.com/w/cpp/utility/any/any. > > > > The patch looks correct to me. We have some old cruft there, like the > > overload your patch removes, it was > > there to support copy-only types, but LWG issues axed that bit. This > > constructor indeed should not check is_constructible, > > because it'll end up instantiating this constructor itself, and > > compute its constraints, and instantiate itself. > > The in_place constructor doesn't have that problem, because it won't > > instantiate itself. > > ..except the change from _Decay to decay_t looks wrong. _Decay also > checks the non-sameness with > any. That change shouldn't be made.
On Tue, 21 Apr 2020 at 04:10, kamlesh kumar <kamleshbhalui@gmail.com> wrote: > > Thank you for reviewing. > without _Decay to decay_t in the constructor which takes inplace_type_t, > cases like this fails > auto a = std::any(std::in_place_type<std::any>, 5); > > for these constructors, standard does not say anything about > not-sameness checks with any. > https://en.cppreference.com/w/cpp/utility/any/any. Well, sure. Thus: - the in_place constructor should not _Decay - the constructor from T should _Decay - the assignment from a T should _Decay - emplace should not _Decay These bugs are not regressions, so presumably they can wait for stage1.
On Tue, 21 Apr 2020 at 09:11, Ville Voutilainen <ville.voutilainen@gmail.com> wrote: > > On Tue, 21 Apr 2020 at 04:10, kamlesh kumar <kamleshbhalui@gmail.com> wrote: > > > > Thank you for reviewing. > > without _Decay to decay_t in the constructor which takes inplace_type_t, > > cases like this fails > > auto a = std::any(std::in_place_type<std::any>, 5); > > > > for these constructors, standard does not say anything about > > not-sameness checks with any. > > https://en.cppreference.com/w/cpp/utility/any/any. > > Well, sure. Thus: > - the in_place constructor should not _Decay > - the constructor from T should _Decay > - the assignment from a T should _Decay > - emplace should not _Decay > > These bugs are not regressions, so presumably they can wait for stage1. ..except that two of them are. :) Anyhoo, the non-any handling needs to be retained for the T-constructor and the T-assignment, and removing it from emplace is not a regression but should be eventually done.
Added the fix for emplace. diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any index 6b7e68f0e63..f35d90e548d 100644 --- a/libstdc++-v3/include/std/any +++ b/libstdc++-v3/include/std/any @@ -178,30 +178,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// Construct with a copy of @p __value as the contained object. template <typename _ValueType, typename _Tp = _Decay<_ValueType>, typename _Mgr = _Manager<_Tp>, - __any_constructible_t<_Tp, _ValueType&&> = true, - enable_if_t<!__is_in_place_type<_Tp>::value, bool> = true> + enable_if_t<is_copy_constructible<_Tp>::value && + !__is_in_place_type<_Tp>::value, bool> = true> any(_ValueType&& __value) : _M_manager(&_Mgr::_S_manage) { _Mgr::_S_create(_M_storage, std::forward<_ValueType>(__value)); } - /// Construct with a copy of @p __value as the contained object. - template <typename _ValueType, typename _Tp = _Decay<_ValueType>, - typename _Mgr = _Manager<_Tp>, - enable_if_t<__and_v<is_copy_constructible<_Tp>, - __not_<is_constructible<_Tp, _ValueType&&>>, - __not_<__is_in_place_type<_Tp>>>, - bool> = false> - any(_ValueType&& __value) - : _M_manager(&_Mgr::_S_manage) - { - _Mgr::_S_create(_M_storage, __value); - } - /// Construct with an object created from @p __args as the contained object. template <typename _ValueType, typename... _Args, - typename _Tp = _Decay<_ValueType>, + typename _Tp = decay_t<_ValueType>, typename _Mgr = _Manager<_Tp>, __any_constructible_t<_Tp, _Args&&...> = false> explicit @@ -214,7 +201,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// Construct with an object created from @p __il and @p __args as /// the contained object. template <typename _ValueType, typename _Up, typename... _Args, - typename _Tp = _Decay<_ValueType>, + typename _Tp = decay_t<_ValueType>, typename _Mgr = _Manager<_Tp>, __any_constructible_t<_Tp, initializer_list<_Up>, _Args&&...> = false> @@ -267,31 +254,34 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } /// Emplace with an object created from @p __args as the contained object. - template <typename _ValueType, typename... _Args> - typename __any_constructible<_Decay<_ValueType>&, - _Decay<_ValueType>, _Args&&...>::type + template <typename _ValueType, + typename... _Args, typename _Tp = decay_t<_ValueType>> + typename __any_constructible<_Tp&, + _Tp, _Args&&...>::type emplace(_Args&&... __args) { - __do_emplace<_Decay<_ValueType>>(std::forward<_Args>(__args)...); + __do_emplace<_Tp>(std::forward<_Args>(__args)...); any::_Arg __arg; this->_M_manager(any::_Op_access, this, &__arg); - return *static_cast<_Decay<_ValueType>*>(__arg._M_obj); + return *static_cast<_Tp*>(__arg._M_obj); } /// Emplace with an object created from @p __il and @p __args as /// the contained object. - template <typename _ValueType, typename _Up, typename... _Args> - typename __any_constructible<_Decay<_ValueType>&, - _Decay<_ValueType>, + template <typename _ValueType, + typename _Up, typename... _Args, + typename _Tp = decay_t<_ValueType>> + typename __any_constructible<_Tp&, + _Tp, initializer_list<_Up>, _Args&&...>::type emplace(initializer_list<_Up> __il, _Args&&... __args) { - __do_emplace<_Decay<_ValueType>, _Up>(__il, + __do_emplace<_Tp, _Up>(__il, std::forward<_Args>(__args)...); any::_Arg __arg; this->_M_manager(any::_Op_access, this, &__arg); - return *static_cast<_Decay<_ValueType>*>(__arg._M_obj); + return *static_cast<_Tp*>(__arg._M_obj); } // modifiers diff --git a/libstdc++-v3/testsuite/20_util/any/misc/92156.cc b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc new file mode 100644 index 00000000000..c4f1ed55aee --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc @@ -0,0 +1,34 @@ +// { dg-options "-std=gnu++17" } +// { dg-do compile } + +// Copyright (C) 2014-2020 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <any> +#include <utility> +#include <tuple> + +int main() { + auto a = std::any(std::in_place_type<std::any>, 5); + auto b = std::any(std::in_place_type<std::any>, {1}); + std::any p = std::pair<std::any, std::any>(1, 1); + (void)p; + std::any t = std::tuple<std::any>(1); + (void)t; + return 0; +} + thanks, On Tue, Apr 21, 2020 at 12:09 PM Ville Voutilainen < ville.voutilainen@gmail.com> wrote: > On Tue, 21 Apr 2020 at 09:11, Ville Voutilainen > <ville.voutilainen@gmail.com> wrote: > > > > On Tue, 21 Apr 2020 at 04:10, kamlesh kumar <kamleshbhalui@gmail.com> > wrote: > > > > > > Thank you for reviewing. > > > without _Decay to decay_t in the constructor which takes > inplace_type_t, > > > cases like this fails > > > auto a = std::any(std::in_place_type<std::any>, 5); > > > > > > for these constructors, standard does not say anything about > > > not-sameness checks with any. > > > https://en.cppreference.com/w/cpp/utility/any/any. > > > > Well, sure. Thus: > > - the in_place constructor should not _Decay > > - the constructor from T should _Decay > > - the assignment from a T should _Decay > > - emplace should not _Decay > > > > These bugs are not regressions, so presumably they can wait for stage1. > > ..except that two of them are. :) Anyhoo, the non-any handling needs > to be retained > for the T-constructor and the T-assignment, and removing it from > emplace is not a regression > but should be eventually done. >
On Tue, 21 Apr 2020 at 11:29, kamlesh kumar <kamleshbhalui@gmail.com> wrote: > > Added the fix for emplace. > > diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any > index 6b7e68f0e63..f35d90e548d 100644 > --- a/libstdc++-v3/include/std/any > +++ b/libstdc++-v3/include/std/any > @@ -178,30 +178,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > /// Construct with a copy of @p __value as the contained object. > template <typename _ValueType, typename _Tp = _Decay<_ValueType>, While we're at it, we should rename _ValueType to _Tp and the decayed type to _VTp, so that it matches the standard's naming as close as possible, and thus removes the ongoing maintenance confusion about which is which. > +int main() { > + auto a = std::any(std::in_place_type<std::any>, 5); > + auto b = std::any(std::in_place_type<std::any>, {1}); > + std::any p = std::pair<std::any, std::any>(1, 1); > + (void)p; > + std::any t = std::tuple<std::any>(1); I think this sort of tests should VERIFY that the constructed any contains what we expect. Iow, do an any_cast and check that, for instance, a and b contain an any.
added VERIFY in test and changed the template parameter naming. diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any index 6b7e68f0e63..d350d0b2575 100644 --- a/libstdc++-v3/include/std/any +++ b/libstdc++-v3/include/std/any @@ -176,36 +176,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION typename __any_constructible<bool, _Tp, _Args...>::type; /// Construct with a copy of @p __value as the contained object. - template <typename _ValueType, typename _Tp = _Decay<_ValueType>, - typename _Mgr = _Manager<_Tp>, - __any_constructible_t<_Tp, _ValueType&&> = true, - enable_if_t<!__is_in_place_type<_Tp>::value, bool> = true> - any(_ValueType&& __value) + template <typename _Tp, typename _VTp = _Decay<_Tp>, + typename _Mgr = _Manager<_VTp>, + enable_if_t<is_copy_constructible<_VTp>::value && + !__is_in_place_type<_VTp>::value, bool> = true> + any(_Tp&& __value) : _M_manager(&_Mgr::_S_manage) { - _Mgr::_S_create(_M_storage, std::forward<_ValueType>(__value)); - } - - /// Construct with a copy of @p __value as the contained object. - template <typename _ValueType, typename _Tp = _Decay<_ValueType>, - typename _Mgr = _Manager<_Tp>, - enable_if_t<__and_v<is_copy_constructible<_Tp>, - __not_<is_constructible<_Tp, _ValueType&&>>, - __not_<__is_in_place_type<_Tp>>>, - bool> = false> - any(_ValueType&& __value) - : _M_manager(&_Mgr::_S_manage) - { - _Mgr::_S_create(_M_storage, __value); + _Mgr::_S_create(_M_storage, std::forward<_Tp>(__value)); } /// Construct with an object created from @p __args as the contained object. - template <typename _ValueType, typename... _Args, - typename _Tp = _Decay<_ValueType>, - typename _Mgr = _Manager<_Tp>, - __any_constructible_t<_Tp, _Args&&...> = false> + template <typename _Tp, typename... _Args, + typename _VTp = decay_t<_Tp>, + typename _Mgr = _Manager<_VTp>, + __any_constructible_t<_VTp, _Args&&...> = false> explicit - any(in_place_type_t<_ValueType>, _Args&&... __args) + any(in_place_type_t<_Tp>, _Args&&... __args) : _M_manager(&_Mgr::_S_manage) { _Mgr::_S_create(_M_storage, std::forward<_Args>(__args)...); @@ -213,13 +200,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// Construct with an object created from @p __il and @p __args as /// the contained object. - template <typename _ValueType, typename _Up, typename... _Args, - typename _Tp = _Decay<_ValueType>, - typename _Mgr = _Manager<_Tp>, - __any_constructible_t<_Tp, initializer_list<_Up>, + template <typename _Tp, typename _Up, typename... _Args, + typename _VTp = decay_t<_Tp>, + typename _Mgr = _Manager<_VTp>, + __any_constructible_t<_VTp, initializer_list<_Up>, _Args&&...> = false> explicit - any(in_place_type_t<_ValueType>, + any(in_place_type_t<_Tp>, initializer_list<_Up> __il, _Args&&... __args) : _M_manager(&_Mgr::_S_manage) { @@ -258,40 +245,43 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } /// Store a copy of @p __rhs as the contained object. - template<typename _ValueType> - enable_if_t<is_copy_constructible<_Decay<_ValueType>>::value, any&> - operator=(_ValueType&& __rhs) + template<typename _Tp> + enable_if_t<is_copy_constructible<_Decay<_Tp>>::value, any&> + operator=(_Tp&& __rhs) { - *this = any(std::forward<_ValueType>(__rhs)); + *this = any(std::forward<_Tp>(__rhs)); return *this; } /// Emplace with an object created from @p __args as the contained object. - template <typename _ValueType, typename... _Args> - typename __any_constructible<_Decay<_ValueType>&, - _Decay<_ValueType>, _Args&&...>::type + template <typename _Tp, + typename... _Args, typename _VTp = decay_t<_Tp>> + typename __any_constructible<_VTp&, + _VTp, _Args&&...>::type emplace(_Args&&... __args) { - __do_emplace<_Decay<_ValueType>>(std::forward<_Args>(__args)...); + __do_emplace<_VTp>(std::forward<_Args>(__args)...); any::_Arg __arg; this->_M_manager(any::_Op_access, this, &__arg); - return *static_cast<_Decay<_ValueType>*>(__arg._M_obj); + return *static_cast<_VTp*>(__arg._M_obj); } /// Emplace with an object created from @p __il and @p __args as /// the contained object. - template <typename _ValueType, typename _Up, typename... _Args> - typename __any_constructible<_Decay<_ValueType>&, - _Decay<_ValueType>, + template <typename _Tp, + typename _Up, typename... _Args, + typename _VTp = decay_t<_Tp>> + typename __any_constructible<_VTp&, + _VTp, initializer_list<_Up>, _Args&&...>::type emplace(initializer_list<_Up> __il, _Args&&... __args) { - __do_emplace<_Decay<_ValueType>, _Up>(__il, + __do_emplace<_VTp, _Up>(__il, std::forward<_Args>(__args)...); any::_Arg __arg; this->_M_manager(any::_Op_access, this, &__arg); - return *static_cast<_Decay<_ValueType>*>(__arg._M_obj); + return *static_cast<_VTp*>(__arg._M_obj); } // modifiers diff --git a/libstdc++-v3/testsuite/20_util/any/misc/92156.cc b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc new file mode 100644 index 00000000000..df6c9deff1b --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc @@ -0,0 +1,39 @@ +// { dg-options "-std=gnu++17" } +// { dg-do compile } + +// Copyright (C) 2014-2020 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <any> +#include <utility> +#include <tuple> +#include <type_traits> +#include <testsuite_hooks.h> + + +int main() { + auto a = std::any(std::in_place_type<std::any>, 5); + VERIFY(std::any_cast<int>(a) == 5); + auto b = std::any(std::in_place_type<std::any>, {1}); + VERIFY(std::any_cast<int>(b) == 1); + std::any p = std::pair<std::any, std::any>(1, 1); + VERIFY((std::any_cast<std::pair<int,int>>(p) == std::pair<int,int>(1,1))); + std::any t = std::tuple<std::any>(1); + VERIFY((std::any_cast<std::tuple<int>>(t) == std::tuple<int>(1))); + return 0; +} + thanks, On Tue, Apr 21, 2020 at 5:14 PM Ville Voutilainen <ville.voutilainen@gmail.com> wrote: > > On Tue, 21 Apr 2020 at 11:29, kamlesh kumar <kamleshbhalui@gmail.com> wrote: > > > > Added the fix for emplace. > > > > diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any > > index 6b7e68f0e63..f35d90e548d 100644 > > --- a/libstdc++-v3/include/std/any > > +++ b/libstdc++-v3/include/std/any > > @@ -178,30 +178,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > /// Construct with a copy of @p __value as the contained object. > > template <typename _ValueType, typename _Tp = _Decay<_ValueType>, > > While we're at it, we should rename _ValueType to _Tp and the decayed > type to _VTp, > so that it matches the standard's naming as close as possible, and > thus removes the ongoing > maintenance confusion about which is which. > > > +int main() { > > + auto a = std::any(std::in_place_type<std::any>, 5); > > + auto b = std::any(std::in_place_type<std::any>, {1}); > > + std::any p = std::pair<std::any, std::any>(1, 1); > > + (void)p; > > + std::any t = std::tuple<std::any>(1); > > I think this sort of tests should VERIFY that the constructed any > contains what we expect. > Iow, do an any_cast and check that, for instance, a and b contain an any.
On 21/04/20 20:58 +0530, kamlesh kumar via Libstdc++ wrote: >added VERIFY in test and changed the template parameter naming. > >diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any >index 6b7e68f0e63..d350d0b2575 100644 >--- a/libstdc++-v3/include/std/any >+++ b/libstdc++-v3/include/std/any >@@ -176,36 +176,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > typename __any_constructible<bool, _Tp, _Args...>::type; > > /// Construct with a copy of @p __value as the contained object. >- template <typename _ValueType, typename _Tp = _Decay<_ValueType>, >- typename _Mgr = _Manager<_Tp>, >- __any_constructible_t<_Tp, _ValueType&&> = true, >- enable_if_t<!__is_in_place_type<_Tp>::value, bool> = true> >- any(_ValueType&& __value) >+ template <typename _Tp, typename _VTp = _Decay<_Tp>, >+ typename _Mgr = _Manager<_VTp>, >+ enable_if_t<is_copy_constructible<_VTp>::value && >+ !__is_in_place_type<_VTp>::value, bool> = true> >+ any(_Tp&& __value) > : _M_manager(&_Mgr::_S_manage) > { >- _Mgr::_S_create(_M_storage, std::forward<_ValueType>(__value)); >- } >- >- /// Construct with a copy of @p __value as the contained object. >- template <typename _ValueType, typename _Tp = _Decay<_ValueType>, >- typename _Mgr = _Manager<_Tp>, >- enable_if_t<__and_v<is_copy_constructible<_Tp>, >- __not_<is_constructible<_Tp, _ValueType&&>>, >- __not_<__is_in_place_type<_Tp>>>, >- bool> = false> >- any(_ValueType&& __value) >- : _M_manager(&_Mgr::_S_manage) >- { >- _Mgr::_S_create(_M_storage, __value); >+ _Mgr::_S_create(_M_storage, std::forward<_Tp>(__value)); > } > > /// Construct with an object created from @p __args as the >contained object. Thanks for the patch, the changes look great ... but the patch is completely mangled by being pasted into the email body. You should send patches as attachments so gmail doesn't replace tabs with spaces and break lines (like the comment line above), preventing the patch from applying. But I can't use the patch anyway, as you don't have a copyright assignment for GCC. Would yo be willing to complete an assignment? See https://gcc.gnu.org/contribute.html#legal for details. Anyway, that aside ... >diff --git a/libstdc++-v3/testsuite/20_util/any/misc/92156.cc >b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc >new file mode 100644 >index 00000000000..df6c9deff1b >--- /dev/null >+++ b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc >@@ -0,0 +1,39 @@ >+// { dg-options "-std=gnu++17" } >+// { dg-do compile } This dg-do directive means the test is only compiled, not executed. It should also use an effective-target of c++17 to indicate it isn't valid for earlier standards: // { dg-do run { target c++17 } } >+// Copyright (C) 2014-2020 Free Software Foundation, Inc. As this is a new test its copyright date should be 2020 only. >+// >+// This file is part of the GNU ISO C++ Library. This library is free >+// software; you can redistribute it and/or modify it under the >+// terms of the GNU General Public License as published by the >+// Free Software Foundation; either version 3, or (at your option) >+// any later version. >+ >+// This library is distributed in the hope that it will be useful, >+// but WITHOUT ANY WARRANTY; without even the implied warranty of >+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >+// GNU General Public License for more details. >+ >+// You should have received a copy of the GNU General Public License along >+// with this library; see the file COPYING3. If not see >+// <http://www.gnu.org/licenses/>. >+ >+#include <any> >+#include <utility> >+#include <tuple> >+#include <type_traits> >+#include <testsuite_hooks.h> >+ >+ >+int main() { >+ auto a = std::any(std::in_place_type<std::any>, 5); >+ VERIFY(std::any_cast<int>(a) == 5); This test cannot possibly pass. It will throw an exception, because the type used in the any_cast is not the type of the contained value. >+ auto b = std::any(std::in_place_type<std::any>, {1}); >+ VERIFY(std::any_cast<int>(b) == 1); Same here. >+ std::any p = std::pair<std::any, std::any>(1, 1); >+ VERIFY((std::any_cast<std::pair<int,int>>(p) == std::pair<int,int>(1,1))); And here. >+ std::any t = std::tuple<std::any>(1); >+ VERIFY((std::any_cast<std::tuple<int>>(t) == std::tuple<int>(1))); And here. So if I change the test from { dg-do compile } to { dg-do run } I get: terminate called after throwing an instance of 'std::bad_any_cast' what(): bad any_cast FAIL: 20_util/any/misc/92156.cc execution test And I also get these because the line numbers have changed (which is easy to fix): FAIL: 20_util/any/misc/any_cast_neg.cc (test for errors, line 461) FAIL: 20_util/any/misc/any_cast_neg.cc (test for errors, line 457) FAIL: 20_util/any/misc/any_cast_neg.cc (test for errors, line 483) FAIL: 20_util/any/misc/any_cast_neg.cc (test for errors, line 501) FAIL: 20_util/any/misc/any_cast_neg.cc (test for errors, line 497) FAIL: 20_util/any/misc/any_cast_neg.cc (test for excess errors) Was the patch properly tested?
On 24/04/20 00:20 +0100, Jonathan Wakely wrote: >On 21/04/20 20:58 +0530, kamlesh kumar via Libstdc++ wrote: >>added VERIFY in test and changed the template parameter naming. >> >>diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any >>index 6b7e68f0e63..d350d0b2575 100644 >>--- a/libstdc++-v3/include/std/any >>+++ b/libstdc++-v3/include/std/any >>@@ -176,36 +176,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> typename __any_constructible<bool, _Tp, _Args...>::type; >> >> /// Construct with a copy of @p __value as the contained object. >>- template <typename _ValueType, typename _Tp = _Decay<_ValueType>, >>- typename _Mgr = _Manager<_Tp>, >>- __any_constructible_t<_Tp, _ValueType&&> = true, >>- enable_if_t<!__is_in_place_type<_Tp>::value, bool> = true> >>- any(_ValueType&& __value) >>+ template <typename _Tp, typename _VTp = _Decay<_Tp>, >>+ typename _Mgr = _Manager<_VTp>, >>+ enable_if_t<is_copy_constructible<_VTp>::value && >>+ !__is_in_place_type<_VTp>::value, bool> = true> >>+ any(_Tp&& __value) >> : _M_manager(&_Mgr::_S_manage) >> { >>- _Mgr::_S_create(_M_storage, std::forward<_ValueType>(__value)); >>- } >>- >>- /// Construct with a copy of @p __value as the contained object. >>- template <typename _ValueType, typename _Tp = _Decay<_ValueType>, >>- typename _Mgr = _Manager<_Tp>, >>- enable_if_t<__and_v<is_copy_constructible<_Tp>, >>- __not_<is_constructible<_Tp, _ValueType&&>>, >>- __not_<__is_in_place_type<_Tp>>>, >>- bool> = false> >>- any(_ValueType&& __value) >>- : _M_manager(&_Mgr::_S_manage) >>- { >>- _Mgr::_S_create(_M_storage, __value); >>+ _Mgr::_S_create(_M_storage, std::forward<_Tp>(__value)); >> } >> >> /// Construct with an object created from @p __args as the >>contained object. > >Thanks for the patch, the changes look great ... but the patch is >completely mangled by being pasted into the email body. > >You should send patches as attachments so gmail doesn't replace tabs >with spaces and break lines (like the comment line above), preventing >the patch from applying. > >But I can't use the patch anyway, as you don't have a copyright >assignment for GCC. Would yo be willing to complete an assignment? > >See https://gcc.gnu.org/contribute.html#legal for details. > >Anyway, that aside ... > >>diff --git a/libstdc++-v3/testsuite/20_util/any/misc/92156.cc >>b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc >>new file mode 100644 >>index 00000000000..df6c9deff1b >>--- /dev/null >>+++ b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc >>@@ -0,0 +1,39 @@ >>+// { dg-options "-std=gnu++17" } >>+// { dg-do compile } > >This dg-do directive means the test is only compiled, not executed. > >It should also use an effective-target of c++17 to indicate it isn't >valid for earlier standards: > >// { dg-do run { target c++17 } } > > >>+// Copyright (C) 2014-2020 Free Software Foundation, Inc. > >As this is a new test its copyright date should be 2020 only. > >>+// >>+// This file is part of the GNU ISO C++ Library. This library is free >>+// software; you can redistribute it and/or modify it under the >>+// terms of the GNU General Public License as published by the >>+// Free Software Foundation; either version 3, or (at your option) >>+// any later version. >>+ >>+// This library is distributed in the hope that it will be useful, >>+// but WITHOUT ANY WARRANTY; without even the implied warranty of >>+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>+// GNU General Public License for more details. >>+ >>+// You should have received a copy of the GNU General Public License along >>+// with this library; see the file COPYING3. If not see >>+// <http://www.gnu.org/licenses/>. >>+ >>+#include <any> >>+#include <utility> >>+#include <tuple> >>+#include <type_traits> >>+#include <testsuite_hooks.h> >>+ >>+ >>+int main() { >>+ auto a = std::any(std::in_place_type<std::any>, 5); >>+ VERIFY(std::any_cast<int>(a) == 5); > >This test cannot possibly pass. It will throw an exception, because >the type used in the any_cast is not the type of the contained value. > >>+ auto b = std::any(std::in_place_type<std::any>, {1}); >>+ VERIFY(std::any_cast<int>(b) == 1); > >Same here. > >>+ std::any p = std::pair<std::any, std::any>(1, 1); >>+ VERIFY((std::any_cast<std::pair<int,int>>(p) == std::pair<int,int>(1,1))); > >And here. > >>+ std::any t = std::tuple<std::any>(1); >>+ VERIFY((std::any_cast<std::tuple<int>>(t) == std::tuple<int>(1))); > >And here. > >So if I change the test from { dg-do compile } to { dg-do run } I get: > >terminate called after throwing an instance of 'std::bad_any_cast' > what(): bad any_cast >FAIL: 20_util/any/misc/92156.cc execution test > > >And I also get these because the line numbers have changed (which is >easy to fix): > >FAIL: 20_util/any/misc/any_cast_neg.cc (test for errors, line 461) >FAIL: 20_util/any/misc/any_cast_neg.cc (test for errors, line 457) >FAIL: 20_util/any/misc/any_cast_neg.cc (test for errors, line 483) >FAIL: 20_util/any/misc/any_cast_neg.cc (test for errors, line 501) >FAIL: 20_util/any/misc/any_cast_neg.cc (test for errors, line 497) >FAIL: 20_util/any/misc/any_cast_neg.cc (test for excess errors) > >Was the patch properly tested? > I've used your solution as part of my own fix, and also applied Ville's suggestions for renaming things and fixing the emplace members. This attached patch has been tested on powerpc64le-linux and has now been committed to master. I'll backport it to gcc-9 soon too. Thanks for identifying the problem and showing the solution! Please do consider completing a copyright assignment for GCC, so that we can use your patches in future.
diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any index 6b7e68f0e63..fb212eb2231 100644 --- a/libstdc++-v3/include/std/any +++ b/libstdc++-v3/include/std/any @@ -178,30 +178,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// Construct with a copy of @p __value as the contained object. template <typename _ValueType, typename _Tp = _Decay<_ValueType>, typename _Mgr = _Manager<_Tp>, - __any_constructible_t<_Tp, _ValueType&&> = true, - enable_if_t<!__is_in_place_type<_Tp>::value, bool> = true> + enable_if_t<is_copy_constructible<_Tp>::value && + !__is_in_place_type<_Tp>::value, bool> = true> any(_ValueType&& __value) : _M_manager(&_Mgr::_S_manage) { _Mgr::_S_create(_M_storage, std::forward<_ValueType>(__value)); } - /// Construct with a copy of @p __value as the contained object. - template <typename _ValueType, typename _Tp = _Decay<_ValueType>, - typename _Mgr = _Manager<_Tp>, - enable_if_t<__and_v<is_copy_constructible<_Tp>, - __not_<is_constructible<_Tp, _ValueType&&>>, - __not_<__is_in_place_type<_Tp>>>, - bool> = false> - any(_ValueType&& __value) - : _M_manager(&_Mgr::_S_manage) - { - _Mgr::_S_create(_M_storage, __value); - } - /// Construct with an object created from @p __args as the contained object. template <typename _ValueType, typename... _Args, - typename _Tp = _Decay<_ValueType>, + typename _Tp = decay_t<_ValueType>, typename _Mgr = _Manager<_Tp>, __any_constructible_t<_Tp, _Args&&...> = false> explicit @@ -214,7 +201,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// Construct with an object created from @p __il and @p __args as /// the contained object. template <typename _ValueType, typename _Up, typename... _Args, - typename _Tp = _Decay<_ValueType>, + typename _Tp = decay_t<_ValueType>, typename _Mgr = _Manager<_Tp>, __any_constructible_t<_Tp, initializer_list<_Up>, _Args&&...> = false> diff --git a/libstdc++-v3/testsuite/20_util/any/misc/92156.cc b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc new file mode 100644 index 00000000000..c4f1ed55aee --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc @@ -0,0 +1,34 @@ +// { dg-options "-std=gnu++17" } +// { dg-do compile } + +// Copyright (C) 2014-2020 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <any> +#include <utility> +#include <tuple> + +int main() { + auto a = std::any(std::in_place_type<std::any>, 5); + auto b = std::any(std::in_place_type<std::any>, {1}); + std::any p = std::pair<std::any, std::any>(1, 1); + (void)p; + std::any t = std::tuple<std::any>(1); + (void)t; + return 0; +} +