[MLton] cvs commit: improved exception history for Overflow

[Stephen Weeks]

> Here is another thought for improving exception history.

I like it.

> Consider a program like the following:
> 
> -------------------------------------------------------------------
> fun loop n =
>    if n > 12345
>       then let 
> 	      val () = raise Fail "here"
> 	   in 2
> 	   end
>    else 1 + (loop (4 * n))
>         handle Overflow => 1
> 
> val _ = loop 1
> -------------------------------------------------------------------
> 
> With -const 'Exn.keepHistory true' we get:
> 
> unhandled exception: Fail: here
> with history:
> 	loop z.sml 1.5
> 	loop z.sml 1.5
> 	loop z.sml 1.5
> 	loop z.sml 1.5
> 	loop z.sml 1.5
> 	loop z.sml 1.5
> 	loop z.sml 1.5
> 	loop z.sml 1.5
> 
> This is fine, but I find it a little disappointing that the actual line
> where the exception is raised is not in the history.

I've implemented your suggestion, and here is what is now displayed.

unhandled exception: Fail: here
with history:
        loop.raise z.sml 4.24
        loop z.sml 1.5
        loop z.sml 1.5
        loop z.sml 1.5
        loop z.sml 1.5
        loop z.sml 1.5
        loop z.sml 1.5
        loop z.sml 1.5
        loop z.sml 1.5

> That eta expansion seems to be something that could easily be
> done in implementExceptions. 

I don't think eta-expansion in XML is the way to go.  The
Enter/Leave's have already been inserted, so going this way would mean
adding source position info to XML.  It seems more direct to insert
Enter/Leave pairs (not eta-expand) at the same pass where all the
other Enter/Leave stuff is done.

> Actually, since elaborate took care of inserting the profile
> statements, the "eta expansion" would simply be to wrap the raise
> expression with a new Enter/Leave pair.  Note, we don't want to do
> the eta expansion in or before elaborate for a couple of reasons:
>  1) The compiler inserts Bind and Match exceptions after elaborate

True.

>  2) The default profile insertion of elaborate would name the function 
>     "anon" or some such, whereas if we do it later, we could give it a 
>     better name of "raise".

Elaborate chooses the name when it inserts the Enter/Leave (see
elaborate-core.fun), so it can do anything it wants.

I ended up going for a two-piece solution that didn't require changing
any ILs.  The elaborator wraps an Enter/Leave around each raise
expression, just as it does around each function body.  The (still
horribly misnamed) defunctorizer wraps an Enter/Leave around each
raise {Bind,Match} that the match compiler generates.  Fortunately, we
already had the source position info there for the warnings generated
by match compilation.

> On the other hand, adding the profile expressions in implementExceptions 
> is late enough in the game that we've lost the distinction between the 
> basis and the user code.  So, that means that _all_ raised exceptions 
> would report the origin, not just those that were raised in user code.  
> This is different than the current behavior where an exception raised in 
> the basis is only given its point of origin when -profile-basis true is a 
> compile option.  On the other hand, it is exactly one more entry in the 
> history, so it doesn't seem that bad.

This is true of the approach I took too.  And I agree; it doesn't seem
bad to me.