[m-rev.] for review: display the reason why a question was asked in the dd

Julien Fischer juliensf at cs.mu.OZ.AU
Sun Mar 13 16:20:31 AEDT 2005


On Sat, 12 Mar 2005, Ian MacLarty wrote:

> For review by anyone.
>
> Estimated hours taken: 14
> Branches: main
>
> Include the reason why a question was asked in the information provided by the
> `info' command.  This includes the place where a marked subterm was bound if
> the user marked a subterm in the previous question.
>
> browser/declarative_analyser.m
> 	Add a new type to record the reason why a question was asked.  Keep
> 	this information with the last question asked in the analyser state, in
> 	case the user issues an `info' command.  Display the reason when the
> 	user issues an `info' command.
>
> 	Change the behaviour of subterm dependency tracking slightly: if the
> 	binding node was previously skipped then ask about it anyway.
>
Briefly mention why here.

> browser/declarative_edt.m
> 	Add two new methods to the mercury_edt typeclass.  One to get the
> 	proc_label of a node and the other to convert an arg_pos to a
> 	user argument number with respect to an atom in a node.  These
> 	are needed to display the question reason to the user in
> 	mdb.declarative_analyser.
>
> 	Add a new type to record the primitive operation that bound a subterm.
>
> browser/declarative_execution.m
> 	Add a predicate to convert an arg_pos into a user arg number.
>
> browser/declarative_oracle.m
> 	Fix a faulty comment.
>
> browser/declarative_tree.m
> 	Implement the new methods added to the mercury_edt typeclass.
>
> 	Return the type of primitive operation that bound a subterm.
>
> doc/user_guide.texi
> 	Document the fact that the reason is now also displayed when the
> 	`info' command is issued.
>
> tests/debugger/declarative/info.exp
> tests/debugger/declarative/info.inp
> tests/debugger/declarative/info.m
> 	Test the info command more thoroughly.
>
> Index: browser/declarative_analyser.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/browser/declarative_analyser.m,v
> retrieving revision 1.21
> diff -u -r1.21 declarative_analyser.m
> --- browser/declarative_analyser.m	12 Mar 2005 04:46:29 -0000	1.21
> +++ browser/declarative_analyser.m	12 Mar 2005 11:54:19 -0000
> @@ -21,8 +21,10 @@
>  :- import_module mdb.declarative_debugger.
>  :- import_module mdb.io_action.
>  :- import_module mdb.declarative_edt.
> +:- import_module mdbcomp.program_representation.
> +:- import_module mdbcomp.prim_data.
>
> -:- import_module std_util, io.
> +:- import_module std_util, io, bool.
>
>  :- type analyser_response(T)
>
> @@ -49,6 +51,35 @@
>  			% this question and then for analysis to continue.
>  	;	revise(decl_question(T)).
>
> +	% The reason a question was asked.
> +	%
I suggest a little more detail there.
e.g The reason the declarative debugger asked a question.

> +:- type question_reason

Perhaps, reason_for_question as the typename.

> +	--->	start 			% The first question.
> +	;	top_down
> +	;	binding_node(
> +			binding_prim_op		:: primitive_op_type,
> +			binding_filename	:: string,
> +			binding_line_no		:: int,
> +			binding_atom_path	:: term_path,
> +			binding_proc		:: proc_label,
> +			binding_node_eliminated	:: bool
> +		)
> +	;	subterm_no_proc_rep	% No proc rep when tracking subterm.
> +	;	binding_node_eliminated
> +	;	binary(
> +			binary_reason_bottom	:: int,
> +			binary_reason_top	:: int,
> +			binary_reason_split	:: int
> +		)
> +	;	divide_and_query(
> +			old_weight		:: int,
> +			choosen_subtree_weight 	:: int
> +		)
> +	;	skipped
> +	;	revise.
> +
> +:- func question_reason_to_string(question_reason) = string.
> +
>  :- type analyser_state(T).
>
>  :- type search_mode.
> @@ -124,6 +155,7 @@
>  :- implementation.
>
>  :- import_module mdb.declarative_edt.
> +:- import_module mdb.declarative_execution.
>  :- import_module mdbcomp.program_representation.
>
>  :- import_module exception, counter, array, list, float.
> @@ -213,7 +245,7 @@
>  	% which is the root of an implicit subtree.
>  	%
>  :- type search_response
> -	--->	question(suspect_id)
> +	--->	question(suspect_id, question_reason)
>  	;	require_explicit_subtree(suspect_id)
>  	;	require_explicit_supertree
>  	;	no_suspects.

