Re: [boost] [outcome] success-or-failure objects

Subject: Re: [boost] [outcome] success-or-failure objects
From: Gavin Lambert (gavinl_at_[hidden])
Date: 2018-01-24 23:34:49

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.