Re: [v3 PATCH] Implement the missing bits of LWG 2769

2018-02-27 Thread Jonathan Wakely

On 27/02/18 10:13 +0200, Ville Voutilainen wrote:

On 26 February 2018 at 22:52, Jonathan Wakely  wrote:

On 25/02/18 23:22 +0200, Ville Voutilainen wrote:


Tested partially on Linux-x64, will test with the full suite on
Linux-PPC64.
Ok for trunk and the gcc-7 branch? This is theoretically a breaking change

This template argument should be aligned with "_ValueType" on the
previous line, not with "is_constructible".

Looking at that file, I'm also wondering if we want the alias _AnyCast
to be defined at namespace scope. It's only used in a few function
bodies, and its name is a bit misleading.

Could you just do:

 using _Up = remove_cv_t>;

in the four functions that use it?

Then I think the is_constructible specializations would fit on one line
anyway.



Done, new patch attached.


OK for trunk and gcc-7-branch, thanks.




Re: [v3 PATCH] Implement the missing bits of LWG 2769

2018-02-27 Thread Ville Voutilainen
On 26 February 2018 at 22:52, Jonathan Wakely  wrote:
> On 25/02/18 23:22 +0200, Ville Voutilainen wrote:
>>
>> Tested partially on Linux-x64, will test with the full suite on
>> Linux-PPC64.
>> Ok for trunk and the gcc-7 branch? This is theoretically a breaking change
> This template argument should be aligned with "_ValueType" on the
> previous line, not with "is_constructible".
>
> Looking at that file, I'm also wondering if we want the alias _AnyCast
> to be defined at namespace scope. It's only used in a few function
> bodies, and its name is a bit misleading.
>
> Could you just do:
>
>  using _Up = remove_cv_t>;
>
> in the four functions that use it?
>
> Then I think the is_constructible specializations would fit on one line
> anyway.


Done, new patch attached.
diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any
index 466b7ca..a37eb38 100644
--- a/libstdc++-v3/include/std/any
+++ b/libstdc++-v3/include/std/any
@@ -438,8 +438,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   return any(in_place_type<_Tp>, __il, std::forward<_Args>(__args)...);
 }
 
