On 06/05/21 18:28 +0100, Jonathan Wakely wrote:
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 <jwak...@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&&;

Here's what I've pushed to trunk, and will push to gcc-11 after
testing it on the branch.

Tested x86_64-linux and powerpc64le-linux.


commit a87ceadf17b4a899f3e74e2da8b6b209461d2742
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Thu May 6 19:14:42 2021

    libstdc++: Fix constraints for rvalue stream insertion/extraction
    
    The __rval_streamable() function was an attempt to test for
    convertibility cheaply and without confusing diagnostics. It doesn't
    work with Clang though, and is probably ill-formed.
    
    Replace that helper function with a check for derivation from ios_base,
    and use that in the alias templates __rvalue_stream_insertion_t and
    __rvalue_stream_extraction_t. Use concepts for the constraints when
    available.
    
    libstdc++-v3/ChangeLog:
    
            * include/std/istream (__rvalue_stream_extraction_t): Replace
            use of __rval_streamable.
            * include/std/ostream (__rvalue_stream_insertion_t): Likewise.
            (__rval_streamable): Remove.
            (_Require_derived_from_ios_base, __derived_from_ios_base): New
            helper for checking constraints.
            * testsuite/27_io/basic_istream/extractors_other/char/4.cc: Fix
            reference to the wrong subclause of the standard.
            * testsuite/27_io/basic_istream/extractors_other/wchar_t/4.cc:
            Likewise.
            * testsuite/27_io/basic_ostream/inserters_other/char/6.cc:
            Likewise.
            * testsuite/27_io/basic_ostream/inserters_other/wchar_t/6.cc:
            Likewise.
            * testsuite/27_io/basic_ostream/inserters_other/char/99692.cc:
            New test.
            * testsuite/27_io/filesystem/path/io/dr2989.cc: Adjust pruned
            errors.

diff --git a/libstdc++-v3/include/std/istream b/libstdc++-v3/include/std/istream
index ea34cce6298..5ad60dbd709 100644
--- a/libstdc++-v3/include/std/istream
+++ b/libstdc++-v3/include/std/istream
@@ -958,12 +958,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // 2328. Rvalue stream extraction should use perfect forwarding
   // 1203. More useful rvalue stream insertion
 
-  // SFINAE helper to check constraints for operator>>(Istream&&, T&&).
-  // If the constraints are satisfied, it is an alias for Istream&&.
-  template<typename _Is, typename _Tp,
-	   typename = decltype(std::__rval_streamable<_Is>()
-				 >> std::declval<_Tp>())>
+#if __cpp_lib_concepts
+  template<typename _Is, typename _Tp>
+    requires __derived_from_ios_base<_Is>
+      && requires (_Is& __is, _Tp&& __t) { __is >> std::forward<_Tp>(__t); }
     using __rvalue_stream_extraction_t = _Is&&;
