[v3 PATCH] PR libstdc++/70437

[v3 PATCH] PR libstdc++/70437

[Ville Voutilainen]

Tested on Linux-PPC64.

2016-04-04  Ville Voutilainen  <ville.voutilainen@gmail.com>

     PR libstdc++/70437
     * include/bits/stl_pair.h (_ConstructiblePair,
    _ImplicitlyConvertiblePair, _MoveConstructiblePair,
    _ImplicitlyMoveConvertiblePair): Add shortcut conditions
    for same-type cases.
    * testsuite/20_util/pair/70437.cc: New.

Re: [v3 PATCH] PR libstdc++/70437

[Ville Voutilainen]

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

And yes, -ENOPATCH.

On 4 April 2016 at 21:42, Ville Voutilainen <ville.voutilainen@gmail.com> wrote:
> Tested on Linux-PPC64.
>
> 2016-04-04  Ville Voutilainen  <ville.voutilainen@gmail.com>
>
>      PR libstdc++/70437
>      * include/bits/stl_pair.h (_ConstructiblePair,
>     _ImplicitlyConvertiblePair, _MoveConstructiblePair,
>     _ImplicitlyMoveConvertiblePair): Add shortcut conditions
>     for same-type cases.
>     * testsuite/20_util/pair/70437.cc: New.

