Boost logo

Boost :

From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2019-12-05 09:30:43

On 2019-12-05 12:05, Andrzej Krzemienski wrote:
> czw., 5 gru 2019 o 09:59 Andrey Semashev via Boost
> <boost_at_[hidden] <mailto:boost_at_[hidden]>> napisał(a):
> On 2019-12-05 11:15, Alexander Grund via Boost wrote:
> >
> >> ```
> >> void fixed_string<N>::resize(size_type s)
> >> {
> >>    BOOST_FIXED_STRING_PRECONDITION(s <= this->capacity());
> >>    // then do the job
> >> }
> >> ```
> >
> > +1 on that. I'm always advocating for safe-by-default and found it a
> > huge mistake to make operator[] the unchecked one instead of at()
> >
> > So using BOOST_FIXED_STRING_PRECONDITION which throws by default
> is the
> > right choice IMO.
> I'm strongly opposed. Make it a BOOST_ASSERT if you like but no checks
> in release mode, please.
> What's the point of this check when your index is guaranteed to not
> exceed size()-1?
> In my whole programming practice, not once did I need at(). Not only
> because I didn't need the check at this point, but also because even if
> I did need a check at some point before operator[] call, I also was not
> satisfied with the exception at() would throw.
> Are you opposing against the idea of user-controlled
> BOOST_FIXED_STRING_PRECONDITION() in general, or to throwing by default
> of to performing runtime-checks in release builds regardless of what
> action is taken later?

I'm strongly opposed to runtime checks regardless of the error outcome
in release mode (ok, when NDEBUG is defined) because most of the time
you already know the index is within bounds. When you don't know, you
most likely should have check that earlier (e.g. before the parsing loop
or before your higher level operation starts).

I'm opposed to throwing an exception in case of failure because the
exception and its error description is likely meaningless to high level
code and nearly useless when debugging. The higher level code may not
want an exception to begin with, in which case it would perform the
check itself, and the check in operator[] becomes useless.

I'm mildly opposed to using custom macros (other than NDEBUG) to modify
behavior at compile time, because it opens up the possibility of
configuration inconsistencies and ODR issues. I'm fine with it as long
as by default (when nothing custom is defined) the behavior is
equivalent to BOOST_ASSERT.

> BOOST_ASSERT() does perform checks in release builds unless you go and
> define NDEBUG, which does not correspond 1-to-1 to release builds.

NDEBUG gets defined by default in release mode by many IDEs and build
systems, it is the standard macro to disable asserts, including the
standard assert(). In practice, NDEBUG and release mode nearly always go
hand in hand and NDEBUG is perceived as an indication of a release build.

Boost list run by bdawes at, gregod at, cpdaniel at, john at