[m-rev.] For review: allow declarative debugger to search upward from initial symptom

[Julien Fischer]

On Mon, 29 Nov 2004, Ian MacLarty wrote:

> For review by anyone.
>

I can't really review the code as I'm not that familiar with the inner
workings of the debugger, so these comments are restricted to the changes
to the user guide.

> Estimated hours taken: 50
> Branches: main
>
> Allow declarative debugger to search nodes above the node where the initial
I suggest: "Allow the declartive debugger"

> `dd' command was given.  If the user asserts that the node at which the `dd'
> command was given is correct or inadmissible then the declaratibe debugger will
s/declartibe/declarative/

> ask questions about ancestors of the node at which the `dd' command was given.
> The declarative debugger will only say it cannot find a bug if the user asserts
> that the main/2 (or whatever the topmost traced call is) call is correct or
> inadmissible.
>
> This is useful when you've found an inadmissible node in the procedural
> debugger, but you're not sure where the erroneous ancestor is.
>
> Fix bug in sub-term dependency tracking when tracking an input sub-term: If the
> sub-term was bound by a primitive operation then the next question was about
> the child of the node in which the sub-term was bound, instead of the node
> itself.
>
> Add --depth-step-size option to mdb `dd' command.  This allows the user to
> specify the depth of each materialized portion of the EDT.
>
> browser/declarative_analyser.m
> 	Allow analyser to request an explicit supertree from the diagnoser
> 	and respond correctly once an explicit subtree has been generated.
>
> 	When the primitive operation that binds a sub-term is found, the
> 	suspect_id of the node containing the primitive op is now returned, so
> 	handle this by asking the next question about the node containing the
> 	primitive op if its status is unknown.
>
> 	Stop tracking the sub-term if it is an input and we encounter an
> 	erroneous node.
>
>	Remove previous_roots field from analyser_state.  It is not
> 	needed because this information is now kept in the search space.
>
> browser/declarative_debugger.m
> 	Add new diagnoser response to tell backend to generate an explicit
> 	supertree.
>
> browser/declarative_edt.m
> 	Add methods to mercury_edt typeclass to get the parent of an EDT node,
> 	tell if two nodes refer to the same event and tell if a node is the
> 	topmost node (usually the 1st call to main/2).
>
> 	Make find_subterm_origin return the suspect in which a primitive
> 	operation was executed.
>
> 	Add predicate to incorporate a new explicit supertree into the search
> 	space.
>
> 	Add predicate to tell the analyser when it's okay to stop tracking
> 	a sub-term.
>
> 	Fix bug in find_subterm_origin so it doesn't report a child as the
> 	binding node when it should be the parent.  Also replace duplicated
> 	code in find_subterm_origin with new predicate resolve_origin.
>
> 	Add extend_search_space_upwards predicate which attempts to add an
> 	extra node to the top of the search space.
>
> 	If a status is changed from erroneous to correct or vica versa then
> 	mark the suspects which were eliminated from the search space by
> 	the original status as unknown.
>
> browser/declarative_execution.m
> 	Rename call_last_exit_redo to call_last_interface, since excp and fail
> 	nodes can also go here.
>
> browser/declarative_tree.m
> 	Add implementations for new methods from mercury_edt typeclass.
>
> doc/user_guide.texi
> 	Document --depth-step-size dd option.
>
> 	Remove duplicate save command documentation.
>
> 	Add a comment about new functionality.
>
> tests/debugger/declarative/Mmakefile
> tests/debugger/declarative/mapinit.exp
> tests/debugger/declarative/mapinit.inp
> 	Use standardized event printing for mapinit test.
>
> tests/debugger/declarative/app.exp
> tests/debugger/declarative/app.inp
> tests/debugger/declarative/revise_2.exp
> tests/debugger/declarative/revise_2.inp
> 	Changed expected output and input because the bug search now continues
> 	in the ancestors of the node the original `dd' command was given in.
>
> tests/debugger/declarative/catch.exp
> 	XXX still trying to work out why this has changed.  Now getting a
> 	"reached unknown label" warning, though otherwise the output is the
> 	same.
>
> tests/debugger/declarative/explicit_subtree.exp
> tests/debugger/declarative/explicit_subtree.exp2
> tests/debugger/declarative/explicit_subtree.inp
> tests/debugger/declarative/explicit_subtree.inp2
> tests/debugger/declarative/explicit_subtree.m
> 	Modify this test to also test generation of an explicit supertree.
>
> trace/mercury_trace_declarative.c
> 	If requested to generate a supertree then retry to a node
> 	above the current top most node and collect events down to the
> 	current top most node.
>
> 	Interactively retry accross IO when building the annotated trace.
s/accross/across/

> 	This is more user friendly than simply aborting if untabled IO is
> 	encountered.
>
> trace/mercury_trace_declarative.h
> 	Export MR_edt_depth_step_size so it can be set with the
> 	--depth-step-size dd option.
>
> trace/mercury_trace_internal.c
> 	Add --depth-step-size option for `dd' command.
>
...

