[m-rev.] already reviewed: add debugger state to ssdb

[oan at missioncriticalit.com]

On 5/10/2007, "Zoltan Somogyi" <zs at csse.unimelb.edu.au> wrote:

>On 05-Oct-2007, Olivier Annet <oan at missioncriticalit.com> wrote:
>> --- compiler/Mmakefile	3 Oct 2007 23:48:15 -0000	1.94
>> +++ compiler/Mmakefile	5 Oct 2007 04:10:30 -0000
>> @@ -13,7 +13,8 @@
>>  # Override the settings in ../Mmake.workspace so that in debugging grades we
>>  # do not include mer_mdbcomp.init twice in the list of files passed to mkinit.
>>  #
>> -C2INITFLAGS = --trace-init-file $(BROWSER_DIR)/$(BROWSER_LIB_NAME).init
>> +C2INITFLAGS = --trace-init-file $(BROWSER_DIR)/$(BROWSER_LIB_NAME).init \
>> +	--trace-init-file $(SSDB_DIR)/$(SSDB_LIB_NAME).init
>>
>>  -include Mmake.compiler.params
>
>This is required only if you want to debug the compiler itself with ssdb.
>Is ssdb usable on the compiler already?

No, not yet.

>
>The place you *should* modify first is the lmc script, so you can try out
>ssdb on small programs.
>
>> +:- type debugger_state
>> +    --->    state(
>> +                event_number    :: int,   % Current event number
>> +                csn             :: int,   % Call Sequence Number
>> +                call_depth      :: int,   % Depth of the function
>> +                stack           :: stack  % The shadow stack
>> +            ).
>
>While it is conceptually nice to package these together, doing so will add
>overheads (the creation of a tuple, additional code to pick the field you want,
>etc). Your code already has such overheads. Have you thought about making
>each field a separate mutable?

I will think on it

>
>>  handle_event(ProcId, Event) :-
>> -    promise_impure (
>> -    trace [io(!IO)] (
>> -        io.write(Event, !IO),
>> -        io.write_string(" ", !IO),
>> +    impure get_event_num_inc(EventNum),
>> +    impure update_depth(Event, PrintDepth),
>> +
>> +    (
>> +	Event = ssdb_call,
>> +	impure get_csn_inc(_),
>> +
>> +	semipure get_debugger_state(InitialState),
>> +	S = elem(ProcId, InitialState),
>> +	impure push(S)
>> +    ;
>> +	Event = ssdb_exit,
>> +	impure pop(S)
>> +    ;
>> +	Event = ssdb_redo,
>> +	error("ssdb_redo: not yet implemented")
>> +    ;
>> +	Event = ssdb_fail,
>> +	error("ssdb_fail: not yet implemented")
>> +    ),
>
>Please use standard indentation.
>
>> +:- impure pred invent_io(io::uo) is det.
>> +:- impure pred consume_io(io::di) is det.
>
>What are these for?

In the future release, the handle_event function will send back the Retry
value and the simple call :
trace[!IO]
(
  body
)
do not accept the use of output variable in his body


>
>I concur with Julien's suggestions.

I changed my code as suggested by Julien. It will be visible in the next
release.

>
>Zoltan.
>--------------------------------------------------------------------------
>mercury-reviews mailing list
>Post messages to:       mercury-reviews at csse.unimelb.edu.au
>Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
>Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
>--------------------------------------------------------------------------

--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------