[-- Attachment #2: 70437.diff --]
[-- Type: text/plain, Size: 3505 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_pair.h b/libstdc++-v3/include/bits/stl_pair.h
index 7057030..206553a 100644
--- a/libstdc++-v3/include/bits/stl_pair.h
+++ b/libstdc++-v3/include/bits/stl_pair.h
@@ -90,29 +90,45 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template <typename _T1, typename _T2, typename _U1, typename _U2>
   constexpr bool _ConstructiblePair()
   {
-    return __and_<is_constructible<_T1, const _U1&>,
-		  is_constructible<_T2, const _U2&>>::value;
+    return __and_<__or_<is_same<typename decay<_T1>::type,
+				typename decay<_U1>::type>,
+			is_constructible<_T1, const _U1&>>,
+		  __or_<is_same<typename decay<_T2>::type,
+				typename decay<_U2>::type>,
+			is_constructible<_T2, const _U2&>>>::value;
   }
 
   template <typename _T1, typename _T2, typename _U1, typename _U2>
   constexpr bool _ImplicitlyConvertiblePair()
   {
-    return __and_<is_convertible<const _U1&, _T1>,
-		  is_convertible<const _U2&, _T2>>::value;
+    return __and_<__or_<is_same<typename decay<_T1>::type,
+				typename decay<_U1>::type>,
+			is_convertible<const _U1&, _T1>>,
+		  __or_<is_same<typename decay<_T2>::type,
+				typename decay<_U2>::type>,
+		       is_convertible<const _U2&, _T2>>>::value;
   }
 
   template <typename _T1, typename _T2, typename _U1, typename _U2>
   constexpr bool _MoveConstructiblePair()
   {
-    return __and_<is_constructible<_T1, _U1&&>,
-		  is_constructible<_T2, _U2&&>>::value;
+    return __and_<__or_<is_same<typename decay<_T1>::type,
+				typename decay<_U1>::type>,
+			is_constructible<_T1, _U1&&>>,
+		  __or_<is_same<typename decay<_T2>::type,
+				typename decay<_U2>::type>,
+			is_constructible<_T2, _U2&&>>>::value;
   }
 
   template <typename _T1, typename _T2, typename _U1, typename _U2>
   constexpr bool _ImplicitlyMoveConvertiblePair()
   {
-    return __and_<is_convertible<_U1&&, _T1>,
-		  is_convertible<_U2&&, _T2>>::value;
+    return __and_<__or_<is_same<typename decay<_T1>::type,
+				typename decay<_U1>::type>,
+			is_convertible<_U1&&, _T1>>,
+		  __or_<is_same<typename decay<_T2>::type,
+				typename decay<_U2>::type>,
+		       is_convertible<_U2&&, _T2>>>::value;
   }
 
 
diff --git a/libstdc++-v3/testsuite/20_util/pair/70437.cc b/libstdc++-v3/testsuite/20_util/pair/70437.cc
new file mode 100644
index 0000000..37e6fb7
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/pair/70437.cc
@@ -0,0 +1,37 @@
+// { dg-options "-std=gnu++11" }
+// { dg-do compile }
+
+// Copyright (C) 2016 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
+// <http://www.gnu.org/licenses/>.
+
+#include <utility>
+
+template <class T> struct B;
+
+template <class T> struct A
+{
+  A(A&&) = default;
+  A(const B<T> &);
+};
+
+template <class T> struct B
+{
+  std::pair<A<T>,int> a;
+  B(B&&) = default;
+};
+
+bool b = std::is_move_constructible<A<int> >::value;

Re: [v3 PATCH] PR libstdc++/70437

[Jonathan Wakely]

On 04/04/16 21:45 +0300, Ville Voutilainen wrote:
>And yes, -ENOPATCH.
>
>On 4 April 2016 at 21:42, Ville Voutilainen <ville.voutilainen@gmail.com> wrote:
>> Tested on Linux-PPC64.
>>
>> 2016-04-04  Ville Voutilainen  <ville.voutilainen@gmail.com>
>>
>>      PR libstdc++/70437
>>      * include/bits/stl_pair.h (_ConstructiblePair,
>>     _ImplicitlyConvertiblePair, _MoveConstructiblePair,
>>     _ImplicitlyMoveConvertiblePair): Add shortcut conditions
>>     for same-type cases.
>>     * testsuite/20_util/pair/70437.cc: New.

Thanks for the fix.

>diff --git a/libstdc++-v3/include/bits/stl_pair.h b/libstdc++-v3/include/bits/stl_pair.h
>index 7057030..206553a 100644
>--- a/libstdc++-v3/include/bits/stl_pair.h
>+++ b/libstdc++-v3/include/bits/stl_pair.h
>@@ -90,29 +90,45 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>   template <typename _T1, typename _T2, typename _U1, typename _U2>
>   constexpr bool _ConstructiblePair()
>   {
>-    return __and_<is_constructible<_T1, const _U1&>,
>-		  is_constructible<_T2, const _U2&>>::value;
>+    return __and_<__or_<is_same<typename decay<_T1>::type,
>+				typename decay<_U1>::type>,
>+			is_constructible<_T1, const _U1&>>,
>+		  __or_<is_same<typename decay<_T2>::type,
>+				typename decay<_U2>::type>,
>+			is_constructible<_T2, const _U2&>>>::value;
>   }

I wonder if we want an __is_samey trait that checks if two decayed
types are the same.

More seriously, a comment might be useful to explain that although
these "concepts" return true for samey types, that is just to prevent 
is_constructible from getting into a mess with incomplete types, and
actually for samey types one of the special member functions might end
up being chosen by overload resolution instead.

Did I get that right? If so, I definitely think it's worth a comment,
as I for one won't remember the details in a few months!

Re: [v3 PATCH] PR libstdc++/70437

[Ville Voutilainen]

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

On 5 April 2016 at 13:53, Jonathan Wakely <jwakely@redhat.com> wrote:
> I wonder if we want an __is_samey trait that checks if two decayed
> types are the same.

If such checks become more common, then yes. For now, perhaps not.

> More seriously, a comment might be useful to explain that although
> these "concepts" return true for samey types, that is just to prevent
> is_constructible from getting into a mess with incomplete types, and
> actually for samey types one of the special member functions might end
> up being chosen by overload resolution instead.
>
> Did I get that right? If so, I definitely think it's worth a comment,
> as I for one won't remember the details in a few months!

How about the attached new patch? I just added a comment at the top of these
"concept utilities". In general, there's an unfortunate amount of such trickery
needed to get pair and tuple right as far as their constraints go, to protect
the innocent overloads from getting input that they can't cope with, as such
constraints are evaluated during overload resolution, and in some cases that
evaluation will be done even for overloads that will certainly not be
chosen, but
they have to be prepared for input that is hard to digest. That's one of the
reasons why 'if constexpr' will be a godsend, but I should not digress
there right now. :)

[-- Attachment #2: 70437.diff2 --]
[-- Type: application/octet-stream, Size: 3765 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_pair.h b/libstdc++-v3/include/bits/stl_pair.h
index 7057030..37ee5cc 100644
--- a/libstdc++-v3/include/bits/stl_pair.h
+++ b/libstdc++-v3/include/bits/stl_pair.h
@@ -87,32 +87,51 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   // Concept utility functions, reused in conditionally-explicit
   // constructors.
+  // See PR 70437, don't look at is_constructible or
+  // is_convertible if the decayed types are the same to
+  // avoid querying those properties for incomplete types.
   template <typename _T1, typename _T2, typename _U1, typename _U2>
   constexpr bool _ConstructiblePair()
   {
-    return __and_<is_constructible<_T1, const _U1&>,
-		  is_constructible<_T2, const _U2&>>::value;
+    return __and_<__or_<is_same<typename decay<_T1>::type,
+				typename decay<_U1>::type>,
+			is_constructible<_T1, const _U1&>>,
+		  __or_<is_same<typename decay<_T2>::type,
+				typename decay<_U2>::type>,
+			is_constructible<_T2, const _U2&>>>::value;
   }
 
   template <typename _T1, typename _T2, typename _U1, typename _U2>
   constexpr bool _ImplicitlyConvertiblePair()
   {
-    return __and_<is_convertible<const _U1&, _T1>,
-		  is_convertible<const _U2&, _T2>>::value;
+    return __and_<__or_<is_same<typename decay<_T1>::type,
+				typename decay<_U1>::type>,
+			is_convertible<const _U1&, _T1>>,
+		  __or_<is_same<typename decay<_T2>::type,
+				typename decay<_U2>::type>,
+		       is_convertible<const _U2&, _T2>>>::value;
   }
 
   template <typename _T1, typename _T2, typename _U1, typename _U2>
   constexpr bool _MoveConstructiblePair()
   {
-    return __and_<is_constructible<_T1, _U1&&>,
-		  is_constructible<_T2, _U2&&>>::value;
+    return __and_<__or_<is_same<typename decay<_T1>::type,
+				typename decay<_U1>::type>,
+			is_constructible<_T1, _U1&&>>,
+		  __or_<is_same<typename decay<_T2>::type,
+				typename decay<_U2>::type>,
+			is_constructible<_T2, _U2&&>>>::value;
   }
 
   template <typename _T1, typename _T2, typename _U1, typename _U2>
   constexpr bool _ImplicitlyMoveConvertiblePair()
   {
-    return __and_<is_convertible<_U1&&, _T1>,
-		  is_convertible<_U2&&, _T2>>::value;
+    return __and_<__or_<is_same<typename decay<_T1>::type,
+				typename decay<_U1>::type>,
+			is_convertible<_U1&&, _T1>>,
+		  __or_<is_same<typename decay<_T2>::type,
+				typename decay<_U2>::type>,
+		       is_convertible<_U2&&, _T2>>>::value;
   }
 
 
diff --git a/libstdc++-v3/testsuite/20_util/pair/70437.cc b/libstdc++-v3/testsuite/20_util/pair/70437.cc
new file mode 100644
index 0000000..37e6fb7
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/pair/70437.cc
@@ -0,0 +1,37 @@
+// { dg-options "-std=gnu++11" }
+// { dg-do compile }
+
+// Copyright (C) 2016 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
+// <http://www.gnu.org/licenses/>.
+
+#include <utility>
+
+template <class T> struct B;
+
+template <class T> struct A
+{
+  A(A&&) = default;
+  A(const B<T> &);
+};
+
+template <class T> struct B
+{
+  std::pair<A<T>,int> a;
+  B(B&&) = default;
+};
+
+bool b = std::is_move_constructible<A<int> >::value;

Re: [v3 PATCH] PR libstdc++/70437

[Jonathan Wakely]

On 05/04/16 14:07 +0300, Ville Voutilainen wrote:
>On 5 April 2016 at 13:53, Jonathan Wakely <jwakely@redhat.com> wrote:
>> I wonder if we want an __is_samey trait that checks if two decayed
>> types are the same.
>
>If such checks become more common, then yes. For now, perhaps not.

We already do it in packaged_task, function, any, and optional.

I'll look into doing that in stage 1.

>> More seriously, a comment might be useful to explain that although
>> these "concepts" return true for samey types, that is just to prevent
>> is_constructible from getting into a mess with incomplete types, and
>> actually for samey types one of the special member functions might end
>> up being chosen by overload resolution instead.
>>
>> Did I get that right? If so, I definitely think it's worth a comment,
>> as I for one won't remember the details in a few months!
>
>How about the attached new patch?

Great - OK for trunk, thanks.

>I just added a comment at the top of these
>"concept utilities". In general, there's an unfortunate amount of such trickery
>needed to get pair and tuple right as far as their constraints go, to protect
>the innocent overloads from getting input that they can't cope with, as such
>constraints are evaluated during overload resolution, and in some cases that
>evaluation will be done even for overloads that will certainly not be
>chosen, but
>they have to be prepared for input that is hard to digest. That's one of the
>reasons why 'if constexpr' will be a godsend, but I should not digress
>there right now. :)

I'd sell a kidney for `if constexpr` right now ;-)