Re: [committed] libstdc++: Implement LWG 1203 for rvalue iostreams

[Jonathan Wakely <jwakely@redhat.com>]

On 06/05/21 18:09 +0100, Jonathan Wakely wrote:
>On 06/05/21 17:55 +0200, Stephan Bergmann wrote:
>>On 30/04/2021 15:48, Jonathan Wakely via Libstdc++ wrote:
>>>This implements the resolution of LWG 1203 so that the constraints for
>>>rvalue stream insertion/extraction are simpler, and the return type is
>>>the original rvalue stream type not its base class.
>>>
>>>Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
>>>
>>>libstdc++-v3/ChangeLog:
>>>
>>>	* include/std/istream (operator>>(Istream&&, x&)): Simplify, as
>>>	per LWG 1203.
>>>	* include/std/ostream (operator<<(Ostream&&, const x&)):
>>>	Likewise.
>>>	* testsuite/27_io/basic_istream/extractors_character/char/lwg2499_neg.cc:
>>>	Adjust dg-error pattern.
>>>	* testsuite/27_io/basic_istream/extractors_character/wchar_t/lwg2499_neg.cc:
>>>	Likewise.
>>>	* testsuite/27_io/basic_istream/extractors_other/char/4.cc: Define
>>>	is_extractable trait to replace std::__is_extractable. Make it
>>>	work with rvalue streams as well as lvalues, to replace f() and
>>>	g() helper functions.
>>>	* testsuite/27_io/basic_istream/extractors_other/wchar_t/4.cc:
>>>	Likewise.
>>>	* testsuite/27_io/basic_ostream/inserters_other/char/6.cc:
>>>	Define is_insertable trait to replace std::__is_insertable. Make
>>>	it work with rvalue streams as well as lvalues, to replace f()
>>>	and g() helper functions.
>>>	* testsuite/27_io/basic_ostream/inserters_other/wchar_t/6.cc:
>>>	Likewise.
>>>	* testsuite/27_io/filesystem/path/io/dr2989.cc: Prune additional
>>>	errors from new constraints.
>>>	* testsuite/27_io/rvalue_streams-2.cc: Remove PR 80675 checks,
>>>	which are no longer expected to compile.
>>>	* testsuite/27_io/rvalue_streams.cc: Adjust existing test.
>>>	Verify LWG 1203 changes.
>>>
>>>Tested powerpc64le-linux. Committed to trunk.
>>
>>FWIW, it looks like this is causing issues for Clang (at least Clang 
>>11 and recent Clang 13 trunk):
>>
>>>$ cat test.cc
>>>#include <ostream>
>>>int i = 1 << std::ios::erase_event;
>>
>>(i.e., using and enum in namespace std),
>>
>>>$ clang++ --gcc-toolchain=~/gcc/trunk/inst -fsyntax-only test.cc
>>>In file included from test.cc:1:
>>>~/gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/12.0.0/../../../../include/c++/12.0.0/ostream:727:33: error: cannot initialize a parameter of type 'std::ios_base *' with an rvalue of type 'int *'
>>>   __rval_streamable(ios_base* = (_Tp*)nullptr);
>>>                               ^ ~~~~~~~~~~~~~
>>>~/gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/12.0.0/../../../../include/c++/12.0.0/ostream:733:25: note: in instantiation of default function argument expression for '__rval_streamable<int, void>' required here
>>>          typename = decltype(std::__rval_streamable<_Os>()
>>>                              ^
>>>~/gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/12.0.0/../../../../include/c++/12.0.0/ostream:748:12: note: in instantiation of default argument for '__rvalue_stream_insertion_t<int, std::ios_base::event>' required here
>>>   inline __rvalue_stream_insertion_t<_Ostream, _Tp>
>>>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>test.cc:2:11: note: while substituting deduced template arguments into function template 'operator<<' [with _Ostream = int, _Tp = std::ios_base::event]
>>>int i = 1 << std::ios::erase_event;
>>>         ^
>>>~/gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/12.0.0/../../../../include/c++/12.0.0/ostream:727:33: note: passing argument to parameter here
>>>   __rval_streamable(ios_base* = (_Tp*)nullptr);
>>>                               ^
>>>1 error generated.
>
>It looks like the failed conversion with the default argument is not
>in the immediate context, so is an error not a substitution failure.
>Clang is probably right, so I'll change it.
>
>The reason I did it that way was to save instantiating
>std::is_convertible but also because it seemed like an easy way to
>avoid confusing diagnostics that say:
>
>error: forming pointer to reference type 'std::basic_ostream<char>&'
>
>for overload resolution failures for operator<< (because those
>diagnostics are already hundreds of lines long and confusing enough
>already).
>
>This seems to work:
>
>--- a/libstdc++-v3/include/std/ostream
>+++ b/libstdc++-v3/include/std/ostream
>@@ -722,9 +722,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>     __rval_streamable();
> #else
>   template<typename _Tp,
>-          typename = _Require<__not_<__is_one_of<_Tp, _Tp&, ios_base>>>>
>+          typename = _Require<__not_<is_same<_Tp, ios_base>>,
>+                              is_convertible<_Tp*, ios_base*>>>
>     _Tp&
>-    __rval_streamable(ios_base* = (_Tp*)nullptr);
>+    __rval_streamable();
> #endif
>
>   // SFINAE helper to check constraints for operator<<(Ostream&&, const T&).
>
>
>I'll finish testing that.

Actually, if the function parameter can't be used for the convertible
check then there's no benefit to that function at all. It's simpler to
just put all the constraints directly on the alias template that also
checks the operator<< expression:

   // SFINAE helper to check constraints for operator<<(Ostream&&, const T&).
   // If the constraints are satisfied, it is an alias for Ostream&&.
#if __cpp_lib_concepts
   // Use concepts if possible because they're cheaper to evaluate.
   template<typename _Os, typename _Tp>
     requires (!is_same_v<_Os, ios_base>)
       && (!is_lvalue_reference_v<_Os>)
       && requires (_Os* __os, ios_base* __b, const _Tp& __t) {
	  __b = __os;
	  *__os << __t;
       }
#else
   template<typename _Os, typename _Tp,
	   typename = _Require<__not_<is_same<_Os, ios_base>>,
			       is_convertible<_Os*, ios_base*>>,
	   typename
	     = decltype(std::declval<_Os&>() << std::declval<const _Tp&>())>
#endif
     using __rvalue_stream_insertion_t = _Os&&;