[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#22294: Patch
From: |
Dmitry Gutov |
Subject: |
bug#22294: Patch |
Date: |
Sun, 7 May 2017 05:34:23 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Thunderbird/53.0 |
Hi Gemini,
Thank you very much for tackling this. I've tried the patch out, and it
seems to work well.
We can install it if nobody else has any strong objections.
On 27.04.2017 2:12, Gemini Lasswell wrote:
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.
It might change the return value of edebug-instrument-function back
again, though.
- If you use edebug-step-in twice on the same generic function it will
reinstrument the methods, as opposed to using edebug-step-in twice on a
regular function where Edebug can figure out that the function is
already instrumented.
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.
Fixing that would require some way to include
dynamic elements in the :name construct of an Edebug spec so that each
method could get a unique deterministic symbol as opposed to an
anonymous generated symbol.
Ideally, we'd do this, I think. If :name spec is allowed to be a
function, it could construct the unique symbol like (intern (format
"%s-%s" name arguments)). Then edebug-instrument-function could also
call this logic itself, instead of relying on the symbol being recorded
in edebug--step-in-symbols.
(On the other hand, the proposed approach probably fixes stepping into
any edebug-able form, not just generic methods).
Or it could be fixed by the "future"
described in edebug-form-data's docstring.
We'd still need to construct the unique symbol this way, at least
somewhere, I think.
A couple notes on the patch itself:
- ;; Func should be a function symbol.
- ;; Return the function symbol, or nil if not instrumented.
- (let ((func-marker (get func 'edebug)))
+ "Instrument the function or generic method FUNC.
+Return the list of function symbols which were instrumented.
+This may be simply (FUNC) for a normal function, or a list of
+generated symbols for methods. If a function or method to
+instrument cannot be found, signal an error."
+ (let ((func-marker (get func 'edebug))
The signature change looked worrying, but all the callers seem fine
(there are not many of them).
+ ((get func 'cl--generic)
+ (let ((method-defs (method-files func)))
+ (unless method-defs
+ (error "Could not find any me
thod definitions for %s" func))
+ (while method-defs
+ (let* ((file (caar method-defs))
+ (spec (cdar method-defs))
It would be better to use `dolist' here, or even `pcase-dolist', see an
example in pcase-dolist.
+
+(require 'cl-generic)
This adds a second empty line in a row.
- bug#22294: Patch,
Dmitry Gutov <=
- bug#22294: Patch, Gemini Lasswell, 2017/05/07
- bug#22294: Patch, Dmitry Gutov, 2017/05/07
- bug#22294: Generating Edebug names for generic methods (was: bug#22294: Patch), Gemini Lasswell, 2017/05/10
- bug#22294: Generating Edebug names for generic methods, Dmitry Gutov, 2017/05/10
- bug#22294: Generating Edebug names for generic methods, Gemini Lasswell, 2017/05/13
- bug#22294: Generating Edebug names for generic methods, Dmitry Gutov, 2017/05/13
- bug#22294: Generating Edebug names for generic methods, Dmitry Gutov, 2017/05/14