On 2019-12-05 12:45, Alexander Grund via Boost wrote:
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.
No, you're giving too much credit to compilers. Compilers cannot analyze at the same level as humans do. For example, if the valid index is stored in a data member in one method of a class and used in another, and the first method is required to be called first (which might also be enforced by a runtime check via another member variable), the compiler is not able to know that the index is always valid. At least, I haven't seen such cleverness.
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?
Mistakes happen, I'm not arguing with that. But defensive programming is not the solution because it doesn't save you from these mistakes. Code that passes invalid index to at() is just as incorrect as the one that does so with operator[].
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.
As I said above, you can't count on that.
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.
You can't recover from an unexpected exception. And it is unexpected by account that we're talking about a human mistake. The correct way of tackling mistakes is human reviewing, testing and debugging. For the debugging part, a crash with a saved backtrace is more useful than an exception with a meaningless error message like "fixed_string::at", as many standard libraries like to do.