[PATCH] libstdc++: Replace deduced return type in ranges::iter_move (PR 92894)

[Jonathan Wakely]

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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 6842 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20200501/68dcf941/attachment.bin>