[m-dev.] for review: add extras/exceptions

[Fergus Henderson]

On 22-Jul-1998, Tyson Dowd <trd at stimpy.cs.mu.oz.au> wrote:
> > +
> > +% Note that throwing an exception across the C interface won't work --
> > +% if you try to do that, you will get undefined behaviour.
> 
> Please explain "across the C interface" a little better.

How about the following?

% Note that throwing an exception across the C interface won't work.
% That is, if a Mercury procedure that is exported to C using `pragma export'
% throws an exception which is not caught within that procedure, then
% you will get undefined behaviour. 


Incidentally, with the current implementation the behaviour will
actually be to jump to nearest enclosing handler.  The trouble is
just that you get resource leaks. 

One resource that leaks is C stack space, since it doesn't restore the
C stack pointer.  It would be possible to fix that without too much
trouble, and with almost no overhead.

But the other kind of resource that leaks is resources allocated in the
C code that threw an exception over -- this can include malloc()ed
memory, file descriptors, and other OS resources.  The fix for this is
not nearly so simple.  I think the only solution to that would be to
change the C interface so that e.g. semidet Mercury procedures return
an enum { Succeed, Failed, GotException } instead of bool.
And this would also have potentially significant overhead, I think --
every call from C to Mercury would need to do a catch, basically.

Another alternative would be to provide some way for C code to
catch Mercury exceptions, by exporting a version of `try_io' to C.
This would probably have a quite high overhead, but it would
avoid any "distributed fat" -- you'd only pay the price if you
were actually interested in trying to avoid resource leaks in C
code in the case of an exception, and it would avoid the need
to complicate the ordinary C interface.  I suppose this is
probably the best approach to take.

Anyway, I don't plan to address these issues in this commit.

> > +%-----------------------------------------------------------------------------%
> > +
> > +% Since this module is implemented using C code with `BEGIN_MODULE()'
> > +% and `END_MODULE', and because it is not part of the standard library,
> > +% to use this module you need the following two lines in your Mmakefile:
> > +%
> > +%	RM_C=:
> > +%	C2INITFLAGS=$($(subst _init.c,,$@).cs)
> > +%
> > +% This ensures that the module initialization code will be run.
> > +% (Actually these two lines are needed only in certain grades, e.g.
> > +% for profiling.)
> 
> One for the wishlist -- make this cleaner.

Actually that code is a bit old; the current mechanism for handling
this stuff is a bit cleaner.  I've changed the C2INITFLAGS to
use the new mechanism:

	C2INITFLAGS=--extra-inits

Is that better?

Something else we could do is to add a `libexception'
target to the Mmakefile, move the test cases into a subdirectory,
and make the test cases use the `libexception' library.
Then code that uses the exception module wouldn't need to
have `C2INITFLAGS=--extra-inits' in its Mmakefile -- only the
Mmakefile for `libexception' would need that.  But the client Mmakefile
would still need `C2INITFLAGS=$(LIBEXCEPTION_DIR)/exception.init',
as well as a bunch of other stuff that you need to use a library,
which is not much of an improvement.

> > +%
> > +:- pred try_io(pred(T, io__state, io__state), exception_result(T),
> > +						io__state, io__state).
> > +:- mode try_io(pred(out, di, uo) is det,      out(cannot_fail),
> > +						di, uo) is cc_multi.
> > +:- mode try_io(pred(out, di, uo) is cc_multi, out(cannot_fail),
> > +						di, uo) is cc_multi.
> > +
> 
> I find this ugly.
> 
> :- pred try_io(pred(T, io__state, io__state),
> 			exception_result(T), io__state, io__state).
> :- mode try_io(pred(out, di, uo) is det,
> 			out(cannot_fail), di, uo) is cc_multi.
> :- mode try_io(pred(out, di, uo) is cc_multi,
> 			out(cannot_fail), di, uo) is cc_multi.
> 
> Perhaps this is a little easier to read?  (not much, I admit!).

Good suggestion, I will reindent the declarations for `try_io' accordingly.

> > +:- pred get_determinism_2(pred(T, io__state, io__state), determinism).
> > +:- mode get_determinism_2(pred(out, di, uo) is det,      out(bound(det)))
> > +	is cc_multi.
> > +:- mode get_determinism_2(pred(out, di, uo) is cc_multi, out(bound(cc_multi)))
> > +	is cc_multi.
> > +
> > +% Unfortunately the only way to implement get_determinism/2 is to use
> > +% the C interface.
> 
> Is this because the wishlist item "different code for different mode"
> isn't done yet? 

