[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