[m-dev.] for review: new method of handling failures, part 7 of 6 :-(

[Fergus Henderson]

Here's a review of the changes to the runtime/ and tests/ directories.

On 02-Jul-1998, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> Index: compiler/notes/failure.html
[I haven't reviewed that one yet]

> Index: runtime/mercury_engine.c
> Index: runtime/mercury_grade.h
Fine.

> Index: runtime/mercury_imp.h
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_imp.h,v
> retrieving revision 1.7
> diff -u -r1.7 mercury_imp.h
> --- mercury_imp.h	1998/06/09 02:08:02	1.7
> +++ mercury_imp.h	1998/07/02 06:51:47
> @@ -20,6 +20,20 @@
>  #define MERCURY_IMP_H
>  
>  /*
> +** This test allows Mercury developers who have not done a cvs update
> +** of the changes introducing the new failure handling method in the
> +** code generator to retain the ability to do bootchecks, by adding
> +** -DMR_DISABLE_REDOFR to EXTRA_CFLAGS in their Mmake.stage.params,
> +** and using bootcheck -r.
> +**
> +** The test should be deleted once there are no such developers.
> +*/
> +
> +#ifdef	MR_DISABLE_REDOFR
> +#undef	MR_USE_REDOFR
> +#endif

s/#undef/  #undef/

I don't quite understand this code --
where will MR_USE_REDOFR get #defined in the first place,
thus requiring that we #undef it here?

> Index: runtime/mercury_stack_trace.c

Fine.

> Index: runtime/mercury_stack_trace.h
...
> +extern	void		MR_dump_nondet_stack_from_layout(FILE *fp,
> +				Word *base_maxfr);
> +

You should really document what this function does.
For example, what is the `base_maxfr' parameter?

>  Code	*MR_stack_trace_bottom;
...
> +Word	*MR_nondet_stack_trace_bottom;

These variables should be declared `extern'.

> Index: runtime/mercury_stacks.h
...
> +/* DEFINITIONS FOR MANIPULATING THE NONDET STACK */
> +
> +#ifdef MR_DEBUG_NONDET_STACK
> +#define mkframe_save_prednm(prednm) (curprednm = prednm)
> +#else
> +#define mkframe_save_prednm(prednm) /* nothing */
> +#endif

s/#define/  #define/

> Index: runtime/mercury_trace_internal.c
> +	} else if (streq(words[0], "D")) {
> +		if (word_count == 1) {
> +			const char	*result;
> +
> +			do_init_modules();
> +			MR_dump_nondet_stack_from_layout(stdout,
> +				MR_saved_maxfr(MR_saved_regs));
> +		} else {

The variable `result' is not used.

> +	} else if (streq(words[0], "toggle_echo")) {
> +		MR_supply_newline = !MR_supply_newline;

Does toggle_echo actually cause the commands to be
echoed, or just it just result in the printing of
a newline?  If the latter, then the command name should
be changed.  If the former, then this code looks wrong.

> Index: tests/hard_coded/Mmakefile
> ===================================================================
> RCS file: /home/mercury1/repository/tests/hard_coded/Mmakefile,v
> retrieving revision 1.34
> diff -u -r1.34 Mmakefile
> --- Mmakefile	1998/06/22 01:06:38	1.34
> +++ Mmakefile	1998/07/01 04:55:49
> @@ -18,6 +18,7 @@
>  	construct \
>  	curry \
>  	curry2 \
> +	cut_test \
>  	deep_copy_bug \
>  	det_in_semidet_cntxt \
>  	division_test \

Apparently you haven't done a `cvs add' for cut_test.m
and cut_test.exp?  At any rate, they didn't appear in the diff.

Oh, I see you listed them at the end.

> Index: tools/bootcheck
...
> @@ -673,6 +673,10 @@
>  
>  	MERCURY_COMPILER=$root/stage2/compiler/mercury_compile
>  	export MERCURY_COMPILER
> +
> +	# just temporarily for ws1
> +	# MERCURY_COMPILER=$root/stage1/mercury_compile
> +	# export MERCURY_COMPILER
>  

Probably you didn't intend to commit that change.

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