[Lazarus] Playing with debuggers

[Martin Frb]

On 14/09/2021 16:11, Joost van der Sluis via lazarus wrote:
>
>> 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).
>
> Yes, as you look carefully at the screencast, you'll see that the 
> history-option is removed. ;)
I am using the history a lot. So that needs to be fixed

>
> And idle... I didn't really thought about that. My idea was that the 
> gui is the most 'responsible' place to determine when to request data. 
> And in this case this is the dialog itself. The dialogs in the 
> screencast fetch their data only when they have to be showed, just to 
> make it fast.

Well, with fpdebug eval is fast. But still, if you stop (maybe 
unexpected on an exception), it is good if the debugger already did the 
work.
And it is needed anyway for the history.

> Catching data in advance when the debugger is idle slightly contradict 
> with that approach. Although, the 'gui'/dialogs can always decide to 
> do so. But in that case a 'global cache' of retrieved variables might 
> be useful, yes.
In fact, IIRC, it is not the debugger that acts on IDLE. the debugger 
just triggers an event.
It is the history, that then starts evaluating.

>
>> 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)
>
> I'll see if I can implement that flag.
See also what to do about TDbgFields. I think they are crucial here. See 
below.

> That's easy. The gui requests a variable using a reference. Once it 
> retrieves the callback with the data that response contains the 
> reference. If the gui does not need that reference anymore, just throw 
> away the data.
>
> The request to the backend only contains the reference. The response 
> is a newly created object, that the receiving party gets ownership to. 
> And, as long as there is no global-cache, a variable has only one 
> 'owner'.
>
> All the gui-related stuff goes into the main thread.

Well yes, thats because you no longer use the global TWatches list. 
(Though subwatches would be the same)

Current eval works by creating a TWatch on the IDE side, and pass that 
object to the debugger.
And that is important, because "eval" carries more than just the watch 
expression "SomeVar".
It may request a memory dump. Or allow function calls.
Other current options (formatting hex/dec) may get dropped. But still....

Also the debugger backend should keep access. So it can invalidate a 
watch, when it continues running.
That way, invalidate can happen in a single place, and not be spread to 
each dialog that has its own set of watches.
(dialogs can still defer updating the display)



>
>>
>> On top of that validity for typeinfo/fields can be dsNotAvail. In 
>> which case there just is a default text value for the watch.
>
> Let's not over complicate things (immediately). But let's see which 
> functionality we need, and then come up with a design for that.
Yep.

But even if we implement only one backend to start with, we need to keep 
the other in mind, to come up with an universal enough design.
Also the design should allow different frontends.

IHMO, new TWatches must be created for each opened value in the tree 
(for arrays some sort of fly-weight object may be done).

Then the question is, if those watches should be (directly) added to the 
global "TWatches" list. (debug inspector may need such a feature anyway, 
so the global list may anyway end up with values not displayed on 
top-level watch window).

The other question is how to return what currently is in TDbgFields. So 
there is the least amount of data duplication.


>
>>>> 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.
>
> Ok, let's dive something more into this particular case. I used flags 
> for variables, for example to indicate that they are 'loading'. But at 
> this moment I do not have an error-flag. When the 'gui' requests a 
> variable from the backend, it can receive an error. In that cast it 
> just fills in the error-message as the value of the variable and it's 
> done.
Not sure I follow.
But that is why I advocate to create a TWatch on the gui site, and pass 
that TWatch to the debugger to be filled in.
That way you have the full "validity" property.

>
>>> 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
>
> Yes, I know. Basicly everything I made is available already. But by 
> changing a few paradigma's I hope that things can become less complex. 
> (But time will tell)

One thing is that there are more backends than frontends. And that is 
likely to stay. So defining frontend specific events puts a lot of 
burden on all backends.
IMHO the backend driven events are the way to go.

Also in the end, the frontend events can only be translations of the 
backend events.
The only point is to make sure we make all relevant events available, 
and possible allow selective access (per watch, rather than for all watches)

Yet, if all watches change together, we don't want to fire all those 
events. There should be only one, saying all changed.

I am thinking along the lines that a listener can  subscribe for events 
(such as validity changed). It can do so on a per watch base (filter).
It gets a message if any of the watches that it is subscribed to 
triggers that event.
There is an optional field SenderWatch, which will be set if only one 
changed.
If all or several changed, then SenderWatch is nil.

Or something  like that.