This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] libstdc++: istreambuf_iterator proxy (was: keep attached streambuf)
- From: Petr Ovtchenkov <ptr at void-ptr dot info>
- To: François Dumont <frs dot dumont at gmail dot com>
- Cc: libstdc++ at gcc dot gnu dot org, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 6 Oct 2017 21:00:49 +0300
- Subject: Re: [PATCH] libstdc++: istreambuf_iterator proxy (was: keep attached streambuf)
- Authentication-results: sourceware.org; auth=none
- References: <E1dveaR-0006O7-My@void-ptr.info> <20170928103425.GG4582@redhat.com> <20170928150643.2f667ec9@void-ptr.info> <20170928123806.GN4582@redhat.com> <20171003233927.6b5a151c@void-ptr.info> <41ed5a91-9b23-97a4-ab38-4728091279d7@gmail.com>
On Fri, 6 Oct 2017 18:01:36 +0200
François Dumont <frs.dumont@gmail.com> wrote:
> ...
> >>> The test itself simulate "stop and go" istream usage.
> >>> stringstream is convenient for behaviuor illustration, but in "real life"
> >>> I can assume socket or tty on this place.
> >> At the very minimum we should have a comment in the test explaining
> >> how it relies on non-standard, non-portable behaviour.
> >>
> >> But I'd prefer to avoid introducing more divergence from other
> >> implementations.
> > Standard itself say nothting about "stop and go" scenario.
> > At least I don't see any words pro or contra.
> > But current implementation block usage of istreambuf_iterator
> > with underlying streams like socket or tty, so istreambuf_iterator become
> > almost useless structure for practice.
> Why not creating a new istreambuf_iterator each time you need to check
> that streambuf is not in eof anymore ?
It's strange question. Because EOL (end-of-life) for istreambuf_iterator
object after EOF of associated streambuf
- not directly follow from standard, just follow from (IMO wrong)
implementations; and this is a balk for useful usage of istreambuf_iterator;
- violate expectations from point of view of C++ objects life cycle;
- require destruction and construction of istreambuf_iterator
object after check for equality with istreambuf_iterator()
(strange trick).
>
> > We have three issues with istreambuf_iterator:
> > - debug-dependent behaviour
> Fixed.
+ __glibcxx_requires_cond(_M_sbuf,
_M_message(__gnu_debug::__msg_inc_istreambuf)
._M_iterator(*this));
vs
+ __glibcxx_requires_cond(_M_sbuf && !_S_is_eof(_M_sbuf->sgetc()),
_M_message(__gnu_debug::__msg_inc_istreambuf)
._M_iterator(*this));
and
+ __glibcxx_requires_cond(_M_sbuf
+ && !traits_type::eq_int_type(_M_c,traits_type::eof()),
_M_message(__gnu_debug::__msg_inc_istreambuf)
._M_iterator(*this));
vs
+ __glibcxx_requires_cond(_M_sbuf && !_S_is_eof(_M_sbuf->sgetc()),
_M_message(__gnu_debug::__msg_inc_istreambuf)
._M_iterator(*this));
I'm insist on the first variant. It free from code that may lead to different
behaviour between debug/non-debug (like _M_sbuf->sgetc()).
> > - EOL of istreambuf_iterator when it reach EOF (but this not mean
> > EOL of associated streambuf)
> Controversial.
> > - postfix increment operator return istreambuf_iterator, but here
> > expected restricted type, that accept only dereference, if it possible.
> I agree that we need to fix this last point too.
>
> Consider this code:
>
> std::istringstream inf("abc");
> std::istreambuf_iterator<char> j(inf), eof;
> std::istreambuf_iterator<char> i = j++;
>
> assert( *i == 'a' );
>
> At this point it looks like i is pointing to 'a' but then when you do:
>
> std::string str(i, eof);
>
> you have:
> assert( str == "ac" );
No. I mean that in last (my) suggestion ([PATCH])
std::istreambuf_iterator<char> i = j++;
is impossible ("impossible" is in aggree with standard).
So test test01() in 2.cc isn't correct.
>
> We jump other the 'b'.
>
> We could improve the situation by adding a debug assertion that _M_c is
> eof when pre-increment is being used or by changing semantic of
> pre-increment to only call sbumpc if _M_c is eof. But then we would need
> to consider _M_c in find overload and in some other places in the lib I
> think.
>
> Rather than going through this complicated path I agree with Petr that
> we need to simply implement the solution advised by the Standard with
> the nested proxy type.
>
> This is what I have done in the attached patch in a naive way. Do we
> need to have abi compatibility here ? If so I'll rework it.
>
> This patch will make libstdc++ pass the llvm test. I even duplicate it
> on our side with a small refinement to check for the return value of the
> proxy::operator*().
>
> François
>
--
- ptr