[m-rev.] for review: Added trust command to mdb

[Zoltan Somogyi]

On 03-Jun-2004, Ian MacLarty <maclarty at cs.mu.OZ.AU> wrote:
> Added a `trust' command to mdb which takes a module name as an argument  
> and
> prevents the declarative debugger from asking questions about any  
> predicates
> or functions declared in that module.  The declarative debugger will  
> assume any
> calls to predicates or functions in the module are correct.

You should do something about the odd wrapping of the mail you send.

The log message is too detailed. The point of the log message is to
give an overview of the change. An overview that is almost as long
as the diff itself doesn't work very well as an overview. For example
for declarative_oracle.m, it suffices to say that you add a database
of trusted modules; the fact that you use it to avoid questions to
the user is implicit in the meaning of "trust".

> browser/declarative_execution.m
> 	Add module_name field to the atom constructor for the trace_atom 
> 	type.
> 	Made necessary changes to predicates that expected 3 fields for the
> 	atom constructor for the trace_atom type.

I thought we agreed to make atom reference the layout structure instead?
Why did you go back to this arrangement?

> trace/mercury_trace_declarative.h
> 	Added MR_decl_add_trusted_module function to add a module to the set 
> 	of
> 	trusted modules for the currect diagnoser (which is stored in a
> 	global).

mercury_trace_declarative.h doesn't have any globals *added* to it by
this diff. What global are you referring to?

> tests/debugger/declarative/trust.inp
> 	Imput to mdb to test the `trust' command.  Contains commands to tell
> 	mdb to trust the trust_1 and trust_2 modules.

Imput -> Input

You haven't done a "cvs add" for the new files you added, so they didn't
show up in the diff.

> Index: browser/declarative_debugger.m

> +
> +	% Adds a trusted module to the given diagnoser.
> +	%
> +:- pred add_trusted_module(string, diagnoser_state(trace_node_id),
> +		diagnoser_state(trace_node_id)).
> +:- mode add_trusted_module(in, in, out) is det.

You should use combined predmode declarations where possible.

> Index: browser/declarative_oracle.m
> +:- import_module mdb__declarative_execution.
>  :- import_module mdb__declarative_user.
>  :- import_module mdb__tree234_cc.
>  :- import_module mdb__set_cc.
>  :- import_module mdb__util.
> +:- import_module set.
> 
>  :- import_module bool, std_util, set.

Set was already being imported.

> +query_oracle_list(OS, [Q | Qs0], As) :-
> +	query_oracle_list(OS, Qs0, As0),
> +	Atom = get_decl_question_atom(Q),
> +	(	
> +		% is the atom in a trusted module?
> +		member(Atom ^ module_name, OS ^ trusted_modules)
> +	->
> +		As = [truth_value(get_decl_question_node(Q), yes) | As0]

Replace the second get_decl_question_node(Q) with Atom.

> tests/debugger/declarative/trust.m
> ===================================================================
> :- module trust.
> 
> :- interface.
> 
> :- import_module trust_1, io.
> 
> :- pred main(io::di,io::uo) is cc_multi.
> 
> :- pred blah(w::out) is det.
> 
> :- implementation.
> 
> :- import_module trust_2.
> 
> blah(w("lala")).
> 
> :- pred dostuff(w::out, comparison_result::uo) is cc_multi.	
> 	
> dostuff(W, R) :-
> 	compare(R, w("aaB"), w("aAB")),
> 	concat(w("aaa"),w("bbb"),W).
> 	
> main(!IO) :-
> 	dostuff(w(S), R),
> 	write_string(S, !IO),
> 	nl(!IO),
> 	write(R, !IO),
> 	nl(!IO).

Since blah is local, move the declaration to the implementation section.

Move the definition of main/2 to be first.

> tests/debugger/declarative/trust.inp
> ===================================================================
> echo on
> trust trust_1
> trust trust_2
> step
> finish
> dd
> n
> y
> continue

You probably want to tell the debugger to switch off the display
of contexts; we already enough test cases to test their display,
and it clutters up your expected output file.

> tests/debugger/declarative/trust.exp
> ===================================================================
>        1:      1  1 CALL pred trust.main/2-0 (cc_multi) ./trust.m:23

Though I wonder how you got a "./" prefix on the context here.

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