I started about "sub watches" before reading your "every watch has a
reference" ....
On 14/09/2021 11:00, Joost van der Sluis via lazarus wrote:
Op 14-09-2021 om 02:18 schreef Martin Frb via lazarus:
But ideally this should just replace/extend the existing watches. Of
course that also would need the debugger intf to be updated. (That
does need an overhaul, and yes it could mean all existing debuggers
will need a bit of work to follow / ideas welcome)
What I did now, is add new dialogs, and created an debug-interface
separate from the existing one. But with the idea that it could
replace/be merged with the existing one.
What we could also do is make the debug-windows (watches, locals,
evaluation pluggable, so that a debugger can choose which one to use.)
Actually all it would need is for the watch to have a new method, and to
have a flag if it can be expanded. (a debugger that can not do that,
will not set the flag).
However, it is a bit more....
1)
All watches should be part of the "watches" list. (or alternative means
must be found). This is so the debugger can update them, if the user
changes memory (set a new value to a variable).
This is also helpful for the history window, and for "idle eval" (if the
watches window is closed but the debugger is idle, then eval starts anyway).
The problem of not being in the "watches" exists for "debug inspector".
It does not always update correctly. Adding to the "watches" would
however mean that each watch needs some sort of owner, so the
watch-window does not display watches added by others. So that ends up
more rework.... (and maybe there is a better way)
Of course instead of adding new watch to the "watches", those watches
can be hold as a sublist by each watch.
Note that watches for structures already have a list, with a description
of each member field. (if the watch request had the correct flag set).
But those list should probably be reworked...
2)
Currently formatting (hex vs dec) is done in the debugger. That is also
nonsense. As much as can be should be moved to the frontend.
3)
Btw, if arrays can be expanded, precautions are needed. I don't think we
want to expand 100000 child values.
----
If we go for the flag and sub-watches, then "debug history" needs to be
aware of it.
Ideally the entire debugger frontend could move into a package of its
own too (all the current windows).
Exactly, that is why I created a new debug-intf package instead of
adapting the existing one.
Well we may need to create an interface for the debugger frontend, and
then add the frontend package. Though DebugManager already has parts of
that.
On the other hand its not strictly mandatory yet. As I see it your new
watches can simply replace the old one. (as soon as watches provide that
flag, so old watches can be displayed too)
Currently the debugger intf allows to fetch watches as plain text, or
as structure. But this has to be passed in as flag at the request time.
Any watch object should instead have methods to fetch "sub watches"
so structures can unfold.
What I have now is that each object has a 'reference', and there is a
separate call to get the sub-watches that belong to the reference.
That way the data (in the object) and the logic (function to retrieve
sub-watches) are separated.
Ok, I have to look at that.
Another important factor on the design of every debugger-object (eg
watches) is "notify on free" or TRefCountedObject.
Any debugger backend holds on to the "watch object" while it evaluates
the data needed. However the frontend may remove (and free/dereference)
that watch during that time. The backend must know that. (and
potentially even call the callback, if the object no longer exists.... /
the editor hint code requires this )
I don't have a solution for requesting the result as text(string)
only. But that's not a bad idea.
Currently we have (as param to eval)
TDBGEvaluateFlag =
(defNoTypeInfo, // No Typeinfo object will be returned
defSimpleTypeInfo, // Returns: Kind (skSimple, skClass, ..);
TypeName (but does make no attempt to avoid an alias)
defFullTypeInfo, // Get all typeinfo, resolve all anchestors
defClassAutoCast, // Find real class of instance, and use,
instead of declared class of variable
defAllowFunctionCall
);
TDBGEvaluateFlags = set of TDBGEvaluateFlag;
Currently
TWatchValue = class(TFreeNotifyingObject)
property TypeInfo: TDBGType;
and
TDBGType = class(TObject)
property Fields: TDBGFields;
which is a list of TDBGField
So if (full)typeinfo is request, you have a list of fields, and you can
then get the sub-watches.
But that is not a good way....
That means you then have lots of dup data => each subwatch, has some
data also in a field.
** The other downside is, that the gdb debugger needs more time to fill
the fields. So not every debugger should provide them by default.....
** The upside is, that having type info (and fields) the frontend can do
all the formatting (almost... / mem dump still needs to get data as byte
stream)
If the backend delivers a point (x,y) as 2 fields, the frontend can
decide to print "(x: 1; y:2)" or "(1,2)". The frontend can also print
the individual values in dec,hex,bin,oct....
So always having type info would be nice.
But for gdb only remote debug targets, in needs to be seen if it can be
done, without slow down.
It should be possible with one limitations.
Currently each Field has type (tkInteger, tkClass, tkRecord, ....) and
ClassName (indicating which parent class introduced that field).
Getting those fields is time consuming.
===================================
So as a first draft I would aim for something like this:
TDBGField needs to be a TWatch. => avoid data duplication / and provide
for recursive eval.
By default the (if no flags are set) the debugger should provide fields.
But only field name, and a value (int or string).
All other info can be retrieved later, on request.
In fact, even the field info can be retrieved on request (editor hint
may not need it, but if formatting code goes to the frontend, then
editor hint also needs to do the formatting).
In the same way, that a watch has a "validity" field (dsEvaluating,
dsValid, dsError, ...) it can have validity for typeinfo and fields.
On top of that validity for typeinfo/fields can be dsNotAvail. In which
case there just is a default text value for the watch.
I see you pass the variable to the debugger, that is ok. But that may
well get abstracted into the Watch, so the watch has its own ref to
the debugger, and makes the needed calls.
I don't follow exactly. But if there's a need for an extra abstraction
layer before sending the variable to the debugger directly, this could
be added.
See above, if the subwatches are already in place of the current fields,
then they have a ds...validity. And if accessed they will further evaluate.
Not sure about passing callbacks directly. Thinking of the current
notification system (data monitors).
What does your new watch window do, if the user modifies a variable?
This triggers re-eval of all watches (so they need to unfold again).
Currently the debugger backend can trigger the re-eval, and through
the data-monitors tell everyone about the updates.
Have to look at it in more detail....
I have something similar as the data-monitors.
Main difference is that the events are based on what the gui needs,
instead of what the debugger does.
So: there is an event to tell the 'gui' that it is inpossible to show
any data. (for example: the application has stopped or there is no
debugger at all) There is an event for the case that debug-data has
become available or unavailable. (Application paused or continued)
And I should add an event for the case that the debug-info should be
re-evaluated. (context switch, or variable-change). And I can think of
another event that can be send when the debug-data could have been
changed, due to unforeseen effect, for example when a propperty with a
getter has been evaluated.
All of those are available already
Most of those are DebuggerState changes. They are currently propagated
hardcoded to each of the existing listeners. So they may need an API to
subscribe to.
But they also need to be done in a prioritized order.
Certain data needs to be done in specific order (partly that is enforced
in the backend anyway)
Context changes are subscribe-able.
CallStackNotification.OnChange := @CallStackChanged;
CallStackNotification.OnCurrent := @CallStackCurrent;
BreakpointsNotification.OnAdd := @BreakPointChanged;
BreakpointsNotification.OnUpdate := @BreakPointChanged;
BreakpointsNotification.OnRemove := @BreakPointChanged;
Note that the debugger is not the only thing that can change context. If
the user selects a history entry, then all watches change.
The advantage of 'events' that are more geared towards the gui, is
that it is easier to add catchy things. For example: to avoid
flickering, when the gui receives an event that debug-data is not
available anymore, it starts a timer of 200 mseconds, and only after
that timer the data is removed from the screen. This way, when
stepping through the code, there is no flickering in the small periods
of time that the application runs.
That can be done with the above Notification too.
It also made it easy to highlight variables that changed.
Ah, but does it?
Say I have an object with a delegate (object / class based => that is
pointer to heap).
TBar = class
i: integer;
end;
TFoo = class
bar: TBar;
end;
foo.bar := otherBar.
If otherBar has the same value in "i", and if the address of "bar" in
tfoo is not displayed. => then the displayed result of the "foo" watch
does not change. The value however has changed.
But atm they are only suitable for displaying variables. Stuff like
the call-stack are not touched.
In generally there may be reasons to extend on the current
monitor/notification.
However, I would avoid passing a callback to a function for that.
Rather adding a notification list [1] to each watch.
So if a single watch change (updates any of its validities), we no
longer need to call the global watch changed.
Downside, if all watches change (to invalid) we don't want to call
hundreds of callbacks.
So there are some design choices to be made.
There needs to be a good design to split into notifications for
individual watches, and for groups of watches (usually the whole list)
There could in future be more than one monitor.
One idea is to allow to open more than one watches-window. So you can
display an history entry next to a live value. (or 2 values from
different stack frames).
But then (even if not sensible) a user could have both windows showing
the same data too (the list of watches is in TWatches, it is not bound
to the window).
--
_______________________________________________
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus
