On Fri, 6 Oct 2017 18:01:36 +0200 François Dumont <frs.dum...@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