subject:"\[PATCH\]\[libstd\+\+\]\[PR92156\]"

Re: [PATCH][libstd++][PR92156]

2020-04-23 Thread Jonathan Wakely via Gcc-patches


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::type;

   /// Construct with a copy of @p __value as the contained object.
-template ,
-  typename _Mgr = _Manager<_Tp>,
-  __any_constructible_t<_Tp, _ValueType&&> = true,
-  enable_if_t::value, bool> = true>
-  any(_ValueType&& __value)
+template ,
+  typename _Mgr = _Manager<_VTp>,
+  enable_if_t::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 _Mgr = _Manager<_Tp>,
-  enable_if_t<__and_v,
-  __not_>,
-  __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 000..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
+// .
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+int main() {
+auto a = std::any(std::in_place_type, 5);
+VERIFY(std::any_cast(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, {1});
+VERIFY(std::any_cast(b) == 1);


Same here.


+std::any p = std::pair(1, 1);
+VERIFY((std::any_cast>(p) == std::pair(1,1)));


And here.


+std::any t = std::tuple(1);
+VERIFY((std::any_cast>(t) == std::tuple(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.

Re: [PATCH][libstd++][PR92156]

2020-04-23 Thread Jonathan Wakely via Gcc-patches


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::type;

/// Construct with a copy of @p __value as the contained object.
-template ,
-  typename _Mgr = _Manager<_Tp>,
-  __any_constructible_t<_Tp, _ValueType&&> = true,
-  enable_if_t::value, bool> = true>
-  any(_ValueType&& __value)
+template ,
+  typename _Mgr = _Manager<_VTp>,
+  enable_if_t::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 _Mgr = _Manager<_Tp>,
-  enable_if_t<__and_v,
-  __not_>,
-  __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 000..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
+// .
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+int main() {
+auto a = std::any(std::in_place_type, 5);
+VERIFY(std::any_cast(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, {1});
+VERIFY(std::any_cast(b) == 1);


Same here.


+std::any p = std::pair(1, 1);
+VERIFY((std::any_cast>(p) == std::pair(1,1)));


And here.


+std::any t = std::tuple(1);
+VERIFY((std::any_cast>(t) == std::tuple(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?

Re: [PATCH][libstd++][PR92156]

2020-04-21 Thread kamlesh kumar via Gcc-patches

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::type;

 /// Construct with a copy of @p __value as the contained object.
-template ,
-  typename _Mgr = _Manager<_Tp>,
-  __any_constructible_t<_Tp, _ValueType&&> = true,
-  enable_if_t::value, bool> = true>
-  any(_ValueType&& __value)
+template ,
+  typename _Mgr = _Manager<_VTp>,
+  enable_if_t::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 _Mgr = _Manager<_Tp>,
-  enable_if_t<__and_v,
-  __not_>,
-  __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 _Mgr = _Manager<_Tp>,
-  __any_constructible_t<_Tp, _Args&&...> = false>
+template ,
+  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 _Mgr = _Manager<_Tp>,
-  __any_constructible_t<_Tp, initializer_list<_Up>,
+template ,
+  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
-  enable_if_t>::value, any&>
-  operator=(_ValueType&& __rhs)
+template
+  enable_if_t>::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 __any_constructible<_Decay<_ValueType>&,
-   _Decay<_ValueType>, _Args&&...>::type
+template >
+  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 __any_constructible<_Decay<_ValueType>&,
-   _Decay<_ValueType>,
+template >
+  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 000..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

Re: [PATCH][libstd++][PR92156]

2020-04-21 Thread Ville Voutilainen via Gcc-patches

On Tue, 21 Apr 2020 at 11:29, kamlesh kumar  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 ,

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, 5);
> +auto b = std::any(std::in_place_type, {1});
> +std::any p = std::pair(1, 1);
> +(void)p;
> +std::any t = std::tuple(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.

Re: [PATCH][libstd++][PR92156]

2020-04-21 Thread kamlesh kumar via Gcc-patches

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 _Mgr = _Manager<_Tp>,
-  __any_constructible_t<_Tp, _ValueType&&> = true,
-  enable_if_t::value, bool> = true>
+  enable_if_t::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 _Mgr = _Manager<_Tp>,
-  enable_if_t<__and_v,
-  __not_>,
-  __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 _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 _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 __any_constructible<_Decay<_ValueType>&,
-   _Decay<_ValueType>, _Args&&...>::type
+template >
+  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 __any_constructible<_Decay<_ValueType>&,
-   _Decay<_ValueType>,
+template >
+  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 000..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
+// .
+
+#include 
+#include 
+#include 
+
+int main() {
+auto a = std::any(std::in_place_type, 5);
+auto b = std::any(std::in_place_type, {1});
+std::any p = std::pair(1, 1);
+(void)p;
+std::any t = std::tuple(1);
+(void)t;
+return 0;
+}
+

thanks,


On Tue, Apr 21, 2020 at 12:09 PM Ville Voutilainen <
ville.voutilai...@gmail.com> wrote:

> On Tue, 21 Apr 2020 at 09:11, Ville Voutilainen
>  wrote:
> >
> > On Tue, 21 Apr 2020 at 04:10, kamlesh kumar 
> 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, 5);
> > >
> > > for these constructors, standard does not say anything about
> > > not-sameness checks with any.
> > >

Re: [PATCH][libstd++][PR92156]

2020-04-21 Thread Ville Voutilainen via Gcc-patches

On Tue, 21 Apr 2020 at 09:11, Ville Voutilainen
 wrote:
>
> On Tue, 21 Apr 2020 at 04:10, kamlesh kumar  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, 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.

Re: [PATCH][libstd++][PR92156]

2020-04-21 Thread Ville Voutilainen via Gcc-patches

On Tue, 21 Apr 2020 at 04:10, kamlesh kumar  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, 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.

Re: [PATCH][libstd++][PR92156]

2020-04-20 Thread kamlesh kumar via Gcc-patches

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, 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
 wrote:
>
> On Mon, 20 Apr 2020 at 21:09, Ville Voutilainen
>  wrote:
> >
> > On Sat, 18 Apr 2020 at 03:35, kamlesh kumar via Libstdc++
> >  wrote:
> > >
> > > On Fri, Apr 17, 2020, 10:59 PM kamlesh kumar 
> > > 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 
> > > > 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.

Re: [PATCH][libstd++][PR92156]

2020-04-20 Thread Ville Voutilainen via Gcc-patches

On Mon, 20 Apr 2020 at 21:09, Ville Voutilainen
 wrote:
>
> On Sat, 18 Apr 2020 at 03:35, kamlesh kumar via Libstdc++
>  wrote:
> >
> > On Fri, Apr 17, 2020, 10:59 PM kamlesh kumar 
> > 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 
> > > 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.

Re: [PATCH][libstd++][PR92156]

2020-04-20 Thread Ville Voutilainen via Gcc-patches

On Sat, 18 Apr 2020 at 03:35, kamlesh kumar via Libstdc++
 wrote:
>
> On Fri, Apr 17, 2020, 10:59 PM kamlesh kumar 
> 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 
> > 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.

Re: [PATCH][libstd++][PR92156]

2020-04-17 Thread kamlesh kumar via Gcc-patches

On Fri, Apr 17, 2020, 10:59 PM kamlesh kumar 
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 
> 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  
> >
> > 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 _Mgr = _Manager<_Tp>,
> > -  __any_constructible_t<_Tp, _ValueType&&> = true,
> > -  enable_if_t::value, bool> = true>
> > +  enable_if_t::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 _Mgr = _Manager<_Tp>,
> > -  enable_if_t<__and_v,
> > -  __not_>,
> > -  __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 _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 _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 000..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
> > +// .
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +int main() {
> > +auto a = std::any(std::in_place_type, 5);
> > +auto b = std::any(std::in_place_type, {1});
> > +std::any p = std::pair(1, 1);
> > +(void)p;
> > +std::any t = std::tuple(1);
> > +(void)t;
> > +return 0;
> > +}
> > +
> >
> > Regtested on X86_64-linux.
> >
> > Thanks,
> >
>

Re: [PATCH][libstd++][PR92156]

2020-04-17 Thread kamlesh kumar via Gcc-patches

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  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  
>
> 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 _Mgr = _Manager<_Tp>,
> -  __any_constructible_t<_Tp, _ValueType&&> = true,
> -  enable_if_t::value, bool> = true>
> +  enable_if_t::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 _Mgr = _Manager<_Tp>,
> -  enable_if_t<__and_v,
> -  __not_>,
> -  __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 _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 _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 000..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
> +// .
> +
> +#include 
> +#include 
> +#include 
> +
> +int main() {
> +auto a = std::any(std::in_place_type, 5);
> +auto b = std::any(std::in_place_type, {1});
> +std::any p = std::pair(1, 1);
> +(void)p;
> +std::any t = std::tuple(1);
> +(void)t;
> +return 0;
> +}
> +
>
> Regtested on X86_64-linux.
>
> Thanks,
>

[PATCH][libstd++][PR92156]

2020-04-17 Thread kamlesh kumar via Gcc-patches

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  

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 _Mgr = _Manager<_Tp>,
-  __any_constructible_t<_Tp, _ValueType&&> = true,
-  enable_if_t::value, bool> = true>
+  enable_if_t::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 _Mgr = _Manager<_Tp>,
-  enable_if_t<__and_v,
-  __not_>,
-  __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 _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 _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 000..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
+// .
+
+#include 
+#include 
+#include 
+
+int main() {
+auto a = std::any(std::in_place_type, 5);
+auto b = std::any(std::in_place_type, {1});
+std::any p = std::pair(1, 1);
+(void)p;
+std::any t = std::tuple(1);
+(void)t;
+return 0;
+}
+

Regtested on X86_64-linux.

Thanks,

Re: [PATCH][libstd++][PR92156]

Re: [PATCH][libstd++][PR92156]

Re: [PATCH][libstd++][PR92156]

Re: [PATCH][libstd++][PR92156]

Re: [PATCH][libstd++][PR92156]

Re: [PATCH][libstd++][PR92156]

Re: [PATCH][libstd++][PR92156]

Re: [PATCH][libstd++][PR92156]

Re: [PATCH][libstd++][PR92156]

Re: [PATCH][libstd++][PR92156]

Re: [PATCH][libstd++][PR92156]

Re: [PATCH][libstd++][PR92156]

[PATCH][libstd++][PR92156]

13 matches

Site Navigation

Mail list logo

Footer information