Re: [lmi] logic vs runtime errors

[Greg Chicares]

On 2018-09-07 13:52, Vadim Zeitlin wrote:
> On Fri, 7 Sep 2018 13:16:13 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2018-09-07 01:22, Vadim Zeitlin wrote:
> GC> >  So at the very least I'd replace this runtime_error with logic_error.

Okay, agreed now.

> GC> I don't know a workable algorithm for deciding which to use. C++17 (N4659)
> GC> says [22.2p2-3]:
> GC> 
> GC> | 2 The distinguishing characteristic of logic errors is that they are
> GC> |   due to errors in the internal logic of the program. In theory, they
> GC> |   are preventable.
> GC> | 3 By contrast, runtime errors are due to events beyond the scope of
> GC> |   the program. They cannot be easily predicted in advance.

[...example: rows per group entered as zero in a GUI program that is a
client of a shared library containing class paginator, because the GUI
neglected to validate that input field...]

> GC> What sort of error is zero rows per group, and where? In the GUI
> GC> client, it could have been prevented as above, so it's a logic_error
> GC> there, except that it doesn't arise there--it's the library that
> GC> throws the exception. But the library can't prevent it AFAICS: it's
> GC> not an error "in the internal logic" of the library, so it's not a
> GC> logic_error there; it must instead be a runtime_error.
> 
>  Why do you think that that "logic error" means "error in the internal
> logic"?

Because the C++17 standard [22.2p2] says so:

| 2 The distinguishing characteristic of logic errors is that they are
|   due to errors in the internal logic of the program.
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> GC> I'm thinking that if a library throws a logic_error, that means there's
> GC> a defect in the library, but in this case the library authors cannot
> GC> even in theory prevent it, so it can't be a logic error.
> 
>  I disagree with this interpretation. I don't say it's not right, but it's
> simply not useful, as the conclusion made above from it shows. IOW from
> this point of view, logic_error is indeed completely useless and
> runtime_error should always be used instead.

That's exactly what my viewpoint has been.

>  Of course, this is the interpretation that the standard library uses: when
> std::vector::at() throws a logic_error (out_of_range), it doesn't indicate
> that there is a bug in std::vector at all, but that there is a bug in the
> code using it.

Excellent, that's a concrete point that I can understand. I was trying to
interpret "internal logic" so as to give a natural meaning to "internal",
i.e., intrinsic to the library itself. But this counterexample, and the
objection to concluding that logic_error is completely useless, lead me
to adopt your POV, even though it seems to require a tortured reading of
the word "internal".

> GC> (2) If we distinguish two types of exceptions, then why not go a level
> GC> deeper into the standard hierarchy and use the seven leaf classes only?
> 
>  This is a more interesting, but OTOH also much less important, question.
> Before addressing the "interesting" part, let me know why do I think it's
> not worth spending too much time on this: after many years (decades?) of
> using C++, I don't remember ever needing to handle either logic_error or
> runtime_error subclasses differently at the catch site.

Then are you saying that you have seen a case where it's useful to
handle classes logic_error and runtime_error (those classes themselves,
as opposed to their derived classes) differently at the catch site?
Can you give a concrete example?

> The difference
> between the 2 parent classes is real and important (IMO), while the
> difference between their subclasses just isn't, in practice. I.e. I can't
> imagine any situation in which I'd want to handle domain_error and
> out_of_range differently. Can you?

No, but neither can I name a situation in which I'd want to handle
logic_error and runtime_error differently.

>  The general rule would be: in case of precondition violation, use the most
> specific logic_error subclass which applies. But I agree that there can
> well be situations in which the right choice isn't totally clear (even if
> this one is not one of them). IMHO it's not really a problem however,
> because it's perfectly fine to just use logic_error itself

Okay, that can be distilled into a simple rule that I can understand:

"Prefer logic_error to runtime_error for precondition violations"

I.e., in such cases where we're going to write one or the other, write
logic_error instead of runtime_error. But otherwise, most importantly
where we're going to write an assertion, the assertion is perfectly okay:

    // in 'math_functions.hpp':
  LMI_ASSERT(0 != denominator); // This is notionally a logic_error.
    // in 'multiple_cell_document.cpp':
  LMI_ASSERT(3 == i_nodes.size()); // Malformed input file: runtime_error.

The assertion macro actually throws std::runtime_error in both cases,
and we needn't change that.

> or, if you
> prefer, define lmi_precondition_error deriving from it and always use this
> one instead.

I read a suggestion like that in a textbook once, and wrote this:

ihs_server7702.cpp:    catch(server7702_implausible_input const& e)
ihs_server7702.cpp:    catch(server7702_inconsistent_input const& e)
ihs_server7702.cpp:    catch(x_product_rule_violated const& e)
ihs_server7702.cpp:    catch(server7702_adjustable_event_forbidden_at_issue 
const& e)
ihs_server7702.cpp:    catch(server7702_guideline_negative const& e)
ihs_server7702.cpp:    catch(server7702_misstatement_of_age_or_gender const& e)

...which now seems regrettable.

> GC>   template<typename T>
> GC>   inline T outward_quotient(T numerator, T denominator)
> GC>   {
> GC>       static_assert(std::is_integral<T>::value);
> GC>       LMI_ASSERT(0 != denominator);
> GC> 
> GC> I think that last quoted line is already ideal and perfect, so
> GC> I don't think a safe_denominator() function can improve upon it;
> 
>  I [slightly] disagree: it's not perfect from the error reporting point of
> view. With the safe_denominator(x, "message if x <= 0") variant, using this
> function would still be preferable as it would provide more information in
> case of failure.

Even though I do agree with the following...

> we're well into the territory of
> diminishing returns by now.

...I'd like to point out that division by negative integers is supported
by this function:

    BOOST_TEST_EQUAL(-1, outward_quotient( 2, -2));
    BOOST_TEST_EQUAL(-1, outward_quotient( 1, -2));
    BOOST_TEST_EQUAL( 0, outward_quotient( 0, -2));
    BOOST_TEST_EQUAL( 1, outward_quotient(-1, -2));
    BOOST_TEST_EQUAL( 1, outward_quotient(-2, -2));