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

[Ian MacLarty]

On Mon, Mar 14, 2005 at 08:58:53AM +1100, Julien Fischer wrote:
> 
> On Sun, 13 Mar 2005, Ian MacLarty wrote:
> 
> > On Sun, Mar 13, 2005 at 04:20:31PM +1100, Julien Fischer wrote:
> > >
> > > 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.
> > >
> >
> > Okay:
> > 	The user can then see in which node the sub-term was bound, which may
> > 	help them in answering the previously skipped question.
> >
> Was there any ever conclusive decision on whether it should be sub-term or
> subterm?
> 

It's supposed to be "subterm".  At least that's what we used in the paper
Zoltan, Mark and I just submitted to AADEBUG.

> 
> The diff looks fine though.
> 

Not quite.  I discovered a bug after posting for review.  I was attempting to always
show the path of the subterm in the binding node, however the subterm will only appear
in the binding node if it is an output of the binding node (if it's an input, then
it's bound elsewhere).

I also renamed the variable PrimitiveOpId to BindingSuspectId, since that describes
the variable's value better.

Here is the relevant part of the interdiff:

diff -u browser/declarative_analyser.m browser/declarative_analyser.m
--- browser/declarative_analyser.m	12 Mar 2005 11:54:19 -0000
+++ browser/declarative_analyser.m	13 Mar 2005 09:11:01 -0000
@@ -710,33 +712,48 @@
 	find_subterm_origin(Store, SuspectId, ArgPos, TermPath, !SearchSpace,
 		FindOriginResponse),
 	(
-		FindOriginResponse = primitive_op(PrimitiveOpId, FileName, 
-			LineNo, PrimOpType),
+		FindOriginResponse = primitive_op(BindingSuspectId, FileName, 
+			LineNo, PrimOpType, Output),
 		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),
+			BindingSuspectId),
+		( 
+			Output = yes,
+			% BindingSuspectId = SuspectId since the
+			% subterm is an output of SuspectId.
+			BindingNode = get_edt_node(!.SearchSpace, 
+				SuspectId),
+			ArgNum = edt_arg_pos_to_user_arg_num(Store, 
+				BindingNode, ArgPos),
+			MaybePath = yes([ArgNum | TermPath])
+		;
+			Output = no,
+			% Since the subterm is not an output of the 
+			% binding node, it will not appear in any of the
+			% arguments of the binding node (it can't be an 
+			% input, because then it would have been bound outside
+			% the node).
+			MaybePath = no
+		),
 		(
 			% We ask about the binding node even if it was 
 			% previously skipped, since this behaviour is
-			% more preditable from the user's perspective.
+			% more predictable from the user's perspective.
 			%
-			( suspect_unknown(!.SearchSpace, PrimitiveOpId)
-			; suspect_skipped(!.SearchSpace, PrimitiveOpId)
+			( suspect_unknown(!.SearchSpace, BindingSuspectId)
+			; suspect_skipped(!.SearchSpace, BindingSuspectId)
 			)
 		->
-			SearchResponse = question(PrimitiveOpId, 
+			SearchResponse = question(BindingSuspectId, 
 				binding_node(PrimOpType, FileName, LineNo,
-					[ArgNum | TermPath], ProcLabel, no)),
-			setup_binary_search(!.SearchSpace, PrimitiveOpId,
+					MaybePath, ProcLabel, no)),
+			setup_binary_search(!.SearchSpace, BindingSuspectId,
 				NewMode)
 		;
 			(
 				LastUnknown = yes(Unknown),
 				Reason = binding_node(PrimOpType, 
-					FileName, LineNo, [ArgNum | TermPath], 
-					ProcLabel, yes),
+					FileName, LineNo, MaybePath, ProcLabel,
+					yes),
 				SearchResponse = question(Unknown, Reason),
 				setup_binary_search(!.SearchSpace, 
 					Unknown, NewMode)
@@ -1133,31 +1148,40 @@
 	ArityStr = int_to_string(Arity),
 	(
 		Eliminated = yes,
-		EliminatedStr = " That node was, however, previously "
+		EliminatedSent = " That node was, however, previously "
 			++ "eliminated from the bug search."
 	;
 		Eliminated = no,
-		EliminatedStr = ""
+		EliminatedSent = ""
+	),
+	(
+		MaybePath = yes(Path),
+		PathStrings = list.map(int_to_string, Path),
+		PathStr = string.join_list("/", PathStrings),
+		PathSent = "The path to the subterm in the atom is " ++
+			PathStr ++ "."
+	;
+		MaybePath = no,
+		PathSent = ""
 	),
 	Str = "the marked subterm was bound by the " ++ 
 		PrimOpStr ++ " inside the " ++ PredOrFuncStr ++
 		" " ++ Module ++ "." ++ Name ++ "/" ++ ArityStr ++
 		" (" ++ FileName ++ ":" ++ LineNoStr ++ "). " ++
-		"The path to the sub-term in the atom is " ++ PathStr ++ "."
-		++ EliminatedStr.
+		PathSent ++ EliminatedSent.
 		
diff -u browser/declarative_edt.m browser/declarative_edt.m
--- browser/declarative_edt.m	12 Mar 2005 11:32:53 -0000
+++ browser/declarative_edt.m	13 Mar 2005 07:48:19 -0000
@@ -337,7 +337,16 @@
 			% the suspect.  The other arguments are the filename,
 			% line number and type of the primitive operation 
 			% that bound the subterm.
-	;	primitive_op(suspect_id, string, int, primitive_op_type)
+	;	primitive_op(
+			suspect_id,		% The node in which the subterm 
+						% was bound.
+			string, 		% File name of primitive op.
+			int, 			% Line number of primitive op.
+			primitive_op_type,	% Type of primitive operation.
+			bool			% Whether the subterm appears
+						% as an output of the
+						% binding node.
+			)
 			
 			% The suspect is the root of an implicit subtree and
 			% the origin lies in one of it's children.
@@ -917,7 +926,7 @@
 		(
 			Suspect ^ parent = yes(ParentId),
 			resolve_origin(Store, Node, ArgPos, TermPath,
-				ParentId, !SearchSpace, Response)
+				ParentId, no, !SearchSpace, Response)
 		;
 			Suspect ^ parent = no,
 			(
@@ -926,7 +935,7 @@
 			->
 				topmost_det(!.SearchSpace, NewRootId),
 				resolve_origin(Store, Node, ArgPos,
-					TermPath, NewRootId, !SearchSpace,
+					TermPath, NewRootId, no, !SearchSpace,
 					Response)
 			;
 				Response = require_explicit_supertree
@@ -935,30 +944,31 @@
 	;
 		Mode = subterm_out,
 		resolve_origin(Store, Node, ArgPos,
-			TermPath, SuspectId, !SearchSpace,
+			TermPath, SuspectId, yes, !SearchSpace,
 			Response)
 	).
 
-	% resolve_origin(Store, Node, ArgPos, TermPath, SuspectId,
+	% resolve_origin(Store, Node, ArgPos, TermPath, SuspectId, Output,
 	% 	!SearchSpace, Response).
 	% Find the origin of the subterm in Node and report the origin as
 	% SuspectId if the origin is a primitive op or an input and as the
 	% appropriate child of SuspectId if the origin is an output.  SuspectId
 	% should point to a parent of Node if the mode of the sub-term is
 	% input and should point to Node itself if the mode of the sub-term is
-	% output.
+	% output.  Output should be yes if the subterm is an output of
+	% SuspectId and no if it isn't.
 	%
 :- pred resolve_origin(S::in, T::in, arg_pos::in, term_path::in, 
-	suspect_id::in, search_space(T)::in, search_space(T)::out, 
+	suspect_id::in, bool::in, search_space(T)::in, search_space(T)::out, 
 	find_origin_response::out) is det <= mercury_edt(S, T).
 	
-resolve_origin(Store, Node, ArgPos, TermPath, SuspectId, !SearchSpace, 
+resolve_origin(Store, Node, ArgPos, TermPath, SuspectId, Output, !SearchSpace, 
 		Response) :-
 	edt_dependency(Store, Node, ArgPos, TermPath, _, Origin),
 	(
 		Origin = primitive_op(FileName, LineNo, PrimOpType),
 		Response = primitive_op(SuspectId, FileName, LineNo, 
-			PrimOpType)
+			PrimOpType, Output)
 	;
 		Origin = not_found,
 		Response = not_found

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