+#else
+  template<typename _Is, typename _Tp,
+	   typename = _Require_derived_from_ios_base<_Is>,
+	   typename = decltype(std::declval<_Is&>() >> std::declval<_Tp>())>
+    using __rvalue_stream_extraction_t = _Is&&;
+#endif
 
   /**
    *  @brief  Generic extractor for rvalue stream
diff --git a/libstdc++-v3/include/std/ostream b/libstdc++-v3/include/std/ostream
index fdd2a87665c..981697324c9 100644
--- a/libstdc++-v3/include/std/ostream
+++ b/libstdc++-v3/include/std/ostream
@@ -708,31 +708,29 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // _GLIBCXX_RESOLVE_LIB_DEFECTS
   // 1203. More useful rvalue stream insertion
 
-  // SFINAE helper to check constraints for operator<<(Ostream&&, const T&).
-  // If Ostream is publicly and unambiguously derived from ios_base, then
-  // __rval_streamable<Ostream>() is equivalent to declval<Ostream&>().
-  // Otherwise, it results in a substitution failure. Specifically, it will
-  // fail if Ostream is an lvalue reference or the same type as ios_base.
-  // Use concepts if possible because they're cheaper to evaluate.
 #if __cpp_lib_concepts
+  // Use concepts if possible because they're cheaper to evaluate.
   template<typename _Tp>
-    requires (!is_same_v<_Tp, ios_base>)
-      && requires (_Tp* __t, ios_base* __b) { __b = __t; }
-    _Tp&
-    __rval_streamable();
-#else
-  template<typename _Tp,
-	   typename = _Require<__not_<__is_one_of<_Tp, _Tp&, ios_base>>>>
-    _Tp&
-    __rval_streamable(ios_base* = (_Tp*)nullptr);
-#endif
+    concept __derived_from_ios_base = is_class_v<_Tp>
+      && (!is_same_v<_Tp, ios_base>)
+      && requires (_Tp* __t, ios_base* __b) { __b = __t; };
 
-  // SFINAE helper to check constraints for operator<<(Ostream&&, const T&).
-  // If the constraints are satisfied, it is an alias for Ostream&&.
-  template<typename _Os, typename _Tp,
-	   typename = decltype(std::__rval_streamable<_Os>()
-				 << std::declval<const _Tp&>())>
+  template<typename _Os, typename _Tp>
+    requires __derived_from_ios_base<_Os>
+      && requires (_Os& __os, const _Tp& __t) { __os << __t; }
     using __rvalue_stream_insertion_t = _Os&&;
+#else
+  template<typename _Tp>
+    using _Require_derived_from_ios_base
+      = _Require<is_class<_Tp>, __not_<is_same<_Tp, ios_base>>,
+		 is_convertible<typename add_pointer<_Tp>::type, ios_base*>>;
+
+  template<typename _Os, typename _Tp,
+	   typename = _Require_derived_from_ios_base<_Os>,
+	   typename
+	     = decltype(std::declval<_Os&>() << std::declval<const _Tp&>())>
+    using __rvalue_stream_insertion_t = _Os&&;
+#endif
 
   /**
    *  @brief  Generic inserter for rvalue stream
diff --git a/libstdc++-v3/testsuite/27_io/basic_istream/extractors_other/char/4.cc b/libstdc++-v3/testsuite/27_io/basic_istream/extractors_other/char/4.cc
index 9f1e293474f..94e41a17e82 100644
--- a/libstdc++-v3/testsuite/27_io/basic_istream/extractors_other/char/4.cc
+++ b/libstdc++-v3/testsuite/27_io/basic_istream/extractors_other/char/4.cc
@@ -17,7 +17,7 @@
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 
-// 27.6.2.5.3 basic_ostream manipulator inserters
+// C++11 27.7.2.6 Rvalue stream extraction [istream.rvalue]
 
 #include <sstream>
 
diff --git a/libstdc++-v3/testsuite/27_io/basic_istream/extractors_other/wchar_t/4.cc b/libstdc++-v3/testsuite/27_io/basic_istream/extractors_other/wchar_t/4.cc
index fc7f5505bf4..b182be7ee3d 100644
--- a/libstdc++-v3/testsuite/27_io/basic_istream/extractors_other/wchar_t/4.cc
+++ b/libstdc++-v3/testsuite/27_io/basic_istream/extractors_other/wchar_t/4.cc
@@ -17,7 +17,7 @@
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 
-// 27.6.2.5.3 basic_ostream manipulator inserters
+// C++11 27.7.2.6 Rvalue stream extraction [istream.rvalue]
 
 #include <sstream>
 
diff --git a/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_other/char/6.cc b/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_other/char/6.cc
index 4801afcba6c..3a748cd630b 100644
--- a/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_other/char/6.cc
+++ b/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_other/char/6.cc
@@ -17,7 +17,7 @@
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 
-// 27.6.2.5.3 basic_ostream manipulator inserters
+// C++11 27.7.3.9 Rvalue stream insertion [ostream.rvalue]
 
 #include <sstream>
 
diff --git a/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_other/char/99692.cc b/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_other/char/99692.cc
new file mode 100644
index 00000000000..e41399a1bf1
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_other/char/99692.cc
@@ -0,0 +1,34 @@
+// { dg-do compile { target c++11 } }
+
+#include <ostream>
+
+struct CustomStream : std::ostream {};
+
+namespace N {
+    class A{};
+}
+
+std::ostream& operator<<(std::ostream& s, const N::A&)
+{
+    return s;
+}
+
+CustomStream&& operator<<(CustomStream&& s, const N::A& v)
+{
+    static_cast<std::ostream&>(s) << v;
+    return std::move(s);
+}
+
+void test_pr99692()
+{
+  // PR libstdc++/99692
+    CustomStream() << N::A{};
+}
+
+int test_shift_ios_enum()
+{
+  // https://gcc.gnu.org/pipermail/libstdc++/2021-May/052507.html
+  int i = 1 << std::ios::erase_event;
+
+  return i;
+}
diff --git a/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_other/wchar_t/6.cc b/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_other/wchar_t/6.cc
index 3efeb804b00..8fc0694dddc 100644
--- a/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_other/wchar_t/6.cc
+++ b/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_other/wchar_t/6.cc
@@ -17,7 +17,7 @@
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 
-// 27.6.2.5.3 basic_ostream manipulator inserters
+// C++11 27.7.3.9 Rvalue stream insertion [ostream.rvalue]
 
 #include <sstream>
 
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/io/dr2989.cc b/libstdc++-v3/testsuite/27_io/filesystem/path/io/dr2989.cc
index 609f1c32a0e..c5cda776477 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/path/io/dr2989.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/path/io/dr2989.cc
@@ -33,4 +33,3 @@ void foo(std::iostream& s) {
   s >> p; // { dg-error "no match" }
 }
 // { dg-prune-output "no type .*enable_if" }
-// { dg-prune-output "no matching function for call to '__rval_streamable" }

Reply via email to