Re: [PATCH] PR libstdc++/80939 Remove unmeetable constexpr specifiers

[Tim Song]

On Fri, Jun 2, 2017 at 9:07 AM, Jonathan Wakely  wrote:
> we aren't qualifying calls to __ref_cast,

Turns out this is fine. I somehow managed to completely misread the
last sentence of [basic.lookup.argdep]/2. It's talking about the case
where an argument to the call is a set of overloaded functions named
by a template-id, not about the postfix-expression.

At least qualifying those calls doesn't hurt either.

Sorry :(

Re: [PATCH] PR libstdc++/80939 Remove unmeetable constexpr specifiers

[Jonathan Wakely]

On 02/06/17 12:19 -0700, Tim Shen wrote:

On Fri, Jun 2, 2017 at 6:07 AM, Jonathan Wakely wrote:

As the PR points out, we aren't qualifying calls to __ref_cast, and
have 'constexpr' on function templates that can never be usable in
constant expressions.


Apology for the constexpr trolling, but that was not intentional. :)

I'm curious why no tests break. Is it because constexpr in a template
function is a no-op instead of a hard error, when the function
definition is not constexpr?

The patch looks good.


Committed to trunk (I'll backport it to gcc-7-branch some time too).

Re: [PATCH] PR libstdc++/80939 Remove unmeetable constexpr specifiers

[Tim Song]

On Fri, Jun 2, 2017 at 3:19 PM, Tim Shen  wrote:
> On Fri, Jun 2, 2017 at 6:07 AM, Jonathan Wakely wrote:
>> As the PR points out, we aren't qualifying calls to __ref_cast, and
>> have 'constexpr' on function templates that can never be usable in
>> constant expressions.
>
> Apology for the constexpr trolling, but that was not intentional. :)
>
> I'm curious why no tests break. Is it because constexpr in a template
> function is a no-op instead of a hard error, when the function
> definition is not constexpr?
>
> The patch looks good.
>

A non-template, non-default constexpr function that can never be used
in a constant expression is ill-formed NDR. ([dcl.constexpr]/5)

A constexpr function template for which no specialization that
satisfies the requirements for a constexpr function (when considered
as a non-template) can be generated is ill-formed NDR.
([dcl.constexpr]/6)

It's not really clear to me whether the second rule incorporates the
first or if it's just talking about the requirements in
[dcl.constexpr]/3, but regardless it's not required to break.

Re: [PATCH] PR libstdc++/80939 Remove unmeetable constexpr specifiers

[Tim Shen via gcc-patches]

On Fri, Jun 2, 2017 at 6:07 AM, Jonathan Wakely wrote:
> As the PR points out, we aren't qualifying calls to __ref_cast, and
> have 'constexpr' on function templates that can never be usable in
> constant expressions.

Apology for the constexpr trolling, but that was not intentional. :)

I'm curious why no tests break. Is it because constexpr in a template
function is a no-op instead of a hard error, when the function
definition is not constexpr?

The patch looks good.

>
> This fixes it, and also simplifies __variant::__erased_dtor by using
> std::_Destroy, although that requires including quite a lot more code,
> for iterator_traits and allocator_traits.  If that matters (probably
> not) then  could be split up to move _Construct
> and _Destroy to a new .  Or maybe I should
> just leave __erased_dtor alone (apart from qualifying the __ref_cast
> call).
>
> Anybody feel strongly either way?
>
> PR libstdc++/80939
> * include/std/variant (__erased_ctor, __erased_assign,
> __erased_swap)
> (__erased_hash): Remove constexpr specifier and qualify calls to
> __ref_cast.
> (__erased_dtor): Remove constexpr specifier and use _Destroy.
>
>



-- 
Regards,
Tim Shen

[PATCH] PR libstdc++/80939 Remove unmeetable constexpr specifiers

[Jonathan Wakely]

As the PR points out, we aren't qualifying calls to __ref_cast, and
have 'constexpr' on function templates that can never be usable in
constant expressions.

This fixes it, and also simplifies __variant::__erased_dtor by using
std::_Destroy, although that requires including quite a lot more code,
for iterator_traits and allocator_traits.  If that matters (probably
not) then  could be split up to move _Construct
and _Destroy to a new .  Or maybe I should
just leave __erased_dtor alone (apart from qualifying the __ref_cast
call).

Anybody feel strongly either way?

PR libstdc++/80939
* include/std/variant (__erased_ctor, __erased_assign, __erased_swap)
(__erased_hash): Remove constexpr specifier and qualify calls to
__ref_cast.
(__erased_dtor): Remove constexpr specifier and use _Destroy.


commit 8a813292bd7170eb160b509122fc3bb388b87f12
Author: Jonathan Wakely 
Date:   Fri Jun 2 13:00:52 2017 +0100

PR libstdc++/80939 Remove unmeetable constexpr specifiers

PR libstdc++/80939
* include/std/variant (__erased_ctor, __erased_assign, __erased_swap)
(__erased_hash): Remove constexpr specifier and qualify calls to
__ref_cast.
(__erased_dtor): Remove constexpr specifier and use _Destroy.

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index b9824a5..e5fe9f9 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -44,6 +44,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 namespace std _GLIBCXX_VISIBILITY(default)
 {
@@ -238,30 +241,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // Various functions as "vtable" entries, where those vtables are used by
   // polymorphic operations.
   template
-constexpr void
+void
 __erased_ctor(void* __lhs, void* __rhs)
-{ ::new (__lhs) remove_reference_t<_Lhs>(__ref_cast<_Rhs>(__rhs)); }
+{
+  using _Type = remove_reference_t<_Lhs>;
+  ::new (__lhs) _Type(__variant::__ref_cast<_Rhs>(__rhs));
+}
 
   template
-constexpr void
+void
 __erased_dtor(_Variant&& __v)
+{ std::_Destroy(std::__addressof(__get<_Np>(__v))); }
+
+  template
+void
+__erased_assign(void* __lhs, void* __rhs)
 {
-  auto&& __element = __get<_Np>(std::forward<_Variant>(__v));
-  using _Type = std::remove_reference_t;
-  __element.~_Type();
+  __variant::__ref_cast<_Lhs>(__lhs) = __variant::__ref_cast<_Rhs>(__rhs);
 }
 
   template
-constexpr void
-__erased_assign(void* __lhs, void* __rhs)
-{ __ref_cast<_Lhs>(__lhs) = __ref_cast<_Rhs>(__rhs); }
-
-  template
-constexpr void
+void
 __erased_swap(void* __lhs, void* __rhs)
 {
   using std::swap;
-  swap(__ref_cast<_Lhs>(__lhs), __ref_cast<_Rhs>(__rhs));
+  swap(__variant::__ref_cast<_Lhs>(__lhs),
+  __variant::__ref_cast<_Rhs>(__rhs));
 }
 
 #define _VARIANT_RELATION_FUNCTION_TEMPLATE(__OP, __NAME) \
@@ -283,11 +288,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #undef _VARIANT_RELATION_FUNCTION_TEMPLATE
 
   template
-constexpr size_t
+size_t
 __erased_hash(void* __t)
 {
   return std::hash>>{}(
- __ref_cast<_Tp>(__t));
+ __variant::__ref_cast<_Tp>(__t));
 }
 
   // Defines members and ctors.