public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libstdc++: Replace deduced return type in ranges::iter_move (PR 92894)
@ 2020-05-01 12:03 Jonathan Wakely
  2020-05-01 13:28 ` Jonathan Wakely
  0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Wakely @ 2020-05-01 12:03 UTC (permalink / raw)
  To: libstdc++, gcc-patches

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

The deduced return type causes the instantiation of the function body,
which can then require the instantiation of std::projected::operator*
which is intentionally not defined.

This patch uses a helper trait to define the return type, so that the
function body doesn't need to be instantiated. That helper trait can
then also be used in other places that currently check the return type
of ranges::iter_move (iter_rvalue_reference_t and indirectly_readable).

2020-05-01  Jonathan Wakely  <jwakely@redhat.com>
	    Patrick Palka  <ppalka@redhat.com>

	PR libstdc++/92894
	* include/bits/iterator_concepts.h (ranges::__cust_imove::_IMove):
	Add trait to determine return type and an alias for it.
	(ranges::__cust_imove::_IMove::operator()): Use __result instead of
	deduced return type.
	(iter_rvalue_reference_t): Use _IMove::__type instead of checking
	the result of ranges::iter_move.
	(__detail::__indirectly_readable_impl): Use iter_rvalue_reference_t
	instead of checking the result of ranges::iter_move.
	* testsuite/24_iterators/indirect_callable/92894.cc: New test.

Patrick, I prefer this to the patch you added to the bug. Avoiding
doing overload resolution on ranges::iter_move(x) seems worthwhile.

What do you think?

I *think* the changes to __indirectly_readable_impl are equivalent. As
far as I can tell, the point of the original { *in }
and { ranges::iter_move(in) } constraints are to check that const In
and In do the same thing. We could leave the { *in } on, but for
consistency it seems better to change both.



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

commit 312bf4457effa0ce99c7c4cb0ebe5e3fb0fd3747
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri May 1 10:50:50 2020 +0100

    libstdc++: Replace deduced return type in ranges::iter_move (PR 92894)
    
    The deduced return type causes the instantiation of the function body,
    which can then require the instantiation of std::projected::operator*
    which is intentionally not defined.
    
    This patch uses a helper trait to define the return type, so that the
    function body doesn't need to be instantiated. That helper trait can
    then also be used in other places that currently check the return type
    of ranges::iter_move (iter_rvalue_reference_t and indirectly_readable).
    
    2020-05-01  Jonathan Wakely  <jwakely@redhat.com>
                Patrick Palka  <ppalka@redhat.com>
    
            PR libstdc++/92894
            * include/bits/iterator_concepts.h (ranges::__cust_imove::_IMove):
            Add trait to determine return type and an alias for it.
            (ranges::__cust_imove::_IMove::operator()): Use __result instead of
            deduced return type.
            (iter_rvalue_reference_t): Use _IMove::__type instead of checking
            the result of ranges::iter_move.
            (__detail::__indirectly_readable_impl): Use iter_rvalue_reference_t
            instead of checking the result of ranges::iter_move.
            * testsuite/24_iterators/indirect_callable/92894.cc: New test.

diff --git a/libstdc++-v3/include/bits/iterator_concepts.h b/libstdc++-v3/include/bits/iterator_concepts.h
index b598532089e..d84e8639ea8 100644
--- a/libstdc++-v3/include/bits/iterator_concepts.h
+++ b/libstdc++-v3/include/bits/iterator_concepts.h
@@ -89,6 +89,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       struct _IMove
       {
       private:
+	template<typename _Tp>
+	  struct __result
+	  { using type = iter_reference_t<_Tp>; };
+
+	template<typename _Tp>
+	  requires __adl_imove<_Tp>
+	  struct __result<_Tp>
+	  { using type = decltype(iter_move(std::declval<_Tp>())); };
+
+	template<typename _Tp>
+	  requires (!__adl_imove<_Tp>)
+	  && is_lvalue_reference_v<iter_reference_t<_Tp>>
+	  struct __result<_Tp>
+	  { using type = remove_reference_t<iter_reference_t<_Tp>>&&; };
+
 	template<typename _Tp>
 	  static constexpr bool
 	  _S_noexcept()
@@ -100,16 +115,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  }
 
       public:
-	template<typename _Tp>
-	  requires __adl_imove<_Tp> || requires(_Tp& __e) { *__e; }
-	  constexpr decltype(auto)
+	// The result type of iter_move(std::declval<_Tp>())
+	template<std::__detail::__dereferenceable _Tp>
+	  using __type = typename __result<_Tp>::type;
+
+	template<std::__detail::__dereferenceable _Tp>
+	  constexpr __type<_Tp>
 	  operator()(_Tp&& __e) const
 	  noexcept(_S_noexcept<_Tp>())
 	  {
 	    if constexpr (__adl_imove<_Tp>)
 	      return iter_move(static_cast<_Tp&&>(__e));
-	    else if constexpr (is_reference_v<iter_reference_t<_Tp>>)
-	      return std::move(*__e);
+	    else if constexpr (is_lvalue_reference_v<iter_reference_t<_Tp>>)
+	      return static_cast<__type<_Tp>>(*__e);
 	    else
 	      return *__e;
 	  }
@@ -123,10 +141,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   } // namespace ranges
 
   template<__detail::__dereferenceable _Tp>
-    requires requires(_Tp& __t)
-    { { ranges::iter_move(__t) } -> __detail::__can_reference; }
+    requires __detail::__can_reference<ranges::__cust_imove::_IMove::__type<_Tp&>>
     using iter_rvalue_reference_t
-      = decltype(ranges::iter_move(std::declval<_Tp&>()));
+      = ranges::__cust_imove::_IMove::__type<_Tp&>;
 
   template<typename> struct incrementable_traits { };
 
@@ -448,8 +465,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	typename iter_value_t<_In>;
 	typename iter_reference_t<_In>;
 	typename iter_rvalue_reference_t<_In>;
-	{ *__in } -> same_as<iter_reference_t<_In>>;
-	{ ranges::iter_move(__in) } -> same_as<iter_rvalue_reference_t<_In>>;
+	requires same_as<iter_reference_t<const _In>,
+			 iter_reference_t<_In>>;
+	requires same_as<iter_rvalue_reference_t<const _In>,
+			 iter_rvalue_reference_t<_In>>;
       }
       && common_reference_with<iter_reference_t<_In>&&, iter_value_t<_In>&>
       && common_reference_with<iter_reference_t<_In>&&,
diff --git a/libstdc++-v3/testsuite/24_iterators/indirect_callable/92894.cc b/libstdc++-v3/testsuite/24_iterators/indirect_callable/92894.cc
new file mode 100644
index 00000000000..78c4585923f
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/indirect_callable/92894.cc
@@ -0,0 +1,59 @@
+// Copyright (C) 2020 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/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <iterator>
+
+using std::projected;
+using std::identity;
+using std::indirect_unary_predicate;
+
+template<typename T,
+         indirect_unary_predicate<projected<T*, identity>> Pred>
+  constexpr void
+  all_of(T*, Pred)
+  { }
+
+void
+test01()
+{
+  // PR libstdc++/92894
+  struct X { };
+  X x;
+  all_of(&x, [](X&) { return false; });
+}
+
+using std::iterator_t;
+using std::ranges::input_range;
+using std::ranges::borrowed_range;
+
+template<input_range R, class Proj = identity,
+	  indirect_unary_predicate<projected<iterator_t<R>, Proj>> Pred>
+  constexpr borrowed_iterator_t<R>
+  find_if(R&& r, Pred pred, Proj proj = {})
+  { }
+
+void
+test02()
+{
+  // PR 94241
+  struct s { int m; };
+  s r[] = { s{0}, s{1}, s{2}, s{3} };
+  find_if(r, [](auto const) { return true; });
+}

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] libstdc++: Replace deduced return type in ranges::iter_move (PR 92894)
  2020-05-01 12:03 [PATCH] libstdc++: Replace deduced return type in ranges::iter_move (PR 92894) Jonathan Wakely
@ 2020-05-01 13:28 ` Jonathan Wakely
  2020-05-01 13:47   ` Jonathan Wakely
  0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Wakely @ 2020-05-01 13:28 UTC (permalink / raw)
  To: libstdc++, gcc-patches

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

On 01/05/20 13:03 +0100, Jonathan Wakely wrote:
>The deduced return type causes the instantiation of the function body,
>which can then require the instantiation of std::projected::operator*
>which is intentionally not defined.
>
>This patch uses a helper trait to define the return type, so that the
>function body doesn't need to be instantiated. That helper trait can
>then also be used in other places that currently check the return type
>of ranges::iter_move (iter_rvalue_reference_t and indirectly_readable).
>
>2020-05-01  Jonathan Wakely  <jwakely@redhat.com>
>	    Patrick Palka  <ppalka@redhat.com>
>
>	PR libstdc++/92894
>	* include/bits/iterator_concepts.h (ranges::__cust_imove::_IMove):
>	Add trait to determine return type and an alias for it.
>	(ranges::__cust_imove::_IMove::operator()): Use __result instead of
>	deduced return type.
>	(iter_rvalue_reference_t): Use _IMove::__type instead of checking
>	the result of ranges::iter_move.
>	(__detail::__indirectly_readable_impl): Use iter_rvalue_reference_t
>	instead of checking the result of ranges::iter_move.
>	* testsuite/24_iterators/indirect_callable/92894.cc: New test.
>
>Patrick, I prefer this to the patch you added to the bug. Avoiding
>doing overload resolution on ranges::iter_move(x) seems worthwhile.
>
>What do you think?
>
>I *think* the changes to __indirectly_readable_impl are equivalent. As
>far as I can tell, the point of the original { *in }
>and { ranges::iter_move(in) } constraints are to check that const In
>and In do the same thing. We could leave the { *in } on, but for
>consistency it seems better to change both.

Here's a slightly improved patch which I'm committing now.

Diffs from the previous patch are removing the useless parameter list
from the __indirectly_readable_impl concept:

@@ -460,7 +461,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        using __iter_concept = typename __iter_concept_impl<_Iter>::type;

    template<typename _In>
-    concept __indirectly_readable_impl = requires(const _In __in)
+    concept __indirectly_readable_impl = requires
        {
         typename iter_value_t<_In>;
         typename iter_reference_t<_In>;

Fixing 24_iterators/indirect_callable/92894.cc so it actually passes,
and adding 24_iterators/customization_points/92894.cc to verify that
iter_move is fixed (which the other tests don't do because
indirectly_readable no longer uses ranges::iter_move).

Tested powerpc64le-linux, committed to master.


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

commit a5f2fb1ff1742685a91dfdf78da871fd4d3292e5
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri May 1 14:27:25 2020 +0100

    libstdc++: Replace deduced return type in ranges::iter_move (PR 92894)
    
    The deduced return type causes the instantiation of the function body,
    which can then require the instantiation of std::projected::operator*
    which is intentionally not defined.
    
    This patch uses a helper trait to define the return type, so that the
    function body doesn't need to be instantiated. That helper trait can
    then also be used in other places that currently check the return type
    of ranges::iter_move (iter_rvalue_reference_t and indirectly_readable).
    
    2020-05-01  Jonathan Wakely  <jwakely@redhat.com>
                Patrick Palka  <ppalka@redhat.com>
    
            PR libstdc++/92894
            * include/bits/iterator_concepts.h (ranges::__cust_imove::_IMove):
            Add trait to determine return type and an alias for it.
            (ranges::__cust_imove::_IMove::operator()): Use __result instead of
            deduced return type.
            (iter_rvalue_reference_t): Use _IMove::__type instead of checking
            the result of ranges::iter_move.
            (__detail::__indirectly_readable_impl): Use iter_rvalue_reference_t
            instead of checking the result of ranges::iter_move.
            * testsuite/24_iterators/customization_points/92894.cc: New test.
            * testsuite/24_iterators/indirect_callable/92894.cc: New test.

diff --git a/libstdc++-v3/include/bits/iterator_concepts.h b/libstdc++-v3/include/bits/iterator_concepts.h
index b598532089e..e221ec70367 100644
--- a/libstdc++-v3/include/bits/iterator_concepts.h
+++ b/libstdc++-v3/include/bits/iterator_concepts.h
@@ -89,6 +89,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       struct _IMove
       {
       private:
+	template<typename _Tp>
+	  struct __result
+	  { using type = iter_reference_t<_Tp>; };
+
+	template<typename _Tp>
+	  requires __adl_imove<_Tp>
+	  struct __result<_Tp>
+	  { using type = decltype(iter_move(std::declval<_Tp>())); };
+
+	template<typename _Tp>
+	  requires (!__adl_imove<_Tp>)
+	  && is_lvalue_reference_v<iter_reference_t<_Tp>>
+	  struct __result<_Tp>
+	  { using type = remove_reference_t<iter_reference_t<_Tp>>&&; };
+
 	template<typename _Tp>
 	  static constexpr bool
 	  _S_noexcept()
@@ -100,16 +115,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  }
 
       public:
-	template<typename _Tp>
-	  requires __adl_imove<_Tp> || requires(_Tp& __e) { *__e; }
-	  constexpr decltype(auto)
+	// The result type of iter_move(std::declval<_Tp>())
+	template<std::__detail::__dereferenceable _Tp>
+	  using __type = typename __result<_Tp>::type;
+
+	template<std::__detail::__dereferenceable _Tp>
+	  constexpr __type<_Tp>
 	  operator()(_Tp&& __e) const
 	  noexcept(_S_noexcept<_Tp>())
 	  {
 	    if constexpr (__adl_imove<_Tp>)
 	      return iter_move(static_cast<_Tp&&>(__e));
-	    else if constexpr (is_reference_v<iter_reference_t<_Tp>>)
-	      return std::move(*__e);
+	    else if constexpr (is_lvalue_reference_v<iter_reference_t<_Tp>>)
+	      return static_cast<__type<_Tp>>(*__e);
 	    else
 	      return *__e;
 	  }
@@ -123,10 +141,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   } // namespace ranges
 
   template<__detail::__dereferenceable _Tp>
-    requires requires(_Tp& __t)
-    { { ranges::iter_move(__t) } -> __detail::__can_reference; }
+    requires __detail::
+      __can_reference<ranges::__cust_imove::_IMove::__type<_Tp&>>
     using iter_rvalue_reference_t
-      = decltype(ranges::iter_move(std::declval<_Tp&>()));
+      = ranges::__cust_imove::_IMove::__type<_Tp&>;
 
   template<typename> struct incrementable_traits { };
 
@@ -443,13 +461,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       using __iter_concept = typename __iter_concept_impl<_Iter>::type;
 
   template<typename _In>
-    concept __indirectly_readable_impl = requires(const _In __in)
+    concept __indirectly_readable_impl = requires
       {
 	typename iter_value_t<_In>;
 	typename iter_reference_t<_In>;
 	typename iter_rvalue_reference_t<_In>;
-	{ *__in } -> same_as<iter_reference_t<_In>>;
-	{ ranges::iter_move(__in) } -> same_as<iter_rvalue_reference_t<_In>>;
+	requires same_as<iter_reference_t<const _In>,
+			 iter_reference_t<_In>>;
+	requires same_as<iter_rvalue_reference_t<const _In>,
+			 iter_rvalue_reference_t<_In>>;
       }
       && common_reference_with<iter_reference_t<_In>&&, iter_value_t<_In>&>
       && common_reference_with<iter_reference_t<_In>&&,
diff --git a/libstdc++-v3/testsuite/24_iterators/customization_points/92894.cc b/libstdc++-v3/testsuite/24_iterators/customization_points/92894.cc
new file mode 100644
index 00000000000..197268fe5e3
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/customization_points/92894.cc
@@ -0,0 +1,52 @@
+// Copyright (C) 2020 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/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <iterator>
+
+using namespace std;
+
+// Define our own of version of indirectly_readable_impl here,
+// to check the use of iter_move even if the real concept in
+// <bits/iterator_concepts.h> no longer uses iter_move.
+template<class In>
+concept indirectly_readable_impl
+  = requires(const In in)
+      {
+	typename iter_value_t<In>;
+	typename iter_reference_t<In>;
+	typename iter_rvalue_reference_t<In>;
+	{ *in } -> same_as<iter_reference_t<In>>;
+	{ ranges::iter_move(in) } -> same_as<iter_rvalue_reference_t<In>>;
+      };
+
+template<class T> requires indirectly_readable_impl<projected<T*, identity>>
+  void algo(T)
+  { }
+
+void
+test01()
+{
+  // PR libstdc++/92894
+  // Verify that the use of range::iter_move above doesn't cause odr-use of
+  // projected<local-class-type, identity>::operator* (which is not defined).
+  struct X { };
+  X a;
+  algo(a);
+}
diff --git a/libstdc++-v3/testsuite/24_iterators/indirect_callable/92894.cc b/libstdc++-v3/testsuite/24_iterators/indirect_callable/92894.cc
new file mode 100644
index 00000000000..3408c76bde1
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/indirect_callable/92894.cc
@@ -0,0 +1,55 @@
+// Copyright (C) 2020 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/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <iterator>
+
+using std::projected;
+using std::identity;
+using std::indirect_unary_predicate;
+
+template<typename T,
+	 indirect_unary_predicate<projected<T*, identity>> Pred>
+  constexpr void
+  all_of(T*, Pred)
+  { }
+
+void
+test01()
+{
+  // PR libstdc++/92894
+  struct X { };
+  X x;
+  all_of(&x, [](X&) { return false; });
+}
+
+template<class R, class Proj = identity,
+	 indirect_unary_predicate<projected<R, Proj>> Pred>
+  constexpr void
+  find_if(R, Pred, Proj = {})
+  { }
+
+void
+test02()
+{
+  // PR 94241
+  struct s { int m; };
+  s r[] = { s{0}, s{1}, s{2}, s{3} };
+  find_if(r, [](auto const) { return true; });
+}

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] libstdc++: Replace deduced return type in ranges::iter_move (PR 92894)
  2020-05-01 13:28 ` Jonathan Wakely
