[PATCH] streambuf_iterator: avoid debug-dependent behaviour

[François Dumont]

Gentle reminder, ok to commit ?

     * include/bits/streambuf_iterator.h
     (istreambuf_iterator<>(const istreambuf_iterator&)): Remove useless
     noexcept qualificatio<n on defaulted implementation.
     (istreambuf_iterator<>::operator*()): Do not capture iterator state
     in Debug assertion.
     (istreambuf_iterator<>::operator++()): Likewise.
     (istreambuf_iterator<>::operator++(int)): Likewise.
     (istreambuf_iterator<>::_M_get()): Simplify implementation.
     (istreambuf_iterator<>::_S_at_eof()): New.
     (istreambuf_iterator<>::_M_at_eof()): Adapt, use latter.
     (find(istreambuf_iterator<>, istreambuf_iterator<>, _CharT)):
     Return an iterator with _M_c set to eof to capture streambuf state
     on evaluation.
     (testsuite/24_iterators/istreambuf_iterator/2.cc): Add checks.

François

On 09/09/2017 22:16, François Dumont wrote:
> Hi
>
>     Completing the execution of tests revealed a lot about the current 
> implementation.
>
>     The main point of current implementation is to delay as much as 
> possible the capture of the current streambuf position. So my original 
> proposal capturing state on instantiation was wrong.
>
>     This new proposal concentrate on the debug-dependent code. Debug 
> assertions now avoids calling _M_at_eof() which also capture iterator 
> state. It also simplifies _M_get() method a little bit like Petr 
> proposed but keeping the _M_sbuf reset when reaching eof. Thanks to 
> this work I realized that std::find specialization could also be 
> simplified by returning a streambuf_iterator which will capture 
> current streambuf state on evaluation.
>
>      Note that I haven't been able to create a test case revealing the 
> problem. This is more a code quality issue as current code violates 
> the principal that debug asserts shouldn't impact object state. AFAIK 
> this is noticeable only under gdb.
>
>     Regarding avoiding the reset of _M_sbuf it might be possible,your 
> test case could be a good reason to do so. But this is going to be a 
> big change for current implementation so don't forget to run all 
> testsuite and to consider the std::copy and std::find specializations.
>
> Tested under Linux x86_64, normal and debug modes.
>
> Ok to commit ?
>
> François
>
>
> On 08/09/2017 07:47, Petr Ovtchenkov wrote:
>> -gcc-patches
>>
>> On Thu, 7 Sep 2017 23:02:15 +0200
>> François Dumont <frs.dumont@gmail.com> wrote:
>>
>>> +    _M_c = _M_sbuf->sgetc();
>>> +    if (_S_at_eof(_M_c))
>>> +      _M_sbuf = 0;
>> _M_sbuf = 0; <--- Is not what I axpect here.
>>
>> Suggestions will be later, after we finish copyright assignment for
>> changes procedure (in progress).
>>
>> WBR,
>>
>> -- 
>>
>>     - ptr
>>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: istreambuf_iterator.patch
Type: text/x-patch
Size: 5021 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20170921/3c4bbb26/attachment.bin>