On 12/01/23 13:00, Jonathan Wakely wrote:
On Thu, 12 Jan 2023 at 05:52, François Dumont wrote:
Small update for an obvious compilation issue and to review new test
case that could have lead to an infinite loop if the increment issue was
not detected.

I also forgot to ask if there is more chance for the instantiation to be
elided when it is implemented like in the _Safe_local_iterator:
return { __cur, this->_M_sequence };
No, that doesn't make any difference.

than in the _Safe_iterator:
return _Safe_iterator(__cur, this->_M_sequence);

In the case where the user code do not use it ?

Fully tested now, ok to commit ?

François

On 11/01/23 07:03, François Dumont wrote:
Thanks for fixing this.

Here is the extension of the fix to all post-increment/decrement
operators we have on _GLIBCXX_DEBUG iterator.
Thanks, I completely forgot we have other partial specializations, I
just fixed the one that showed a deadlock in the user's example!

I prefer to restore somehow previous implementation to continue to
have _GLIBCXX_DEBUG post operators implemented in terms of normal post
operators.
Why?

Implementing post-increment as:

     auto tmp = *this;
     ++*this;
     return tmp;

is the idiomatic way to write it, and it works fine in this case. I
don't think it performs any more work than your version, does it?
Why not use the idiomatic form?

Is it just so that post-inc of a debug iterator uses post-inc of the
underlying iterator? Why does that matter?

A little yes, but that's a minor reason that is just making me happy.

Main reason is that this form could produce a __msg_init_copy_singular before the __msg_bad_inc.

And moreover I plan to propose a patch later to skip any check in the call to _Safe_iterator(__cur, _M_sequence) as we already know that __cur is ok here like anywhere else in the lib.

There will still be one in the constructor normally elided unless --no-elide-constructors but there is not much I can do about it.


Reply via email to