On Thu, Jan 25, 2018 at 12:34 AM, Gavin Lambert via Boost
On 24/01/2018 19:33, Emil Dotchevski wrote:
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.
That can be a simple rule of thumb for many applications and errors. However, it still depends on the application and the kind of error. For example, there are applications where the risk of unintended side-effects like "leaving bad data" after a logic error might not be acceptable and therefore throwing "for cleaning up" might be deemed too risky vs. just terminating. For other applications, there might be security ramification as well, e.g. if you start unwinding when you know your state is corrupted, you could actually be helping an attacker rather than just dying. Therefore, one can easily argue that in the case of logic errors the rule of thumb should be to terminate, rather than to throw; unless you have considered the risks very carefully. You can do the same reasoning on the topic of uncaught exceptions. Miguel