Re: [review][fixed_string] Andrzej's feedback

From: Alexander Grund (alexander.grund_at_[hidden])
Date: 2019-12-05 09:45:20

> 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).

That was always my counter argument: If you can prove it to a reviewer 
looking at your code that the index is in-bounds, then the compiler can 
do the same and remove the check and exception.

I never bought "most of the time you already know the index is within 
bounds" because it is equivalent to "most of the time your code is 
correct, let's not use checks/unit tests/hardening/..."
If what you say is true, then buffer overflows would be non-existant. 
Are they?

> 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.
No, the check gets optimized away if you do everything right. If you 
don't (do it right and have no check/exception/abort) you'll get UB, 
buffer overflows and security risks. Especially for a stack entity this 
will pretty much always be a SERIOUS security issue.
So to protect users one would need at least std::abort, but using an 
exception allows to recover from it.
> 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.
ODR is a real issue, true that because you may end up using a compiled 
library having it defined differently.