[m-rev.] for review: improvements to subterm dependency tracking

Ian MacLarty maclarty at cs.mu.OZ.AU
Tue Nov 1 20:46:12 AEDT 2005


On Mon, 31 Oct 2005, Zoltan Somogyi wrote:

> On 30-Oct-2005, Ian MacLarty <maclarty at cs.mu.OZ.AU> wrote:
> > is not all that useful and requires much more thought from the user before they
> > track a subterm, because the user must be sure the atom is in fact erroneous or
> > inadmissible before tracking the subterm.  The user may just be suspicious of
> > the subterm and may not be willing or able to assert that the atom is
> > erroneous or inadmissible.
>
> This is fine as the default, However, there should me a mechanism that tracks
> the subterm AND asserts the incorrectness of the atom.
>
> > doc/user_guide.texi:
> > 	Change the documentation accordingly.
> >
> > browser/browse.m:
> > browser/browser_info.m:
> > browser/parse.m:
> > browser/declarative_user.m:
> > 	Change the `mark' command to `track' and allow an `--accurate' or `-a'
> > 	argument.
> > 	Pass information about tracked subterms to the declarative debugger.
>
> What's the rationale for the change in name?
>
> We could use "mark" to mean "track, and assert the atom is not valid".
>

I've done what you suggest.  My rational for changing the name was that `track'
better describes what the command does, especially when it doesn't assert
anything about the atom.

> >  	N = CN - Zero.
> >
> > +:- pred lexer_arg(list(char)::in(non_empty_list), list(token)::out) is det.
> > +
> > +lexer_arg([Head | Tail], Toks) :-
> > +	( Head = ('-') ->
> > +		string__from_char_list(Tail, ArgName)
> > +	;
> > +		string__from_char_list([Head | Tail], ArgName)
> > +	),
> > +	Toks = [arg(ArgName)].
> > +
>
> Am I correct in believing that these changes are for debugging your diff?
> You should probably keep them, but document them in the manual, though you
> may wish the documentation to be commented out, so as to be visible only
> to developers.
>

No.  These changes are for parsing the --accurate argument to the track or mark
commands.  I've already documented the --accurate argument in the user guide.

I've made all the other changes you suggested.

Here's the interdiff:

diff -u doc/user_guide.texi doc/user_guide.texi
--- doc/user_guide.texi	30 Oct 2005 02:28:59 -0000
+++ doc/user_guide.texi	1 Nov 2005 09:35:53 -0000
@@ -4278,6 +4278,10 @@
 To tell the declarative debugger to
 use the second, more accurate but slower algorithm,
 give the @samp{-a} or @samp{--accurate} option to the @samp{track} command.
+ at item mark [-a] [@var{term-path}]
+The @samp{mark} command has the same effect as the @samp{track} command
+except that it also asserts that the atom is inadmissible or erroneous,
+depending on whether the subterm is input or output respectively.
 @sp 1
 @item pd
 Commence procedural debugging from the current point.
@@ -4435,7 +4439,7 @@
 of extra information that can be given and how to convey this information are
 explained in this section.

- at subsubsection Tracking incorrect subterms
+ at subsubsection Tracking suspicious subterms

 An incorrect subterm can be tracked to the call that bound the subterm
 from within the interactive term browser
diff -u browser/browse.m browser/browse.m
--- browser/browse.m	29 Oct 2005 09:08:15 -0000
+++ browser/browse.m	1 Nov 2005 06:49:06 -0000
@@ -26,6 +26,7 @@
 :- import_module mdb.browser_term.

 :- import_module io.
