[m-rev.] for review: syntactically correct goals in decl debug

[Mark Brown]

On 16-Oct-2002, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> On 16-Oct-2002, Mark Brown <dougl at cs.mu.OZ.AU> wrote:
> > -	{ check_trace_atom_size(Indent, Which, TraceAtom, RemSize) },
> > -	(
> > -		{ RemSize > 0 },
> > -		{ IoActions = [] }
> > -	->
> > -		write_decl_atom_direct(User ^ outstr, TraceAtom, Which)
> > -	;
> > -		write_decl_atom_limited(User, DeclAtom, Which)
> > -	).
> > +	{ maybe_filter_headvars(Which, Args0, Args1) },
> > +	{ list__map(trace_atom_arg_to_univ, Args1, Args) },
> > +		%
> > +		% Call the term browser to print the atom as a goal.
> > +		%
> > +	browse__print_synthetic(Functor, Args, is_function(PredOrFunc),
> > +		User ^ outstr, CallerType, User ^ browser),
> > +	write_io_actions(User, IoActions).
> 
> The comment here is misleading. print_synthetic doesn't always print the atom
> fully, since it imposes a size limit.

Fixed.

> 
> >  mdb> dd
> > -pred app
> > -	[4, 5]
> > -	[6, 7, 8]
> > -	[4, 5, 6, 7, 8]
> > +app([|](4, [|](5, [])), [|](6, [|](7, [|]/2)), [|](4, [|](5, [|]/2)))
> 
> It is nice that we print the atom as an atom, but printing lists like that
> looks like a regression.

The previous behaviour was inconsistent with the behaviour of 'print' in
mdb, because the declarative debugger was checking the size of each
argument against the threshold for using io__write, whereas 'print'
checks the size of the whole goal against the threshold.  The behaviour
is now consistent, since the two operations now use the same code.

> The default size is also too small, since some of the
> arguments are abbreviated.

That is the real problem (as well as the fact that we don't use nice
list syntax).  It can be fixed separately; I'm not too concerned about the
loss of information in the test cases, because there is still sufficient
information remaining for the tests to be effective.

> Could I also ask you to change browse__print_common to check for lists
> and to print them nicely? That could be done in a separate change, or as part
> of this one.

I'll do it separately.

> We should also have a test case in tests/debugger/declarative that tests the
> printing of the atoms of function calls.

There is one: func_call.  It didn't appear in the diff because some
goals were already being printed as goals and the behaviour in these
cases has not changed; that test is one of those cases.  Note that,
although the behaviour has not changed, the implementation has, so this
test case does test the new code.

> Apart from those points, the change is fine.

I'll commit it now, with the following modification:

diff -u browser/declarative_user.m browser/declarative_user.m
--- browser/declarative_user.m	15 Oct 2002 10:49:42 -0000
+++ browser/declarative_user.m	16 Oct 2002 02:33:03 -0000
@@ -497,7 +497,8 @@
 	{ maybe_filter_headvars(Which, Args0, Args1) },
 	{ list__map(trace_atom_arg_to_univ, Args1, Args) },
 		%
-		% Call the term browser to print the atom as a goal.
+		% Call the term browser to print the atom (or part of it
+		% up to a size limit) as a goal.
 		%
 	browse__print_synthetic(Functor, Args, is_function(PredOrFunc),
 		User ^ outstr, CallerType, User ^ browser),
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------