[m-rev.] for review: handle EXCP nodes in wrong and missing answer diagnosis

[Mark Brown]

On 06-Jan-2005, Ian MacLarty <maclarty at cs.mu.OZ.AU> wrote:
> For review by anyone.
> 
> Estimated hours taken: 0.5
> Branches: main
> 
> Include EXCP nodes as wrong and missing answer children, instead of aborting.
> 
> browser/declarative_tree.m
> 	Instead of aborting when an EXCP node is encountered, add it as
> 	a child for both missing and wrong answer diagnosis.
> 
> tests/debugger/declarative/catch.exp
> tests/debugger/declarative/catch.inp
> 	This test doesn't abort with an error message anymore, but
> 	asks if the caught exception was expected.
> 
> 	Note that I have not adjusted catch.exp2 since the catch test has been
> 	failing in the decldebug grade for a different reason, namely because 
> 	tracing information is not being included in the stack for 
> 	builtin_catch.

There needs to be a whole lot more test cases, utilising each of the
variants of 'try'.

> 
> Index: browser/declarative_tree.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/browser/declarative_tree.m,v
> retrieving revision 1.11
> diff -u -r1.11 declarative_tree.m
> --- browser/declarative_tree.m	16 Dec 2004 00:12:39 -0000	1.11
> +++ browser/declarative_tree.m	4 Jan 2005 01:04:18 -0000
> @@ -327,11 +327,9 @@
>  		throw(internal_error("wrong_answer_children_2",
>  			"unexpected start of contour"))
>  	;
> -		Node = excp(_, _, _, _, _)
> -	->
> -		throw(unimplemented_feature("code that catches exceptions"))
> -	;
> -		Node = exit(_, _, _, _, _, _)
> +		( Node = exit(_, _, _, _, _, _)
> +		; Node = excp(_, _, _, _, _)
> +		)
>  	->
>  			%
>  			% Add a child for this node.

(I've discussed this with Ian in person; I'm posting it here to document
the discussion.)

This behaviour is unsound.  For example if the exception event is reached
via a call to try_all, then the explanation of the wrong answer might not
be that the exception was unexpected, but that one of the answers generated
prior to the exception was wrong.  These answers will not be in the contour
that is being traversed at this point.  (You should make sure that this
example is covered by test cases.)

You will need to switch to something like missing answer analysis when an
exception is reached here.  However, it isn't really missing answer analysis
so the name missing_answer_children is the wrong name to use.  Probably the
best solution would be to replace it with "stratum_children" similarly to
how you've added "contour_children" in your follow up to this diff.

> @@ -425,12 +423,9 @@
>  		throw(internal_error("missing_answer_children_2",
>  			"unexpected start of contour"))
>  	;
> -		Node = excp(_, _, _, _, _)
> -	->
> -		throw(unimplemented_feature("code that catches exceptions"))
> -	;
>  		( Node = exit(_, _, _, _, _, _)
>  		; Node = fail(_, _, _, _)
> +		; Node = excp(_, _, _, _, _)
>  		)
>  	->
>  			%

Similarly, what you are doing here isn't really missing answer analysis,
since there won't be any fail events in the stratum being traversed.  Again,
"stratum_children" is probably the right concept.

Please post a full diff (including the change to add contour_children, which
I haven't yet reviewed in detail).  Make sure you do a cvs update first,
since there have been other changes committed since you posted this.

Cheers,
Mark.

--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------