Re: [lmi] [PATCH] Add undisplayable_exception

[Greg Chicares]

On 2014-10-09 00:41Z, Vadim Zeitlin wrote:
> On Wed, 08 Oct 2014 15:33:35 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2014-10-06 18:08Z, Vadim Zeitlin wrote:
> GC> [...]
> GC> > +class undisplayable_exception
> GC> 
> GC> I get a segfault in a case that shouldn't be allowed to arise.
> 
>  Sorry, I hadn't thought about the possibility of throwing such exception
> from non-testing code. To be honest, I think this is unlikely to happen by
> accident,

Yet it could happen due to carelessness...

> especially if I update the comment to make it more explicit that
> this exception should be used only in the testing code. And maybe it should
> be renamed to something scarier, e.g. habitually_fatal_exception.

...and failure to heed good documentation would be careless. You or I
wouldn't do that, especially after this discussion; but someone else
might be working on lmi someday. What happens when you have a big red
(physical) pushbutton with a sign below it that says
  "NEVER PRESS THIS BUTTON UNDER ANY CIRCUMSTANCES"
? See:
  
https://www.google.com/search?q=raymond%27s+last+day+as+the+band%27s+sound+technician&tbm=isch
That's why I'm struggling with this patch--so I really welcome your
suggestion here:

>  But if this is not enough, I can think of another, arguably better
> solution: instead of modifying report_exception() to know about this
> exception, could we make report_exception() itself customizable? E.g.
> delegate its work to some function pointer, if it's non-NULL?

Just a thought: why not avoid the function pointer (so we never have to
set it, or check whether it has been set), and instead declare, e.g.:
  class wx_test_exception { public: explicit wx_test_exception(std::string); };
in a header used only by the GUI-test program, and then implement the
ctor only in a translation unit that's linked only into that program?

But I don't mind if you disregard that idea as long as we do this:

> This would
> allow to not mention undisplayable_exception in the main program code at
> all and make it local to main_wx_test.cpp and so impossible to throw from
> anywhere else -- and, in any case, the main program would report all
> exceptions, avoiding the problem entirely, as it would use exactly the same
> report_exception() as now.

Excellent idea. Let's do it that way, keeping the production system unchanged.

Thus, the only remaining patch I need to apply to the production system is the
one for LMI_ASSERT_MESSAGE and LMI_ASSERT_EQUAL (along with your "trivial patch"
below), and then you can send me a vast monolithic GUI-testing patch and I'll
quickly apply that.

>  And I'd also like to ask for the permission to commit the trivial patch
> below, which shows the message inside the dialog and not in its title bar
> (I realize that caring about such details in a fatal abort message is akin
> to rearranging chairs on the desk of a sinking liner, but this is still not
> a reason to get them wrong).

Humans have a poor record of distinguishing that case from wasting time
checking the lifeboats on a liner that can't possibly sink.

I'll apply that [snipped] patch soon. Right now, we expect to issue an
interim release soon, so we're under a continual code freeze; that's
why I prefer to do this myself, so that I'm contributing a second pair
of eyes.