@ 2020-05-01 13:47   ` Jonathan Wakely
  0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Wakely @ 2020-05-01 13:47 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Patrick Palka, Jakub Jelinek

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

On 01/05/20 14:28 +0100, Jonathan Wakely wrote:
>On 01/05/20 13:03 +0100, Jonathan Wakely wrote:
>>The deduced return type causes the instantiation of the function body,
>>which can then require the instantiation of std::projected::operator*
>>which is intentionally not defined.
>>
>>This patch uses a helper trait to define the return type, so that the
>>function body doesn't need to be instantiated. That helper trait can
>>then also be used in other places that currently check the return type
>>of ranges::iter_move (iter_rvalue_reference_t and indirectly_readable).
>>
>>2020-05-01  Jonathan Wakely  <jwakely@redhat.com>
>>	    Patrick Palka  <ppalka@redhat.com>
>>
>>	PR libstdc++/92894
>>	* include/bits/iterator_concepts.h (ranges::__cust_imove::_IMove):
>>	Add trait to determine return type and an alias for it.
>>	(ranges::__cust_imove::_IMove::operator()): Use __result instead of
>>	deduced return type.
>>	(iter_rvalue_reference_t): Use _IMove::__type instead of checking
>>	the result of ranges::iter_move.
>>	(__detail::__indirectly_readable_impl): Use iter_rvalue_reference_t
>>	instead of checking the result of ranges::iter_move.
>>	* testsuite/24_iterators/indirect_callable/92894.cc: New test.
>>
>>Patrick, I prefer this to the patch you added to the bug. Avoiding
>>doing overload resolution on ranges::iter_move(x) seems worthwhile.
>>
>>What do you think?
>>
>>I *think* the changes to __indirectly_readable_impl are equivalent. As
>>far as I can tell, the point of the original { *in }
>>and { ranges::iter_move(in) } constraints are to check that const In
>>and In do the same thing. We could leave the { *in } on, but for
>>consistency it seems better to change both.
>
>Here's a slightly improved patch which I'm committing now.
>
>Diffs from the previous patch are removing the useless parameter list
>from the __indirectly_readable_impl concept:
>
>@@ -460,7 +461,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       using __iter_concept = typename __iter_concept_impl<_Iter>::type;
>
>   template<typename _In>
>-    concept __indirectly_readable_impl = requires(const _In __in)
>+    concept __indirectly_readable_impl = requires
>       {
>        typename iter_value_t<_In>;
>        typename iter_reference_t<_In>;
>
>Fixing 24_iterators/indirect_callable/92894.cc so it actually passes,
>and adding 24_iterators/customization_points/92894.cc to verify that
>iter_move is fixed (which the other tests don't do because
>indirectly_readable no longer uses ranges::iter_move).

This is the patch I'm testing for the gcc-10 branch (if it gets RM
approval). It doesn't do the compile-time optimization for
iter_rvalue_reference_t and __indirectly_readable_impl, it just fixes
the bug in ranges::iter_move. The optimization isn't required for
correctness.



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

commit 838d37183df0592ae1eb814bbf0461e17cf95e33
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri May 1 14:35:12 2020 +0100

    libstdc++: Replace deduced return type in ranges::iter_move (PR 92894)
    
    The deduced return type causes the instantiation of the function body,
    which can then require the instantiation of std::projected::operator*
    which is intentionally not defined.
    
    This patch uses a helper trait to define the return type, so that the
    function body doesn't need to be instantiated.
    
    Unlike on the master branch, this backport to gcc-10 does not change the
    iter_rvalue_reference_t alias template and __indirectly_readable_impl
    concept to use the new trait.
    
    Backport from mainline
    2020-05-01  Jonathan Wakely  <jwakely@redhat.com>
                Patrick Palka  <ppalka@redhat.com>
    
            PR libstdc++/92894
            * include/bits/iterator_concepts.h (ranges::__cust_imove::_IMove):
            Add trait to determine return type and an alias for it.
            (ranges::__cust_imove::_IMove::operator()): Use __result instead of
            deduced return type.
            * testsuite/24_iterators/customization_points/92894.cc: New test.
            * testsuite/24_iterators/indirect_callable/92894.cc: New test.

diff --git a/libstdc++-v3/include/bits/iterator_concepts.h b/libstdc++-v3/include/bits/iterator_concepts.h
index b598532089e..c5b6247cde7 100644
--- a/libstdc++-v3/include/bits/iterator_concepts.h
+++ b/libstdc++-v3/include/bits/iterator_concepts.h
@@ -89,6 +89,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       struct _IMove
       {
       private:
+	template<typename _Tp>
+	  struct __result
+	  { using type = iter_reference_t<_Tp>; };
+
+	template<typename _Tp>
+	  requires __adl_imove<_Tp>
+	  struct __result<_Tp>
+	  { using type = decltype(iter_move(std::declval<_Tp>())); };
+
+	template<typename _Tp>
+	  requires (!__adl_imove<_Tp>)
+	  && is_lvalue_reference_v<iter_reference_t<_Tp>>
+	  struct __result<_Tp>
+	  { using type = remove_reference_t<iter_reference_t<_Tp>>&&; };
+
 	template<typename _Tp>
 	  static constexpr bool
 	  _S_noexcept()
@@ -100,16 +115,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  }
 
       public:
-	template<typename _Tp>
-	  requires __adl_imove<_Tp> || requires(_Tp& __e) { *__e; }
-	  constexpr decltype(auto)
+	// The result type of iter_move(std::declval<_Tp>())
+	template<std::__detail::__dereferenceable _Tp>
+	  using __type = typename __result<_Tp>::type;
+
+	template<std::__detail::__dereferenceable _Tp>
+	  constexpr __type<_Tp>
 	  operator()(_Tp&& __e) const
 	  noexcept(_S_noexcept<_Tp>())
 	  {
 	    if constexpr (__adl_imove<_Tp>)
 	      return iter_move(static_cast<_Tp&&>(__e));
-	    else if constexpr (is_reference_v<iter_reference_t<_Tp>>)
-	      return std::move(*__e);
+	    else if constexpr (is_lvalue_reference_v<iter_reference_t<_Tp>>)
+	      return static_cast<__type<_Tp>>(*__e);
 	    else
 	      return *__e;
 	  }
diff --git a/libstdc++-v3/testsuite/24_iterators/customization_points/92894.cc b/libstdc++-v3/testsuite/24_iterators/customization_points/92894.cc
new file mode 100644
index 00000000000..197268fe5e3
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/customization_points/92894.cc
@@ -0,0 +1,52 @@
+// Copyright (C) 2020 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/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <iterator>
+
+using namespace std;
+
+// Define our own of version of indirectly_readable_impl here,
+// to check the use of iter_move even if the real concept in
+// <bits/iterator_concepts.h> no longer uses iter_move.
+template<class In>
+concept indirectly_readable_impl
+  = requires(const In in)
+      {
+	typename iter_value_t<In>;
+	typename iter_reference_t<In>;
+	typename iter_rvalue_reference_t<In>;
+	{ *in } -> same_as<iter_reference_t<In>>;
+	{ ranges::iter_move(in) } -> same_as<iter_rvalue_reference_t<In>>;
+      };
+
+template<class T> requires indirectly_readable_impl<projected<T*, identity>>
+  void algo(T)
+  { }
+
+void
+test01()
+{
+  // PR libstdc++/92894
+  // Verify that the use of range::iter_move above doesn't cause odr-use of
+  // projected<local-class-type, identity>::operator* (which is not defined).
+  struct X { };
+  X a;
+  algo(a);
+}
diff --git a/libstdc++-v3/testsuite/24_iterators/indirect_callable/92894.cc b/libstdc++-v3/testsuite/24_iterators/indirect_callable/92894.cc
new file mode 100644
index 00000000000..3408c76bde1
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/indirect_callable/92894.cc
@@ -0,0 +1,55 @@
+// Copyright (C) 2020 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/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <iterator>
+
+using std::projected;
+using std::identity;
+using std::indirect_unary_predicate;
+
+template<typename T,
+	 indirect_unary_predicate<projected<T*, identity>> Pred>
+  constexpr void
+  all_of(T*, Pred)
+  { }
+
+void
+test01()
+{
+  // PR libstdc++/92894
+  struct X { };
+  X x;
+  all_of(&x, [](X&) { return false; });
+}
+
+template<class R, class Proj = identity,
+	 indirect_unary_predicate<projected<R, Proj>> Pred>
+  constexpr void
+  find_if(R, Pred, Proj = {})
+  { }
+
+void
+test02()
+{
+  // PR 94241
+  struct s { int m; };
+  s r[] = { s{0}, s{1}, s{2}, s{3} };
+  find_if(r, [](auto const) { return true; });
+}

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-05-01 13:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-01 12:03 [PATCH] libstdc++: Replace deduced return type in ranges::iter_move (PR 92894) Jonathan Wakely
2020-05-01 13:28 ` Jonathan Wakely
2020-05-01 13:47   ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).