-  template 
-using _AnyCast = remove_cv_t>;
   /**
* @brief Access the contained object.
*
@@ -453,9 +451,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template
 inline _ValueType any_cast(const any& __any)
 {
+  using _Up = remove_cv_t>;
   static_assert(any::__is_valid_cast<_ValueType>(),
  "Template argument must be a reference or CopyConstructible type");
-  auto __p = any_cast<_AnyCast<_ValueType>>(&__any);
+  static_assert(is_constructible_v<_ValueType, const _Up&>,
+ "Template argument must be constructible from a const value.");
+  auto __p = any_cast<_Up>(&__any);
   if (__p)
return static_cast<_ValueType>(*__p);
   __throw_bad_any_cast();
@@ -476,37 +477,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template
 inline _ValueType any_cast(any& __any)
 {
+  using _Up = remove_cv_t>;
   static_assert(any::__is_valid_cast<_ValueType>(),
  "Template argument must be a reference or CopyConstructible type");
-  auto __p = any_cast<_AnyCast<_ValueType>>(&__any);
+  static_assert(is_constructible_v<_ValueType, _Up&>,
+ "Template argument must be constructible from an lvalue.");
+  auto __p = any_cast<_Up>(&__any);
   if (__p)
return static_cast<_ValueType>(*__p);
   __throw_bad_any_cast();
 }
 
-  template::value
-  || is_lvalue_reference<_ValueType>::value,
-  bool>::type = true>
-inline _ValueType any_cast(any&& __any)
-{
-  static_assert(any::__is_valid_cast<_ValueType>(),
- "Template argument must be a reference or CopyConstructible type");
-  auto __p = any_cast<_AnyCast<_ValueType>>(&__any);
-  if (__p)
-   return static_cast<_ValueType>(*__p);
-  __throw_bad_any_cast();
-}
-
-  template::value
-  && !is_lvalue_reference<_ValueType>::value,
-  bool>::type = false>
+  template
 inline _ValueType any_cast(any&& __any)
 {
+  using _Up = remove_cv_t>;
   static_assert(any::__is_valid_cast<_ValueType>(),
  "Template argument must be a reference or CopyConstructible type");
-  auto __p = any_cast<_AnyCast<_ValueType>>(&__any);
+  static_assert(is_constructible_v<_ValueType, _Up>,
+ "Template argument must be constructible from an rvalue.");
+  auto __p = any_cast<_Up>(&__any);
   if (__p)
return static_cast<_ValueType>(std::move(*__p));
   __throw_bad_any_cast();
diff --git a/libstdc++-v3/testsuite/20_util/any/misc/any_cast.cc 
b/libstdc++-v3/testsuite/20_util/any/misc/any_cast.cc
index 45d8b63..37a24d7 100644
--- a/libstdc++-v3/testsuite/20_util/any/misc/any_cast.cc
+++ b/libstdc++-v3/testsuite/20_util/any/misc/any_cast.cc
@@ -95,15 +95,6 @@ void test03()
   VERIFY(move_count == 1);
   MoveEnabled&& m3 = any_cast(any(m));
   VERIFY(move_count == 1);
-  struct MoveDeleted
-  {
-MoveDeleted(MoveDeleted&&) = delete;
-MoveDeleted() = default;
-MoveDeleted(const MoveDeleted&) = default;
-  };
-  MoveDeleted md;
-  MoveDeleted&& md2 = any_cast(any(std::move(md)));
-  MoveDeleted&& md3 = any_cast(any(std::move(md)));
 }
 
 void test04()
diff --git a/libstdc++-v3/testsuite/20_util/any/misc/any_cast_neg.cc 
b/libstdc++-v3/testsuite/20_util/any/misc/any_cast_neg.cc
index 50a9a67..62d7aaa 100644
--- a/libstdc++-v3/testsuite/20_util/any/misc/any_cast_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/any/misc/any_cast_neg.cc
@@ -20,11 +20,26 @@
 
 #include 
 
+using std::any;
+using std::any_cast;
+
 void test01()
 {
-  using std::any;
-  using std::any_cast;
-
   const any y(1);
-  any_cast(y); // { 

Re: [v3 PATCH] Implement the missing bits of LWG 2769

2018-02-26 Thread Ville Voutilainen
On 26 February 2018 at 22:52, Jonathan Wakely  wrote:
> But those are just stylistic issues, the technical side of the patch
> is fine. I had to look up why we had two overloads for any_cast(any&&)
> and that seems to be a leftover from experimental::any, so thanks for
> cleaning that up too.


It was added by me when we (well, I) decided to support
copyable-but-not-movable types.
See, the current specification doesn't allow getting those types out
of an rvalue any by value.
Such an operation will perform an ill-formed move, with the
two-overload solution it would've
fallen back on copying.
Weird as such types are, I thought it not-too-much-trouble to
SFINAE-hack it to work. The specification
has since been made stricter so that it doesn't leave any
implementation leeway to allow that,
so hence the removal of that lenience.


Re: [v3 PATCH] Implement the missing bits of LWG 2769

2018-02-26 Thread Jonathan Wakely

On 25/02/18 23:22 +0200, Ville Voutilainen wrote:

Tested partially on Linux-x64, will test with the full suite on Linux-PPC64.
Ok for trunk and the gcc-7 branch? This is theoretically a breaking change
for the branch, but the committee has decided that they don't want
the support for copyable-but-not-movable types.

2018-02-25  Ville Voutilainen  

   Implement the missing bits of LWG 2769
   * include/std/any (any_cast(const any&)): Add static_assert.
   (any_cast(any&)): Likewise.
   (any_cast(any&&)): Likewise, and remove the handling
   for copyable-but-not-movable type.
   * testsuite/20_util/any/misc/any_cast.cc: Adjust.
   * testsuite/20_util/any/misc/any_cast_neg.cc: Likewise, and
   add new tests.



diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any
index 466b7ca..e0aba3c 100644
--- a/libstdc++-v3/include/std/any
+++ b/libstdc++-v3/include/std/any
@@ -455,6 +455,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
  static_assert(any::__is_valid_cast<_ValueType>(),
  "Template argument must be a reference or CopyConstructible type");
+  static_assert(is_constructible_v<_ValueType,
+   const _AnyCast<_ValueType>&>,


This template argument should be aligned with "_ValueType" on the
previous line, not with "is_constructible".

Looking at that file, I'm also wondering if we want the alias _AnyCast
to be defined at namespace scope. It's only used in a few function
bodies, and its name is a bit misleading.

Could you just do:

 using _Up = remove_cv_t>;

in the four functions that use it?

Then I think the is_constructible specializations would fit on one line
anyway.

But those are just stylistic issues, the technical side of the patch
is fine. I had to look up why we had two overloads for any_cast(any&&)
and that seems to be a leftover from experimental::any, so thanks for
cleaning that up too.



[v3 PATCH] Implement the missing bits of LWG 2769

2018-02-25 Thread Ville Voutilainen
Tested partially on Linux-x64, will test with the full suite on Linux-PPC64.
Ok for trunk and the gcc-7 branch? This is theoretically a breaking change
for the branch, but the committee has decided that they don't want
the support for copyable-but-not-movable types.

2018-02-25  Ville Voutilainen  

Implement the missing bits of LWG 2769
* include/std/any (any_cast(const any&)): Add static_assert.
(any_cast(any&)): Likewise.
(any_cast(any&&)): Likewise, and remove the handling
for copyable-but-not-movable type.
* testsuite/20_util/any/misc/any_cast.cc: Adjust.
* testsuite/20_util/any/misc/any_cast_neg.cc: Likewise, and
add new tests.
diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any
index 466b7ca..e0aba3c 100644
--- a/libstdc++-v3/include/std/any
+++ b/libstdc++-v3/include/std/any
@@ -455,6 +455,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 {
   static_assert(any::__is_valid_cast<_ValueType>(),
  "Template argument must be a reference or CopyConstructible type");
+  static_assert(is_constructible_v<_ValueType,
+   const _AnyCast<_ValueType>&>,
+ "Template argument must be constructible from a const value.");
   auto __p = any_cast<_AnyCast<_ValueType>>(&__any);
   if (__p)
return static_cast<_ValueType>(*__p);
@@ -478,34 +481,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 {
   static_assert(any::__is_valid_cast<_ValueType>(),
  "Template argument must be a reference or CopyConstructible type");
+  static_assert(is_constructible_v<_ValueType,
+   _AnyCast<_ValueType>&>,
+ "Template argument must be constructible from an lvalue.");
   auto __p = any_cast<_AnyCast<_ValueType>>(&__any);
   if (__p)
return static_cast<_ValueType>(*__p);
   __throw_bad_any_cast();
 }
 
-  template::value
-  || is_lvalue_reference<_ValueType>::value,
-  bool>::type = true>
-inline _ValueType any_cast(any&& __any)
-{
-  static_assert(any::__is_valid_cast<_ValueType>(),
- "Template argument must be a reference or CopyConstructible type");
-  auto __p = any_cast<_AnyCast<_ValueType>>(&__any);
-  if (__p)
-   return static_cast<_ValueType>(*__p);
-  __throw_bad_any_cast();
-}
-
-  template::value
-  && !is_lvalue_reference<_ValueType>::value,
-  bool>::type = false>
+  template
 inline _ValueType any_cast(any&& __any)
 {
   static_assert(any::__is_valid_cast<_ValueType>(),
  "Template argument must be a reference or CopyConstructible type");
+  static_assert(is_constructible_v<_ValueType,
+   _AnyCast<_ValueType>>,
+ "Template argument must be constructible from an rvalue.");
   auto __p = any_cast<_AnyCast<_ValueType>>(&__any);
   if (__p)
return static_cast<_ValueType>(std::move(*__p));
diff --git a/libstdc++-v3/testsuite/20_util/any/misc/any_cast.cc 
b/libstdc++-v3/testsuite/20_util/any/misc/any_cast.cc
index 45d8b63..37a24d7 100644
--- a/libstdc++-v3/testsuite/20_util/any/misc/any_cast.cc
+++ b/libstdc++-v3/testsuite/20_util/any/misc/any_cast.cc
@@ -95,15 +95,6 @@ void test03()
   VERIFY(move_count == 1);
   MoveEnabled&& m3 = any_cast(any(m));
   VERIFY(move_count == 1);
-  struct MoveDeleted
-  {
-MoveDeleted(MoveDeleted&&) = delete;
-MoveDeleted() = default;
-MoveDeleted(const MoveDeleted&) = default;
-  };
-  MoveDeleted md;
-  MoveDeleted&& md2 = any_cast(any(std::move(md)));
-  MoveDeleted&& md3 = any_cast(any(std::move(md)));
 }
 
 void test04()
diff --git a/libstdc++-v3/testsuite/20_util/any/misc/any_cast_neg.cc 
b/libstdc++-v3/testsuite/20_util/any/misc/any_cast_neg.cc
index 50a9a67..08da07d 100644
--- a/libstdc++-v3/testsuite/20_util/any/misc/any_cast_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/any/misc/any_cast_neg.cc
@@ -20,11 +20,26 @@
 
 #include 
 
+using std::any;
+using std::any_cast;
+
 void test01()
 {
-  using std::any;
-  using std::any_cast;
-
   const any y(1);
-  any_cast(y); // { dg-error "invalid static_cast" "" { target { *-*-* } 
} 460 }
+  any_cast(y); // { dg-error "invalid static_cast" "" { target { *-*-* } 
} 463 }
+  // { dg-error "Template argument must be constructible from a const value" 
"" { target { *-*-* } } 458 }
+}
+
+void test02()
+{
+  any y(1);
+  any_cast(y);
+  // { dg-error "Template argument must be constructible from an lvalue" "" { 
target { *-*-* } } 484 }
+}
+
+void test03()
+{
+  any y(1);
+  any_cast(std::move(y));  // { dg-error "invalid static_cast" "" { 
target { *-*-* } } 503 }
+  // { dg-error "Template argument must be constructible from an rvalue" "" { 
target { *-*-* } } 498 }
 }