bug#6415: [PATCH] fix edebug instrumentation of dotted pairs in macros

[Stefan Monnier <monnier@iro.umontreal.ca>]

> 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