+:- import_module list.
 :- import_module std_util.

     % The interactive term browser. The caller type will be `browse', and
@@ -35,7 +36,7 @@
     %
 :- pred browse_browser_term_no_modes(browser_term::in,
     io__input_stream::in, io__output_stream::in,
-    maybe_track_subterm_browser::out,
+    maybe_track_subterm(list(dir))::out,
     browser_persistent_state::in, browser_persistent_state::out,
     io::di, io::uo) is cc_multi.

@@ -44,7 +45,7 @@
     %
 :- pred browse_browser_term(browser_term::in,
     io__input_stream::in, io__output_stream::in,
-    maybe(browser_mode_func)::in, maybe_track_subterm_browser::out,
+    maybe(browser_mode_func)::in, maybe_track_subterm(list(dir))::out,
     browser_persistent_state::in, browser_persistent_state::out,
     io::di, io::uo) is cc_multi.

@@ -158,7 +159,6 @@
 :- import_module deconstruct.
 :- import_module getopt.
 :- import_module int.
-:- import_module list.
 :- import_module map.
 :- import_module parser.
 :- import_module pprint.
@@ -514,7 +514,7 @@

 :- pred browse_common(debugger::in, browser_term::in, io__input_stream::in,
     io__output_stream::in, maybe(portray_format)::in,
-    maybe(browser_mode_func)::in, maybe_track_subterm_browser::out,
+    maybe(browser_mode_func)::in, maybe_track_subterm(list(dir))::out,
     browser_persistent_state::in, browser_persistent_state::out,
     io::di, io::uo) is cc_multi.

@@ -627,40 +627,27 @@
         nl_debugger(Debugger, !IO),
         Quit = no
     ;
-        Command = track_accurate,
-        !:Info = !.Info ^ maybe_track := track_accurate(!.Info ^ dirs),
-        Quit = yes
-    ;
-        Command = track_accurate(Path),
+        Command = track(HowTrack, ShouldAssertInvalid, MaybePath),
+        (
+            MaybePath = yes(Path),
         change_dir(!.Info ^ dirs, Path, NewPwd),
         deref_subterm(!.Info ^ term, NewPwd, [], SubResult),
         (
             SubResult = deref_result(_),
-            !:Info = !.Info ^ maybe_track := track_accurate(NewPwd),
+                !:Info = !.Info ^ maybe_track := track(HowTrack,
+                    ShouldAssertInvalid, NewPwd),
             Quit = yes
         ;
             SubResult = deref_error(_, _),
-            write_string_debugger(Debugger, "error: cannot track subterm\n",
-                !IO),
+                write_string_debugger(Debugger,
+                    "error: cannot track subterm\n", !IO),
             Quit = no
         )
     ;
-        Command = track_fast,
-        !:Info = !.Info ^ maybe_track := track_fast(!.Info ^ dirs),
-        Quit = yes
-    ;
-        Command = track_fast(Path),
-        change_dir(!.Info ^ dirs, Path, NewPwd),
-        deref_subterm(!.Info ^ term, NewPwd, [], SubResult),
-        (
-            SubResult = deref_result(_),
-            !:Info = !.Info ^ maybe_track := track_fast(NewPwd),
+            MaybePath = no,
+            !:Info = !.Info ^ maybe_track :=
+                track(HowTrack, ShouldAssertInvalid, !.Info ^ dirs),
             Quit = yes
-        ;
-            SubResult = deref_error(_, _),
-            write_string_debugger(Debugger, "error: cannot track subterm\n",
-                !IO),
-            Quit = no
         )
     ;
         Command = mode_query,
diff -u browser/browser_info.m browser/browser_info.m
--- browser/browser_info.m	30 Oct 2005 02:17:09 -0000
+++ browser/browser_info.m	1 Nov 2005 06:10:17 -0000
@@ -43,9 +43,9 @@
 					% mdb command.
 			state		:: browser_persistent_state,
 					% Persistent settings.
-			maybe_track	:: maybe_track_subterm_browser,
+			maybe_track	:: maybe_track_subterm(list(dir)),
 					% Location of subterm for which the
-					% `track' command was given,
+					% `track' or `mark' command was given,
 					% or `no_track' if the `track' command
 					% was not given.
 			maybe_mode_func	:: maybe(browser_mode_func)
