bug#22294: Patch

[Gemini Lasswell]

Dmitry Gutov <dgutov@yandex.ru> writes:

>> Some limitations:
>>
>> - All the methods get debug instrumentation and temporary breakpoints,
>> not just the one that's about to be executed. But given the potential
>> complexity of method dispatch, it seems that fixing that would require
>> some deep intertwining of Edebug and cl-generic.
>
> This sounds totally fine to me, at this stage. I _think_ it shouldn't
> be too hard to change this, given that all the arguments are known by
> the time edebug-step-in, but it's not a major issue.

Actually the arguments are not known because they have not yet been
evaluated when edebug-step-in is invoked. So it would have to set a
breakpoint after argument evaluation, run the code under debugging
until it gets to that breakpoint, look at the evaluated arguments,
figure out what method to instrument, instrument the method and set a
breakpoint in it, and then run again.

> This is a bit wasteful. But more importantly, it causes us to collect
> the list of anonymous symbols in a dynamic variable, instead of a more
> explicit data flow. Which is not great.

Agreed. Edebug already has far too many dynamic variables obscuring
its logic, making it appear the safest change is to simply add another
one rather than change the logic to allow more explicit data flow. I
don't think there is an easy answer. I have started writing tests for
Edebug, as a step towards making it possible to improve it without
worrying about breaking things that are working. Probably it will help
me understand the code in there better too.

> A couple notes on the patch itself:
> It would be better to use `dolist' here, or even `pcase-dolist', see
> an example in pcase-dolist.

Here's a revised patch. Thanks for letting me know about pcase-dolist
as it looks very useful. It's not documented and I didn't know it
existed.

0001-Make-edebug-step-in-work-on-generic-methods-Bug-2229.patch
Description: Text document