[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#6415: [PATCH] fix edebug instrumentation of dotted pairs in macros
From: |
Stefan Monnier |
Subject: |
bug#6415: [PATCH] fix edebug instrumentation of dotted pairs in macros |
Date: |
Mon, 26 Sep 2011 21:43:46 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) |
> It so happens that the `cl-macro-list1' edebug-spec does all three
> of these things properly.
I haven't looked into it, so I'll trust on that one.
> The second problem is in edebug. The unification algorithm has
> improper or missing handling for dotted pairs in specs. I chose
> to add the handling to `edebug-match-specs' since it seemed to be
> the cleanest place to insert it.
This edebug-dotted-spec business is really ugly, I wonder if/how we
could just get rid of this variable. Or at least document clearly what
it is supposed to mean.
> - ;; Is the form dotted?
> - ((not (listp (edebug-cursor-expressions cursor)));; allow nil
> + ;; Special handling for the tail of a dotted form.
> + ((and
> + ;; Is the cursor on the tail of a dotted form?
> + (not (listp (edebug-cursor-expressions cursor)));; allow nil
> + ;; When matching a dotted form such as (a b . c) against a
> + ;; spec list that looks like
> + ;; ([&rest ...] [&optional ...]+ . [&or arg nil])
> + ;; ,e.g., the `cl-macro-list1' edebug-spec, then the &rest spec
> + ;; will consume everything up to the dotted tail (`c' in this
> + ;; example). At that point the spec list will look like so:
> + ;; ([&optional ...]+ . [&or arg nil])
> + ;; We need to be able to consume zero or more [&optional ...]
> + ;; spec(s) without moving the cursor or signaling an error.
> + ;; The current continuation provides no state that tells us
> + ;; about the upcoming &optional specs, so we use lookahead:
> +
> + ;; Recurse normally if we're about to process an optional spec.
> + (not (eq (car specs) '&optional))
> + ;; Recurse normally if the spec is a dotted list.
> + (not (and (listp specs)
> + (not (listp (cdr (last specs)))))))
> + ;; Otherwise we need to be on the tail of a dotted spec.
> (if (not edebug-dotted-spec)
> (edebug-no-match cursor "Dotted spec required."))
> ;; Cancel dotted spec and dotted form.
Questions:
- Should it really only be &optional? it looks like any &foo might work
just as well. Also shouldn't we check the (eq (aref (car specs) 0)
'&optional) instead?
- What's the purpose of the
(not (and (listp specs) (not (listp (cdr (last specs))))))?
For one (listp specs) will always be t (we've checked (atom specs)
earlier and we've just called (car specs) on the previous line)
so the code is really (not (and t (not (listp (cdr (last specs))))))
aka (listp (cdr (last specs))) but if the car of specs is not an
&optional, then we have a mismatch anyway, no?
Stefan