On 24/01/2018 19:33, Emil Dotchevski wrote:
Yes. Notably, assert does not throw.
BOOST_ASSERT can be configured to throw. This is useful.
I feel we're going in circles. If you define behavior for a given condition, it is not a precondition and from the viewpoint of the function it is not a logic error if it is violated (because it is designed to deal with it).
Don't get me wrong, I'm not saying that it is a bad idea to write functions which throw when they receive "bad" arguments. I'm saying that in this case they should throw exceptions other than logic_error because calling the function with "bad" arguments is not necessarily logic error; it is something the caller can, correctly, rely on.
I'm not talking about documented checks where the method expressly promises to throw on certain inputs in all cases. As you said, they're not preconditions. (They're also at higher risk of creating exceptions-as-control-flow, which is also a bad idea.) I'm talking about a method or class that can be configured in "performance mode" where methods have rigid preconditions that are not checked at all (and thus cause UB if violated by the caller), vs. "safety mode" where the preconditions do get checked and something well-defined happens on failure (typically one of abort(), throw, return error; ideally caller-selectable). Often classes will implement these things but tied to assert() and debug builds. Sometimes you want an "instrumented" build, which is optimised like release builds but still contains these checks. Sometimes you want the process to abort the current operation but still continue running as a whole if a check fails rather than instantly halt. Sometimes you want to be able to log as many failures as possible at once because the cycle time of fixing one thing and re-testing takes too long. There are many reasons why this can be useful.
How can you guarantee this? In some other language, maybe. In C/C++, you can't.
I cannot think of a case where it would be false that does not require some kind of memory corruption as a prerequisite. Granted, memory corruption is vastly easier to achieve in C/C++ than in some other languages, but modern C++ is trying to discourage practices that lead to it.
If it is a logic error for the caller to not expect this state, you have no idea what the caller will end up doing and you most definitely can't expect you can safely throw.
You can always safely throw. Sometimes it will have poor consequences (eg. leaking memory, leaving bad data, abandoning locks, aborting the process), but those are all well-defined consequences and are themselves side effects of poor coding practices that can be corrected. So that doesn't make any sense. If you call r.error() on an r that doesn't have an error, there are four choices: 1. No checking. This is performance optimised and leads to UB. (In Ye Olde Outcome, it would have returned a garbage bit pattern that could cause all sorts of weird logic, though probably not any far-reaching harm due to the way error_code itself works. In Outcome v2 it's most likely going to just return a default-initialised one, which is entirely harmless.) 2. Check and assert. This aborts the process if you do something wrong. This is great for developer builds. Not so great if it happens in production (due to insufficient testing). 3. Check and throw. This will *also* abort the process if left uncaught, but otherwise admits the possibility of unwinding an operation and moving on to the next operation, which will perhaps not encounter the same problem (as if the problem were commonly exercised it would have been found sooner). 4. Check and return. This makes it a defined behaviour of the method and thus isn't a logic error any more, so doesn't need further consideration. #1 has the potential to ruin everybody's data and corrupt all the things. But its faster as long as *all* callers obey the rules. #2 is "abandon ship!" mode. It's a good default behaviour for most applications, but is often an over-reaction to things that were prevented before they caused memory corruption. Worse, if implemented using assert() then it will devolve to #1 in release builds. #3 also defaults to "abandon ship!" mode, but allows for the possibility of a controlled shutdown or of abandoning a subtask and moving on, in the cases where tasks are reasonably separated and don't depend on each other. (And even in cases where they do depend on each other, you can detect that a task depends on a task that failed and mark it as errored as well.) #3 is always the safest option. #2 has gained some popularity because it allows quickly finding issues during development, but it has the wrong fallback behaviour -- it should fall back to #3 by default, and require explicit opt-in for #1 for areas identified as performance-critical in profiling (and have been extensively tested). #2 has also gained some popularity because of people abusing exceptions-as-control-flow and catching and ignoring all exceptions, which is itself a misuse of the tools. But tools being misused does not necessarily mean they are bad tools. The right balance, I think, is to default to #2 in debug builds with a fallback to *#3* in release builds, then get code into a state where *no exceptions are ever actually thrown* (except when exercising unit tests, of course), including when run in production on real data. Then, gradually, as and when *their callers* are proved safe, the checks can be omitted (#1) in the performance-critical paths only.