[m-dev.] for review: improve DD user interface

[Mark Anthony BROWN]

Fergus Henderson writes:
> 
> > +confirm_bug(Store, Bug, Response, Diagnoser0, Diagnoser) -->
> > +	{
> > +		Bug = e_bug(Node),
> > +		Message = "Incorrect node found:\n"
> > +	;
> > +		Bug = i_bug(Node),
> > +		Message = "Inadmissible call node found:\n"
> > +	},
> 
> I'm not sure that "node" is the ideal term to describe
> these things to the user.

You are right.  It would be better to describe i-bugs as
"inadmissible call", and e-bugs as "incorrect contour" (like incorrect
clause), or "partially uncovered atom", depending on the type of node.
However, the best way to do this is for the analyser to return a value
which represents the bug directly rather than return a node and let the
diagnoser interpret that node.  Such a modification would be more closely
related to the analyser interface than to the representation of trace
atoms, so I'd prefer to leave it for a separate change if that is ok.

> 
> > +++ browser/declarative_execution.m	2000/02/20 05:48:31
> > @@ -92,20 +92,16 @@
> >  		)
> >  	.
> >  
> > -	% If either of the following two types are modified, some of
> > -	% the macros in trace/mercury_trace_declarative.h may need
> > -	% to be updated.
> > -	%
> >  :- type trace_atom
> >  	--->	atom(
> >  			string,			% Procedure name.
> > -			list(univ)		% Arguments.
> > -			% XXX we also need to store some information about
> > -			% where the arguments come from, since they will
> > -			% not necessarily be in the right order or all
> > -			% present (we do not store unbound variables).
> > +			list(maybe(univ))	% Arguments.
> >  		).
> 
> Please put an XXX there saying that this representation
> won't handle partially instantiated data structures.

Done.

> > +:- func construct_trace_atom(string, int) = trace_atom.
> > +:- pragma export(construct_trace_atom(in, in) = out,
> > +		"MR_DD_construct_trace_atom").
> > +
> > +construct_trace_atom(Functor, Arity) = atom(Functor, Args) :-
> > +	list__duplicate(Arity, no, Args).
> > +
> > +:- func add_trace_atom_arg(trace_atom, int, univ) = trace_atom.
> > +:- pragma export(add_trace_atom_arg(in, in, in) = out,
> > +		"MR_DD_add_trace_atom_arg").
> > +
> > +add_trace_atom_arg(atom(F, Args0), Num, Val) = atom(F, Args) :-
> > +	list__replace_nth_det(Args0, Num, yes(Val), Args).
> 
> That is not a particularly efficient way of constructing
> a list -- for N arguments it's O(N^2).
> However, I suppose it probably doesn't matter
> (that's not in the inner loop, right?).

The advantage of the way I have chosen is that the interface between
the back end and the Mercury code is kept fairly simple and flexible.
Also, the C code that calls the Mercury is fairly simple---it doesn't
need to add arguments in any particular order, and it doesn't need
to do anything for free arguments.  So I'd prefer to leave the
implementation as it is unless and until profiling says that it is
a problem.

> 
> > +:- pred user_confirm_bug_help(user_state, io__state, io__state).
> > +:- mode user_confirm_bug_help(in, di, uo) is det.
> > +
> > +user_confirm_bug_help(user(_, OutStr)) -->
> > +	io__write_strings(OutStr, [
> > +		"Answer one of:\n",
> > +		"\ty\tyes\t\tconfirm that the suspect is a bug\n",
> > +		"\tn\tno\t\tdo not accept that the suspect is a bug\n",
> > +%		"\tb\tbrowse\t\tbrowse the suspect\n",
> > +		"\ta\tabort\t\tabort this diagnosis session\n",
> > +		"\th, ?\thelp\t\tthis help message\n"
> > +	]).
> 
> Does the abort command return to the mdb prompt?
> If so, it might be a good idea to mention that
> to the explanation above.
> 

Done.

> > +/*
> > +** If the variable specified by n is a head variable, then return
> > +** its argument position, otherwise return an error.
> > +*/
> > +
> > +extern	const char	*MR_trace_headvar_num(int n, int *num);
> 
> The comment here is potentially misleading: you don't return the
> argument position, you store the argument position in *num and return
> NULL.
> 

Fixed.

Here is the relative diff:

--- /home/pgrad/dougl/ws1/mercury/browser/declarative_execution.m	Mon Feb 21 14:01:07 2000
+++ declarative_execution.m	Mon Feb 21 17:40:06 2000
@@ -94,8 +94,16 @@
 
 :- type trace_atom
 	--->	atom(
-			string,			% Procedure name.
-			list(maybe(univ))	% Arguments.
+				% Procedure name.
+				%
+			string,
+
+				% Arguments.
+				% XXX this representation will not be
+				% able to handle partially instantiated
+				% data structures.
+				%
+			list(maybe(univ))
 		).
 
 	% If the following two type is modified, some of
--- /home/pgrad/dougl/ws1/mercury/browser/declarative_user.m	Mon Feb 21 14:01:09 2000
+++ declarative_user.m	Mon Feb 21 18:02:15 2000
@@ -155,7 +155,8 @@
 		"\ts\tskip\t\tskip this question\n",
 		"\tr\trestart\t\task the skipped questions again\n",
 %		"\tb\tbrowse\t\tbrowse the atom\n",
-		"\ta\tabort\t\tabort this diagnosis session\n",
+		"\ta\tabort\t\t",
+			"abort this diagnosis session and return to mdb\n",
 		"\th, ?\thelp\t\tthis help message\n"
 	]).
 
@@ -168,7 +169,8 @@
 		"\ty\tyes\t\tconfirm that the suspect is a bug\n",
 		"\tn\tno\t\tdo not accept that the suspect is a bug\n",
 %		"\tb\tbrowse\t\tbrowse the suspect\n",
-		"\ta\tabort\t\tabort this diagnosis session\n",
+		"\ta\tabort\t\t",
+			"abort this diagnosis session and return to mdb\n",
 		"\th, ?\thelp\t\tthis help message\n"
 	]).
 
--- /home/pgrad/dougl/ws1/mercury/trace/mercury_trace_vars.h	Mon Feb 21 14:01:19 2000
+++ mercury_trace_vars.h	Mon Feb 21 17:46:15 2000
@@ -77,17 +77,18 @@
 extern	const char	*MR_trace_list_vars(FILE *out);
 
 /*
-** Return the name, type and value of the specified variable in the specified
-** locations, except those which are NULL.  Variable number n must be
-** in the range 1..MR_trace_var_count().
+** Return as a side effect the name, type and value of the specified
+** variable in the specified locations, except those which are NULL.
+** Variable number n must be in the range 1..MR_trace_var_count().
 */
 
 extern	const char	*MR_trace_return_var_info(int n, const char **name_ptr,
 				Word *type_info_ptr, Word *value_ptr);
 
 /*
-** If the variable specified by n is a head variable, then return
-** its argument position, otherwise return an error.
+** If the variable specified by n is a head variable, then store
+** its argument position in *num and return NULL, otherwise return
+** an error.
 */
 
 extern	const char	*MR_trace_headvar_num(int n, int *num);

-- 
Mark Brown, PhD student            )O+  |  "Another of Fortran's breakthroughs
(m.brown at cs.mu.oz.au)                   |  was the GOTO statement, which was...
Dept. of Computer Science and Software  |  uniquely simple and understandable"
Engineering, University of Melbourne    |              -- IEEE, 1994
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------