Re: [PATCH][_GLIBCXX_DEBUG] Remove useless checks

[Jonathan Wakely <jwakely@redhat.com>]

On Mon, 23 Jan 2023 at 18:15, François Dumont <frs.dumont@gmail.com> wrote:
>
> On 23/01/23 10:22, Jonathan Wakely wrote:
> > On Mon, 23 Jan 2023 at 06:02, François Dumont via Libstdc++
> > <libstdc++@gcc.gnu.org> wrote:
> >>       libstdc++: [_GLIBCXX_DEBUG] Remove useless constructor checks
> >>
> >>       Creating a safe iterator from a normal iterator is done within the
> >> library where we
> >>       already know that it is done correctly. The rare situation where a
> >> user would use safe
> >>       iterators for his own purpose is non-Standard code so outside
> >> _GLIBCXX_DEBUG scope. For
> >>       those reasons the __msg_init_singular is useless and can be removed.
> >>
> >>       Additionally in the copy constructor used for post-increment and
> >> post-decrement operators
> >>       the __msg_init_copy_singular check can also be ommitted because of
> >> the preliminary
> >>       __msg_bad_inc and __msg_bad_dec checks.
> >>
> >>       libstdc++-v3/ChangeLog:
> >>
> >>               * include/debug/safe_iterator.h
> >> (_Safe_iterator<>::_Unsafe_call): New.
> > I don't like the name "unsafe call". Why is it unsafe? As you say
> > above, we don't need to check because we know that it's only called in
> > a context where it's safe. Can we call it _Unchecked instead of
> > _Unsafe_call? That seems like a more accurate description of the
> > behaviour.
> >
> >
> >>               (_Safe_iterator(const _Safe_iterator&, _Unsafe_call): New.
> >>               (_Safe_iterator::operator++(int)): Use latter.
> >>               (_Safe_iterator::operator--(int)): Likewise.
> >>               (_Safe_iterator(_Iterator, const _Safe_sequence_base*)):
> >> Remove !_M_insular()
> >>               check.
> >>               * include/debug/safe_local_iterator.h
> >> (_Safe_local_iterator<>::_Unsafe_call):
> >>               New.
> >>               (_Safe_local_iterator(const _Safe_local_iterator&,
> >> _Unsafe_call): New.
> >>               (_Safe_local_iterator::operator++(int)): Use latter.
> >>               * src/c++11/debug.cc (_S_debug_messages): Add as comment
> >> the _Debug_msg_id
> >>               entry associated to the array entry.
> > These comments are a great idea, thanks.
> >
> > If you agree with the _Unchecked name, OK to commit with that change.
> >
> It's unsafe because it's unchecked so _Unchecked is fine for me too :-)

But it doesn't need to be checked, because we know it's safe in all
the places that it's used. So it's not unsafe :-)

In some industries the words "safe" and "unsafe" have special meaning.
Calling something "unsafe" when actually it's "safe by construction"
will just raise red flags and unnecessary pain if an automated audit
scans for the word "unsafe". Some poor person will have to fill out a
form to say "it's not unsafe, it's just called that in the code" to be
able to use the code.

> Committed with the requested change.

Great, thanks.