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

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

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 854 bytes --]

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 <bits/stl_construct.h> could be split up to move _Construct
and _Destroy to a new <bits/stl_construct_base.h>.  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.



[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 2837 bytes --]

commit 8a813292bd7170eb160b509122fc3bb388b87f12
Author: Jonathan Wakely <jwakely@redhat.com>
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 <bits/invoke.h>
 #include <ext/aligned_buffer.h>
 #include <bits/parse_numbers.h>
+#include <bits/stl_iterator_base_types.h>
+#include <bits/stl_iterator_base_funcs.h>
+#include <bits/stl_construct.h>
 
 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<typename _Lhs, typename _Rhs>
-    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<typename _Variant, size_t _Np>
-    constexpr void
+    void
     __erased_dtor(_Variant&& __v)
+    { std::_Destroy(std::__addressof(__get<_Np>(__v))); }
+
+  template<typename _Lhs, typename _Rhs>
+    void
+    __erased_assign(void* __lhs, void* __rhs)
     {
-      auto&& __element = __get<_Np>(std::forward<_Variant>(__v));
-      using _Type = std::remove_reference_t<decltype(__element)>;
-      __element.~_Type();
+      __variant::__ref_cast<_Lhs>(__lhs) = __variant::__ref_cast<_Rhs>(__rhs);
     }
 
   template<typename _Lhs, typename _Rhs>
-    constexpr void
-    __erased_assign(void* __lhs, void* __rhs)
-    { __ref_cast<_Lhs>(__lhs) = __ref_cast<_Rhs>(__rhs); }
-
-  template<typename _Lhs, typename _Rhs>
-    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<typename _Tp>
-    constexpr size_t
+    size_t
     __erased_hash(void* __t)
     {
       return std::hash<remove_cv_t<remove_reference_t<_Tp>>>{}(
-	  __ref_cast<_Tp>(__t));
+	  __variant::__ref_cast<_Tp>(__t));
     }
 
   // Defines members and ctors.

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 <bits/stl_construct.h> could be split up to move _Construct
> and _Destroy to a new <bits/stl_construct_base.h>.  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

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

[Tim Song]

On Fri, Jun 2, 2017 at 3:19 PM, Tim Shen <timshen@google.com> 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

[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 9:07 AM, Jonathan Wakely <jwakely@redhat.com> 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 :(