Re: [lmi] logic vs runtime errors (was: [lmi-commits] master b518132 4/4: Remove the latent defect just added)

[Vadim Zeitlin]

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> > On Fri, 7 Sep 2018 00:45:38 +0000 Greg Chicares <address@hidden> wrote:
GC> > 
GC> > GC> I hope you'll find commit ad23b9ee less obtrusive.
GC> > 
GC> >  Now that I do see it, I can say that I absolutely find it a big
GC> > improvement, but, being what I am, still can't prevent myself from 
thinking
GC> > that it would be even better if we had a safe_denominator() function which
GC> > would really assert in case the precondition is not satisfied because IMO
GC> > this problem should be flagged as an assertion failure, i.e. a programming
GC> > mistake, and not just a run of the mill runtime_error because there is a
GC> > big difference between the former, which has to be fixed, and the latter,
GC> > which is unavoidable and just needs to be handled correctly.
GC> > 
GC> >  So at the very least I'd replace this runtime_error with logic_error. But
GC> > an assertion failure would be even more perfectly suitable here IMHO.
GC> There are at least two distinct matters to discuss here.
GC> 
GC> (1) Which class derived from std::exception would be best here? I do tend
GC> to use runtime_error in almost every situation, treating runtime vs. logic
GC> errors as "who" vs. "whom" in contemporary American English, where case
GC> distinctions continue to be lost and "who" is always acceptable.

 Putting aside my sadness about the fate of "whom", I would hope that a
programming language would follow stricter rules than any natural one
(which is why Perl is so universally hated, BTW: it was consciously
designed to mimic a natural language).

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.

 And while I hadn't known about this part of the standard (I hope you don't
think too badly of me for not remembering the entirely of the Holy Standard
by heart), I would have written almost exactly the same thing if you asked
me to define the difference between the two: this makes perfect sense to me
and I think distinguishing between two classes of errors is very useful.

GC> Suppose I write a standalone GUI program for formatting tables, with
GC> an input screen like:
GC> 
GC>   [spin control] "total rows"
GC>   [spin control] "rows per group"
GC>   [spin control] "max lines per page"
GC> 
GC> and, following the signature
GC>   paginator::paginator(int total_rows, int rows_per_group, int 
max_lines_per_page)
GC> , I constrain the spin controls to be integers. If someone enters "0"
GC> in the middle one, that would seem to be a logic_error, because I
GC> could have prevented it by constraining the middle one to be positive.

 Yes.

GC> However, further suppose that class paginator is in a shared library,
GC> and the GUI program is a physically distinct client of that library.
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"? It can also perfectly well be a logic error in the calling code.
>From the point of view of the library, the important thing is whether the
caller can trivially ensure that the precondition is met or not. In this
example this is obviously the case: the caller can easily check whether
rows_per_group and so must do it.

 On the other end of the spectrum, if the library provided a function
reading from a file, it clearly wouldn't be wise to impose "file can be
read without errors" as a precondition because it's impossible to guarantee
in the caller. So a failure to read from a file provided to the library
function would be a runtime_error and not a logic_error.

GC> Put more tersely: if wxWidgets threw exceptions, and lmi attempts to
GC> instantiate an impossible wxObject of some sort, then which kind of
GC> exception would the library report?

 This depends on why exactly is it impossible, but assuming it's something
trivial, e.g. specifying a nullptr as parent for a non-TLW, then it would
be a logic_error of course. If wxWidgets threw exceptions, all instances of
wxASSERT/wxCHECK/wxFAIL would throw logic_error because none of these
macros is used in a situation where runtime_error would make sense.

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. But IMO, we can usefully
distinguish between the two and this requires understanding logic_error as
a logic error anywhere in the code, not necessarily inside the component
which generates it.

 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.

 To summarize, I'm rather surprised that you would view logic_error in this
way as this is completely incompatible with the way it is used in C++ and,
to be honest, just doesn't seem to be very useful way to view it to me.


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. 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?


GC> Here, I find the standard less helpful--the likeliest four say only:
GC> 
GC> | invalid_argument ... to report an invalid argument
GC> | domain_error ... to report domain errors
GC> | out_of_range ... to report an argument value not in its expected range
GC> | range_error ... to report range errors in internal computations
GC> 
GC> Zero rows per group is...which one? It's an invalid argument, because
GC> valid arguments are positive. It's arguably a domain error, because
GC> the argument's domain is positive integers. That argument is also
GC> not in its expected range. And the immediate problem is division by
GC> zero as a consequence of using this argument in internal computations,
GC> so is it a range_error? Or, more precisely, isn't it domain error in
GC> internal computations, thus partaking of both the domain_error and
GC> the range_error nature, and therefore of both the logic_error and the
GC> runtime_error nature because those are the respective parent classes?

 I think it's relatively clear that in this case it's a domain_error
because it's a particular (and hence more useful than general) case of
invalid_argument and it's not an out_of_range, which means "out of [x, y]
interval range" and doesn't apply to "]0, ∞[" interval case in C++, and a
range_error is not a logic_error in the first place.

GC> I'm not firmly opposed to making this particular one a logic_error;
GC> but I need a sensible rule that I can figure out how to follow.

 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 or, if you
prefer, define lmi_precondition_error deriving from it and always use this
one instead.


GC> >> it would be even better if we had a safe_denominator() function
GC> 
GC> In the case at hand, I wouldn't use it, because I want to make a
GC> stronger "assertion", namely, that rows_per_group is positive:
GC> 
GC>     ,rows_per_group_     {0 < rows_per_group ? rows_per_group : throw yikes}
GC>     ,lines_per_group_    {rows_per_group_ + 1}
GC>     ,groups_per_page_    {(max_lines_per_page_ + 1) / lines_per_group_}
GC> 
GC> Ensuring that the denominator {rows_per_group_ + 1} is nonzero is
GC> a weaker requirement.

 Oops, yes, I should have seen this. I think dividing by a negative integer
is such a rare operation that it wouldn't be a stretch to impose the
(strict) positivity condition in safe_denominator() neither.

GC> As for the general case, consider this code in 'math_functions.hpp':
GC> 
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. I do admit that the difference between the current version
and the "perfect" one is pretty small and we're well into the territory of
diminishing returns by now.

 Regards,
VZ