> Index: doc/user_guide.texi
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/doc/user_guide.texi,v
> retrieving revision 1.398
> diff -u -r1.398 user_guide.texi
> --- doc/user_guide.texi	19 Nov 2004 11:54:21 -0000	1.398
> +++ doc/user_guide.texi	29 Nov 2004 09:30:27 -0000
> @@ -3215,8 +3215,17 @@
>  @c makes an assertion, and if the assertion is incorrect, the resulting
>  @c behaviour would be hard for non-developers to understand. The option is
>  @c therefore deliberately not documented.
> -Starts declarative debugging
> -using the current event as the initial symptom.
> +Starts declarative debugging using the current event as the initial symptom.
> + at sp 1
> +The declarative debugger searches for bugs in a debug tree.  Only a portion of

Dropping in the term "debug tree" without any further explanation isn't so
good - the rest of the documentation for the declarative debugger
doesn't use the term.

> +the debug tree is materialized at any time to save memory.  When a new portion
> +of the tree needs to be materialized the program being debugged is re-executed.

I think the concept of the tree being "materialized" needs further
explanation.  Perhaps it would be better to say that the dd can only
consider parts of the program at once and that it is the size of these
parsts that the`--depth-step-size' option controls?

> +How much of the debug tree is materialized at any one time can be controlled
> +with the @samp{-d at var{depth}} or @samp{--depth-step-size @var{depth}} option.
> + at var{depth} is the maximum depth of any newly materialized portion of the debug
> +tree.  A higher value of @var{depth} will mean more memory is used, but the

I suggest rewording that as "A higher value ... will mean that more ..."

> +program will have to be re-executed less often.  Set a higher @var{depth} for
> +long-running programs to improve the performance of the declarative debugger.

I don't think that this last sentence is necessary - the performance
ramifications should be obvious from the sentence before.

>  @sp 1
>  @item trust @var{module-name}|@var{proc-spec}
>  @kindex trust (mdb command)
> @@ -3289,16 +3298,13 @@
>  @sp 1
>  @item save @var{filename}
>  @kindex save (mdb command)
> -Saves current set of breakpoints, the current set of aliases and the
> +Saves the current set of breakpoints, the current set of aliases and the
>  current set of objects trusted by the declarative debugger
>  in the named file as a set of @samp{break}, @samp{alias} and @samp{trust}
>  commands.
>  Sourcing the file will recreate the current breakpoints and aliases and will
>  trust the currently trusted objects.
> - at sp 1
> -Saves current set of breakpoints and the current set of aliases
> -in the named file as a set of @samp{break} and @samp{alias} commands.
> -Sourcing the file will recreate the current breakpoints and aliases.
> +objects.

Delete the above line - it erroneously repeats the end of the previous
sentence.

>  @sp 1
>  @item quit [-y]
>  @kindex quit (mdb command)
> @@ -3900,6 +3906,15 @@
>  If, say, the date was only wrong in the year part, then we could also have
>  marked the year subterm in which case the next question would have been about
>  the call that constructed the year part of the date.
> +
> +This feature is also useful when using the procedural debugger.  Suppose you
> +come accross a CALL event and you'd like to know where a particular input
s/accross/across/
s/you'd/you would/

I suggest rewording that sentence as:

For example, suppose that you come across a CALL event and you would like to know
the source of a particular input to the call.

> +to the call came from.  To find out you could first go to the final event by

I'd change "could" to "would" there unless there is another way of doing
it.

> +issuing a @samp{finish} command.  Then invoke the declarative debugger with
> +a @samp{dd} command.  Then mark the input term you're interested in.

How about:

Invoke the declarative debugger with a @samp{dd} command and then mark
the input term you are interested in.

> The
> +next question should be about the call that bound the term at which point you
s/should/will/?
> +could issue a @samp{pd} command to return to the procedural debugger at the
> +final event of the call that bound the term.
>
It seems like two sentences have run into each other there.  I suggest
splitting that sentence up.

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