[PATCH] [libcxx] Fix ostream insertion iterator so that it returns an lvalue. Fixes PR20596

[Eric Fiselier]

Hi Howard,

Glad to see your keeping watch. Thanks for the input and I agree on all
points.
I don't want to offer an opinion on if this code should be accepted.
However I want to offer a couple of points on the current implementation.
- The overload accepts const rvalues.
- Only the ostream operator implements the DR. The istream operator is
implemented as specified.

If we choose to keep the current implementation I believe we should
consider disabling the overload for const rvalues and bring the istream
iterator in line with the ostream iterator.

/Eric

P.S. I would prefer if we could keep this conversation on the actual
review. Please consider making an account for future discussion.


On Fri, Aug 8, 2014 at 8:54 PM, Howard Hinnant <howard.hinnant at gmail.com>
wrote:

> If every design decision fastidiously adhered to every single detail of
> the standard, we would never get a better standard next year.
>
> The bug report shows that something that is made easy by libc++ is made
> harder by a strictly conforming implementation.  The bug report does not
> show any downside.  Thus the question we should be asking here is:
>
>     Should libc++ conform to the standard, or should the standard be
> changed to agree with libc++?
>
> The latter is what is often done to improve the standard for the entire
> C++ community.  I’ve personally done this dozens of times.  When you get to
> the meeting, the question is often asked:
>
>    Is there any field experience for this improvement?
>
> If the answer is yes, and it is positive, the committee can be swayed to
> make an improvement.
>
> So, do the people here believe that the standard spec is better than the
> libc++ implementation?  Or do they wish only to blindly follow the current
> standard spec and have no wish for the standard spec to improve in the
> future?
>
> Imho, never deviating from the standard results in mediocrity and
> stagnation in technological progress.  Wildly deviating from the standard
> results in chaos.  And an engineer will seek an optimum between these two
> extremes.
>
> Howard
>
> On Aug 8, 2014, at 10:24 PM, Eric Fiselier <eric at efcs.ca> wrote:
>
> > Hi mclow.lists, danalbert,
> >
> > This patch fixes ostreams operator<< so that it properly returns an
> lvalue reference.
> > It also forces the input and return type to be basic_ostream. This is
> inline with the standard.
> >
> > It appears as though http://cplusplus.github.io/LWG/lwg-closed.html#1203
> was implemented even though it was never accepted (and is now closed).
> >
> > http://reviews.llvm.org/D4835
> >
> > Files:
> >  include/ostream
> >
>  test/input.output/iostream.format/output.streams/ostream.rvalue/return_value.pass.cpp
> >
> > Index: include/ostream
> > ===================================================================
> > --- include/ostream
> > +++ include/ostream
> > @@ -1044,18 +1044,13 @@
> >
> > #ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
> >
> > -template <class _Stream, class _Tp>
> > +template <class _CharT, class _Traits, class _Tp>
> > inline _LIBCPP_INLINE_VISIBILITY
> > -typename enable_if
> > -<
> > -    !is_lvalue_reference<_Stream>::value &&
> > -    is_base_of<ios_base, _Stream>::value,
> > -    _Stream&&
> > ->::type
> > -operator<<(_Stream&& __os, const _Tp& __x)
> > +basic_ostream<_CharT,  _Traits>&
> > +operator<<(basic_ostream<_CharT, _Traits> && __os, const _Tp& __x)
> > {
> >     __os << __x;
> > -    return _VSTD::move(__os);
> > +    return __os;
> > }
> >
> > #endif  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
> > Index:
> test/input.output/iostream.format/output.streams/ostream.rvalue/return_value.pass.cpp
> > ===================================================================
> > --- /dev/null
> > +++
> test/input.output/iostream.format/output.streams/ostream.rvalue/return_value.pass.cpp
> > @@ -0,0 +1,34 @@
> >
> +//===----------------------------------------------------------------------===//
> > +//
> > +//                     The LLVM Compiler Infrastructure
> > +//
> > +// This file is dual licensed under the MIT and the University of
> Illinois Open
> > +// Source Licenses. See LICENSE.TXT for details.
> > +//
> >
> +//===----------------------------------------------------------------------===//
> > +
> > +// <ostream>
> > +
> > +// template <class charT, class traits = char_traits<charT> >
> > +//   class basic_ostream;
> > +
> > +// template <class charT, class traits, class T>
> > +//   basic_ostream<charT, traits>&
> > +//   operator<<(basic_ostream<charT, traits>&& os, const T& x);
> > +
> > +#include <ostream>
> > +#include <type_traits>
> > +
> > +int main()
> > +{
> > +#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
> > +    {
> > +        typedef decltype(std::ostream(nullptr) << "testing...") T;
> > +        static_assert(std::is_same<T,  std::ostream &>::value,  "must
> be lvalue");
> > +    }
> > +    {
> > +        typedef decltype(std::wostream(nullptr) <<  "testing...") T;
> > +        static_assert(std::is_same<T,  std::wostream &>::value,  "must
> be lvalue");
> > +    }
> > +#endif  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
> > +}
> > <D4835.12323.patch>_______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140808/0039369f/attachment.html>