Yes.  I've updated the comment to make this clear.

> > +rethrow(succeeded(_)) :-
> > +	error("rethrow/1: invalid argument").
> > +rethrow(failed) :-
> > +	error("rethrow/1: invalid argument").
> 
> More informative error messages please.
> 
> rethrow(succeeded(_)) :-
> 	error("rethrow/1: invalid argument: succeeded(_)").
> rethrow(failed) :-
> 	error("rethrow/1: invalid argument: failed").

Done.

> Would it make sense to have rethrow(in(bound(exception(...)))) and
> avoid these (or rather, force the caller to avoid these?

I thought about that.  But often you will want to write

	( Exception = 

> > +:- pred very_unsafe_perform_io(pred(T, io__state, io__state), T).
> > +:- mode very_unsafe_perform_io(pred(out, di, uo) is det, out) is det.
> > +:- mode very_unsafe_perform_io(pred(out, di, uo) is cc_multi, out)
> > +								is det.
> > +% Mercury doesn't support impure higher-order pred terms, so if we want
> > +% to form a closure from unsafe_perform_io, as we need to do above,
> > +% then we must (falsely!) promise that it is pure.
> > +:- pragma promise_pure(very_unsafe_perform_io/2). % XXX this is a lie
> > +
> 
> I can find no mention of this limitation of purity anywhere (reference
> manual, LIMITATIONS, purity.m).  It would be good to be in all three.
> (How many wishes to do I get?).

This is a good idea, except I think it should be in the reference manual
and purity.m but not in LIMITATIONS.  The idea of the LIMITATIONS file
is that it documents implementation limitations -- places where the
implementation does not implement the language as documented in the
reference manual -- whereas this one should IMHO be considered a
language design limitation rather than an implementation limitation.
(Fixing the limitation would require adding new builtin types
`impure pred', `impure pred(T)', which is clearly a language extension...)

But it's not related to exception handling as such, so it should be a
separate change.  I'm hoping that Peter Schachte will do this one ;-)

> > +Declare_label(mercury__exception__builtin_catch_3_2_i1);
...
> 
> Please add MR_MAKE_STACK_LAYOUT_ENTRY(...) and
> MR_MAKE_STACK_LAYOUT_INTERNAL(...) for all these labels.

Shall do.

> > +Define_entry(mercury__exception__builtin_catch_3_0); /* det */
> > +#ifdef PROFILE_CALLS
> > +{
> > +	tailcall(ENTRY(mercury__exception__builtin_catch_3_2), 
> > +		ENTRY(mercury__exception__builtin_catch_3_0));
> > +}
> > +#endif
> > +Define_entry(mercury__exception__builtin_catch_3_2); /* cc_multi */
> 
> Once upone a time PROFILE_CALLS was the only code that needed this,
> but it's now useful for stack traces (e.g. debug grade)

Are you sure?  How is it useful for stack traces?

> > +#ifdef COMPACT_ARGS
> > +	FRAMEVARS->handler = r3;	/* save the Handler closure */
> > +#else
> > +	FRAMEVARS->handler = r4;	/* save the Handler closure */
> > +#endif
> 
> I prefer the approach of
> 	#ifdef COMPACT_ARGS
> 	  #define handler_reg r3
> 	#else
> 	  #define handler_reg r4
> 	#endif
> particularly as you have more conditional code below.
> 
> Actually, COMPACT_ARGS makes a real mess of all this code.  Are you
> sure we need to support it?

Well, we definitely need to support the COMPACT_ARGS convention.
The question is whether we need to support the non-COMPACT_ARGS convention.

I think it's best to leave it in for now.  It can be deleted if/when
we delete all the other stuff in the compiler, runtime, and library
that supports non-COMPACT_ARGS.

> > +:- pred nondet_fail(string::out) is nondet.
> > +nondet_fail("nondet_fail 1") :- fail.
> > +nondet_fail("nondet_fail 2") :- fail.
> > +
> > +:- pred cc_nondet_fail(string::out) is cc_nondet.
> > +cc_nondet_fail("cc_nondet_fail") :- fail.
> > +cc_nondet_fail("cc_nondet_succeed 2") :- fail.
> 
> Is that right?  
> Shouldn't it be "cc_nondet_fail 1" and  "cc_nondet_fail 2"

Yes, that would probably be better.  I've changed it.

> (hopefully it will always pick the same one and we won't need
> a non-det testcase).

Actually the clauses always fail, so the output is ignored.
It's a pretty silly test case I suppose.

I'll post a relative diff shortly, and then I'll commit my changes.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.