Re: [v3 PATCH] Implement the missing bits of LWG 2769
On 27/02/18 10:13 +0200, Ville Voutilainen wrote: On 26 February 2018 at 22:52, Jonathan Wakelywrote: 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
On 26 February 2018 at 22:52, Jonathan Wakelywrote: > 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
On 26 February 2018 at 22:52, Jonathan Wakelywrote: > 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
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 VoutilainenImplement 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
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 VoutilainenImplement 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 } }