@@ -54,10 +54,17 @@
 					% the user issue a `mode' query.
 		).

-:- type maybe_track_subterm_browser
+:- type maybe_track_subterm(P)
 	--->	no_track
-	;	track_accurate(list(dir))
-	;	track_fast(list(dir)).
+	;	track(how_track_subterm, should_assert_invalid, P).
+
+:- type how_track_subterm
+	--->	track_accurate
+	;	track_fast.
+
+:- type should_assert_invalid
+	--->	assert_invalid
+	;	no_assert_invalid.

 	% A signature for functions that can be used by the browser to work
 	% out the mode of a sub-term.
diff -u browser/parse.m browser/parse.m
--- browser/parse.m	29 Oct 2005 09:39:32 -0000
+++ browser/parse.m	1 Nov 2005 06:40:19 -0000
@@ -35,6 +35,9 @@
 %		"write"
 %		"set" [[setoptions] varvalue]
 %		"track" [--accurate] [path]
+%		"t" [--accurate] [path]
+%		"mark" [--accurate] [path]
+%		"m" [--accurate] [path]
 %		"mode" [path]
 %		"quit"
 %
@@ -120,10 +123,7 @@
 		)
 	;	cd(path)
 	;	cd
-	;	track_accurate(path)
-	;	track_fast(path)
-	;	track_accurate
-	;	track_fast
+	;	track(how_track_subterm, should_assert_invalid, maybe(path))
 	;	mode_query(path)
 	;	mode_query
 	;	pwd
@@ -388,27 +388,44 @@
 		ArgTokens = [],
 		Command = pwd
 	;
-		CmdToken = name("track")
+		(
+			CmdToken = name("track"),
+			AssertInvalid = no_assert_invalid
+		;
+			CmdToken = name("t"),
+			AssertInvalid = no_assert_invalid
+		;
+			CmdToken = name("mark"),
+			AssertInvalid = assert_invalid
+		;
+			CmdToken = name("m"),
+			AssertInvalid = assert_invalid
+		)
 	->
 		( ArgTokens = [] ->
-			Command = track_fast
+			HowTrack = track_fast,
+			MaybePath = no
 		;
 			( ArgTokens = [arg("accurate")]
 			; ArgTokens = [arg("a")]
-			)
+		)
 		->
-			Command = track_accurate
+			HowTrack = track_accurate,
+			MaybePath = no
 		;
 			( ArgTokens = [arg("accurate") | Rest]
 			; ArgTokens = [arg("a") | Rest]
 			)
 		->
+			HowTrack = track_accurate,
 			parse_path(Rest, Path),
-			Command = track_accurate(Path)
+			MaybePath = yes(Path)
 		;
+			HowTrack = track_fast,
 			parse_path(ArgTokens, Path),
-			Command = track_fast(Path)
-		)
+			MaybePath = yes(Path)
+		),
+		Command = track(HowTrack, AssertInvalid, MaybePath)
 	;
 		CmdToken = name("mode")
 	->
diff -u browser/declarative_user.m browser/declarative_user.m
--- browser/declarative_user.m	29 Oct 2005 09:18:21 -0000
+++ browser/declarative_user.m	1 Nov 2005 06:27:01 -0000
@@ -202,18 +202,12 @@
 			query_user(UserQuestion, Response,
 				!User, !IO)
 		;
-			MaybeTrack = track_fast(TermPath),
+			MaybeTrack = track(HowTrack, ShouldAssertInvalid,
+				TermPath),
 			ArgPos = arg_num_to_arg_pos(ArgNum),
 			Node = get_decl_question_node(Question),
 			Answer = suspicious_subterm(Node, ArgPos, TermPath,
-				track_fast),
-			Response = user_answer(Question, Answer)
-		;
-			MaybeTrack = track_accurate(TermPath),
-			ArgPos = arg_num_to_arg_pos(ArgNum),
-			Node = get_decl_question_node(Question),
-			Answer = suspicious_subterm(Node, ArgPos, TermPath,
-				track_accurate),
+				HowTrack, ShouldAssertInvalid),
 			Response = user_answer(Question, Answer)
 		)
 	;
@@ -224,30 +218,21 @@
 			query_user(UserQuestion, Response,
 				!User, !IO)
 		;
-			MaybeTrack = track_fast([ArgNum | TermPath]),
+			MaybeTrack = track(HowTrack, ShouldAssertInvalid,
+				[ArgNum | TermPath]),
 			ArgPos = arg_num_to_arg_pos(ArgNum),
 			Node = get_decl_question_node(Question),
 			Answer = suspicious_subterm(Node, ArgPos, TermPath,
-				track_fast),
-			Response = user_answer(Question, Answer)
-		;
-			MaybeTrack = track_accurate([ArgNum | TermPath]),
-			ArgPos = arg_num_to_arg_pos(ArgNum),
-			Node = get_decl_question_node(Question),
-			Answer = suspicious_subterm(Node, ArgPos, TermPath,
-				track_accurate),
+				HowTrack, ShouldAssertInvalid),
 			Response = user_answer(Question, Answer)
 		;
 			%
 			% Tracking the entire atom doesn't make sense.
 			%
-			( MaybeTrack = track_fast([])
-			; MaybeTrack = track_accurate([])
-			),
+			MaybeTrack = track(_, _, []),
 			io.write_string(!.User ^ outstr,
 				"Cannot track the entire atom. " ++
-				"Please select a subterm to track.\n",
-				!IO),
+				"Please select a subterm to track.\n", !IO),
 			query_user(UserQuestion, Response, !User, !IO)
 		)
 	).
@@ -452,7 +437,7 @@
 decl_bug_io_actions(i_bug(inadmissible_call(_, _, _, _)), no).

 :- pred browse_chosen_io_action(maybe(io_action_range)::in, int::in,
-	maybe_track_subterm_user::out, user_state::in, user_state::out,
+	maybe_track_subterm(term_path)::out, user_state::in, user_state::out,
 	io::di, io::uo) is cc_multi.

 browse_chosen_io_action(MaybeIoActions, ActionNum, MaybeTrack, !User, !IO) :-
@@ -537,14 +522,14 @@
 		OK = no
 	).

-:- pred browse_io_action(io_action::in, maybe_track_subterm_user::out,
+:- pred browse_io_action(io_action::in, maybe_track_subterm(term_path)::out,
 	user_state::in, user_state::out, io::di, io::uo) is cc_multi.

 browse_io_action(IoAction, MaybeTrack, !User, !IO) :-
 	Term = io_action_to_browser_term(IoAction),
 	browse_browser_term(Term, !.User ^ instr, !.User ^ outstr, no,
-		MaybeTrackBrowser, !.User ^ browser, Browser, !IO),
-	convert_maybe_track_browser_to_maybe_track_user(MaybeTrackBrowser,
+		MaybeTrackDirs, !.User ^ browser, Browser, !IO),
+	convert_maybe_track_dirs_to_term_path(MaybeTrackDirs,
 		MaybeTrack),
 	!:User = !.User ^ browser := Browser.

@@ -575,13 +560,8 @@
 		browse_xml_atom(FinalAtom, User, !IO)
 	).

-:- type maybe_track_subterm_user
-	--->	no_track
-	;	track_accurate(term_path)
-	;	track_fast(term_path).
-
 :- pred browse_atom_argument(trace_atom::in, trace_atom::in, int::in,
-	maybe_track_subterm_user::out, user_state::in, user_state::out,
+	maybe_track_subterm(term_path)::out, user_state::in, user_state::out,
 	io::di, io::uo) is cc_multi.

 browse_atom_argument(InitAtom, FinalAtom, ArgNum, MaybeTrack, !User, !IO) :-
@@ -597,9 +577,9 @@
 			!.User ^ instr, !.User ^ outstr,
 			yes(get_subterm_mode_from_atoms_for_arg(ArgNum,
 				InitAtom, FinalAtom)),
-			MaybeTrackBrowser, !.User ^ browser, Browser, !IO),
-		convert_maybe_track_browser_to_maybe_track_user(
-			MaybeTrackBrowser, MaybeTrack),
+			MaybeTrackDirs, !.User ^ browser, Browser, !IO),
+		convert_maybe_track_dirs_to_term_path(
+			MaybeTrackDirs, MaybeTrack),
 		!:User = !.User ^ browser := Browser
 	;
 		io.write_string(!.User ^ outstr, "Invalid argument number\n",
@@ -627,7 +607,7 @@
 	).

 :- pred browse_atom(trace_atom::in, trace_atom::in,
-	maybe_track_subterm_user::out,
+	maybe_track_subterm(term_path)::out,
 	user_state::in, user_state::out, io::di, io::uo) is cc_multi.

 browse_atom(InitAtom, FinalAtom, MaybeTrack, !User, !IO) :-
@@ -641,9 +621,9 @@
 		ArgValues, IsFunction),
 	browse_browser_term(BrowserTerm, !.User ^ instr, !.User ^ outstr,
 		yes(get_subterm_mode_from_atoms(InitAtom, FinalAtom)),
-		MaybeTrackBrowser, !.User ^ browser, Browser, !IO),
-	convert_maybe_track_browser_to_maybe_track_user(
-		MaybeTrackBrowser, MaybeTrack),
+		MaybeTrackDirs, !.User ^ browser, Browser, !IO),
+	convert_maybe_track_dirs_to_term_path(
+		MaybeTrackDirs, MaybeTrack),
 	!:User = !.User ^ browser := Browser.

 :- pred browse_xml_atom(trace_atom::in, user_state::in, io::di, io::uo)
@@ -754,15 +734,13 @@
 		OK = no
 	).

-:- pred convert_maybe_track_browser_to_maybe_track_user(
-	maybe_track_subterm_browser::in, maybe_track_subterm_user::out) is det.
-
-convert_maybe_track_browser_to_maybe_track_user(no_track, no_track).
-convert_maybe_track_browser_to_maybe_track_user(track_fast(Dirs),
-		track_fast(TermPath)) :-
-	convert_dirs_to_term_path(Dirs, TermPath).
-convert_maybe_track_browser_to_maybe_track_user(track_accurate(Dirs),
-		track_accurate(TermPath)) :-
+:- pred convert_maybe_track_dirs_to_term_path(
+	maybe_track_subterm(list(dir))::in,
+	maybe_track_subterm(term_path)::out) is det.
+
+convert_maybe_track_dirs_to_term_path(no_track, no_track).
+convert_maybe_track_dirs_to_term_path(track(HowTrack, ShouldAssertInvalid,
+		Dirs), track(HowTrack, ShouldAssertInvalid, TermPath)) :-
 	convert_dirs_to_term_path(Dirs, TermPath).

 	% Reverse the first argument and append the second to it.
diff -u browser/declarative_analyser.m browser/declarative_analyser.m
--- browser/declarative_analyser.m	29 Oct 2005 10:08:41 -0000
+++ browser/declarative_analyser.m	1 Nov 2005 06:31:53 -0000
@@ -141,6 +141,7 @@

 :- implementation.

+:- import_module mdb.browser_info.
 :- import_module mdb.declarative_edt.
 :- import_module mdb.declarative_execution.
 :- import_module mdbcomp.prim_data.
@@ -499,8 +500,8 @@
 		SearchSpace),
 	!:Analyser = !.Analyser ^ search_space := SearchSpace.

-process_answer(Store, suspicious_subterm(Node, ArgPos, TermPath, HowTrack),
-	SuspectId, !Analyser) :-
+process_answer(Store, suspicious_subterm(Node, ArgPos, TermPath, HowTrack,
+		ShouldAssertInvalid), SuspectId, !Analyser) :-
 	%
 	% XXX The following 2 lines just done so that debugging info can be
 	% printed for tests run when declarative_analyser.m not compiled with
@@ -509,7 +510,22 @@
 	%
 	edt_dependency(Store, Node, ArgPos, TermPath, _, DebugOrigin),
 	!:Analyser = !.Analyser ^ debug_origin := yes(DebugOrigin),
-
+	(
+		ShouldAssertInvalid = assert_invalid,
+	edt_subterm_mode(Store, Node, ArgPos, TermPath, Mode),
+	(
+		Mode = subterm_in,
+		assert_suspect_is_inadmissible(SuspectId,
+			!.Analyser ^ search_space, SearchSpace)
+	;
+		Mode = subterm_out,
+		assert_suspect_is_erroneous(SuspectId,
+			!.Analyser ^ search_space, SearchSpace)
+	),
+		!:Analyser = !.Analyser ^ search_space := SearchSpace
+	;
+		ShouldAssertInvalid = no_assert_invalid
+	),
 	!:Analyser = !.Analyser ^ search_mode := follow_subterm_end(SuspectId,
 		ArgPos, TermPath, no, HowTrack).

diff -u browser/declarative_debugger.m browser/declarative_debugger.m
--- browser/declarative_debugger.m	29 Oct 2005 08:53:28 -0000
+++ browser/declarative_debugger.m	1 Nov 2005 06:27:43 -0000
@@ -183,7 +183,8 @@
 			% value, but is suspicious of the subterm at the
 			% given term_path and arg_pos.
 			%
-	;	suspicious_subterm(T, arg_pos, term_path, how_track_subterm)
+	;	suspicious_subterm(T, arg_pos, term_path, how_track_subterm,
+			should_assert_invalid)

 			% This node should be ignored.  It cannot contain a bug
 			% but its children may or may not contain a bug.
@@ -193,10 +194,6 @@
 			% The oracle has deferred answering this question.
 	;	skip(T).

-:- type how_track_subterm
-	--->	track_fast
-	;	track_accurate.
-
 	% Answers that are known by the oracle without having to consult the
 	% user, such as answers stored in the knowledge base or answers about
 	% trusted predicates.  mdb.declarative_oracle.answer_known/3 returns
diff -u browser/declarative_edt.m browser/declarative_edt.m
--- browser/declarative_edt.m	30 Oct 2005 02:20:59 -0000
+++ browser/declarative_edt.m	1 Nov 2005 06:11:09 -0000
@@ -58,6 +58,7 @@

 :- interface.

+:- import_module mdb.browser_info.
 :- import_module mdb.declarative_debugger.
 :- import_module mdb.declarative_oracle.
 :- import_module mdbcomp.prim_data.
@@ -352,14 +353,15 @@
 	% find_subterm_origin can use heuristics to avoid materialising
 	% subtrees unnecessarily.  If the subterm is being tracked through an
 	% output argument, and there is an input argument with the same
-	% name as the output argumnet, except for a numerical suffix, then
+	% name as the output argument, except for a numerical suffix, then
 	% find_subterm_origin will check if the subterm appears in the same
 	% position in the input argument.  If it does then it will continue
 	% tracking the subterm in the input argument, thus bypassing the
-	% subtree rooted at the call.  Since dereferencing a subterm in a
+	% subtree rooted at the call and so possibly avoiding materialising
+	% that subtree.  Since dereferencing a subterm in a
 	% large structure can be expensive, find_subterm_origin will only
 	% try to bypass calls to procedures it has not tried to bypass
-	% before.  The HowTrack argument specfies whether to use the bypassing
+	% before.  The HowTrack argument specifies whether to use the bypassing
 	% heuristics and !TriedShortcutProcs keeps track of which procedures'
 	% calls it has already tried to bypass.
 	%
diff -u browser/declarative_oracle.m browser/declarative_oracle.m
--- browser/declarative_oracle.m	29 Oct 2005 08:55:56 -0000
+++ browser/declarative_oracle.m	1 Nov 2005 06:32:25 -0000
@@ -613,7 +613,7 @@
 		oracle_kb).
 :- mode assert_oracle_kb(in, in, in, out) is det.

-assert_oracle_kb(_, suspicious_subterm(_, _, _, _), KB, KB).
+assert_oracle_kb(_, suspicious_subterm(_, _, _, _, _), KB, KB).

 assert_oracle_kb(_, ignore(_), KB, KB).


Thanks for the review,

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



More information about the reviews mailing list