...

> @@ -671,22 +710,34 @@
>  	find_subterm_origin(Store, SuspectId, ArgPos, TermPath, !SearchSpace,
>  		FindOriginResponse),
>  	(
> -		FindOriginResponse = primitive_op(PrimitiveOpId, _, _),
> -		%
> -		% XXX In future the filename and line number of the primitive
> -		% operation could be printed out if the node in which the
> -		% primitive operation occured turned out to be a bug.
> -		%
> +		FindOriginResponse = primitive_op(PrimitiveOpId, FileName,
> +			LineNo, PrimOpType),
> +		ProcLabel = get_proc_label_for_suspect(Store, !.SearchSpace,
> +			PrimitiveOpId),
> +		BindingNode = get_edt_node(!.SearchSpace, PrimitiveOpId),
> +		ArgNum = edt_arg_pos_to_user_arg_num(Store, BindingNode,
> +			ArgPos),
>  		(
> -			suspect_unknown(!.SearchSpace, PrimitiveOpId)
> +			% We ask about the binding node even if it was
> +			% previously skipped, since this behaviour is
> +			% more preditable from the user's perspective.

s/preditable/predictable/

> +			%
> +			( suspect_unknown(!.SearchSpace, PrimitiveOpId)
> +			; suspect_skipped(!.SearchSpace, PrimitiveOpId)
> +			)
>  		->
> -			SearchResponse = question(PrimitiveOpId),
> -			setup_binary_search(!.SearchSpace,  PrimitiveOpId,
> +			SearchResponse = question(PrimitiveOpId,
> +				binding_node(PrimOpType, FileName, LineNo,
> +					[ArgNum | TermPath], ProcLabel, no)),
> +			setup_binary_search(!.SearchSpace, PrimitiveOpId,
>  				NewMode)
>  		;
>  			(
>  				LastUnknown = yes(Unknown),
> -				SearchResponse = question(Unknown),
> +				Reason = binding_node(PrimOpType,
> +					FileName, LineNo, [ArgNum | TermPath],
> +					ProcLabel, yes),
> +				SearchResponse = question(Unknown, Reason),
>  				setup_binary_search(!.SearchSpace,
>  					Unknown, NewMode)
>  			;
> @@ -694,15 +745,15 @@
>  				search(Store, !SearchSpace, FallBackSearchMode,
>  					FallBackSearchMode, NewMode,
>  					SearchResponse)
> -				)
> +			)
>  		)
>  	;
>  		FindOriginResponse = not_found,
>  		(
>  			LastUnknown = yes(Unknown),
> -			SearchResponse = question(Unknown),
> -			setup_binary_search(!.SearchSpace,
> -				Unknown, NewMode)
> +			SearchResponse = question(Unknown,
> +				subterm_no_proc_rep),
> +			setup_binary_search(!.SearchSpace, Unknown, NewMode)
>  		;
>  			LastUnknown = no,
>  			search(Store, !SearchSpace, FallBackSearchMode,
> @@ -745,7 +796,8 @@
>  		->
>  			(
>  				LastUnknown = yes(Unknown),
> -				SearchResponse = question(Unknown),
> +				SearchResponse = question(Unknown,
> +					binding_node_eliminated),
>  				setup_binary_search(!.SearchSpace,
>  					Unknown, NewMode)
>  			;


> +question_reason_to_string(start) = "this is the node where the `dd' command "
> +	++ "was given.".
Perhaps s/given/issued/?

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



More information about the reviews mailing list