abo-abo / avy

Jump to things in Emacs tree-style
1.72k stars 110 forks source link

Using avy-goto-line for `goto-line` throws an error #182

Closed kaushalmodi closed 7 years ago

kaushalmodi commented 7 years ago

How to recreate the problem?

Backtrace

Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil)
  avy-action-goto(nil)
  (if (eq r t) nil (avy-action-goto r))
  (let* ((avy-handler-old avy-handler-function) (avy-handler-function (function (lambda (char) (if (or (< char 48) (> char 57)) (funcall avy-handler-old char) (let (...) (if line ...)))))) (r (avy--line (eq arg 4)))) (if (eq r t) nil (avy-action-goto r)))
  (let ((avy-keys (or (cdr (assq (quote avy-goto-line) avy-keys-alist)) avy-keys)) (avy-style (or (cdr (assq (quote avy-goto-line) avy-styles-alist)) avy-style))) (setq avy-action nil) (fset (quote avy-resume) (function (lambda nil (interactive) (let* ((avy-handler-old avy-handler-function) (avy-handler-function (function ...)) (r (avy--line ...))) (if (eq r t) nil (avy-action-goto r)))))) (let* ((avy-handler-old avy-handler-function) (avy-handler-function (function (lambda (char) (if (or ... ...) (funcall avy-handler-old char) (let ... ...))))) (r (avy--line (eq arg 4)))) (if (eq r t) nil (avy-action-goto r))))
  (if (not (memq arg (quote (1 4)))) (progn (goto-char (point-min)) (forward-line (1- arg))) (let ((avy-keys (or (cdr (assq (quote avy-goto-line) avy-keys-alist)) avy-keys)) (avy-style (or (cdr (assq (quote avy-goto-line) avy-styles-alist)) avy-style))) (setq avy-action nil) (fset (quote avy-resume) (function (lambda nil (interactive) (let* ((avy-handler-old avy-handler-function) (avy-handler-function ...) (r ...)) (if (eq r t) nil (avy-action-goto r)))))) (let* ((avy-handler-old avy-handler-function) (avy-handler-function (function (lambda (char) (if ... ... ...)))) (r (avy--line (eq arg 4)))) (if (eq r t) nil (avy-action-goto r)))))
  avy-goto-line(1)
  funcall-interactively(avy-goto-line 1)
  call-interactively(avy-goto-line record nil)
  command-execute(avy-goto-line record)
  #[257 "\304\305!\203\f
ivy-call()
  ivy-read("M-x " ("avy-goto-line" ...) :predicate nil :require-match t :history extended-command-history :action #[257 "\304\305!\203\f ..)
  counsel-M-x()
  funcall-interactively(counsel-M-x)
  call-interactively(counsel-M-x nil nil)
  command-execute(counsel-M-x)

Emacs version: Latest build from master as of today.

abo-abo commented 7 years ago

Can't reproduce on Emacs25. The last commit for avy was 3 weeks ago. So the cause is likely a regression by a change made to Emacs master (or your config).

Can you reproduce with a stable version of Emacs and emacs -Q?

kaushalmodi commented 7 years ago

Can't reproduce on Emacs25.

Argh. I feared that. Will deep dive into debug for this issue.

kaushalmodi commented 7 years ago

This will be an interesting debug.. my config is fine.. works on emacs 25.1, doesn't work on master.

kaushalmodi commented 7 years ago

Running update..

It is this line:

(defun avy-goto-line (&optional arg)
  "Jump to a line start in current buffer.

When ARG is 1, jump to lines currently visible, with the option
to cancel to `goto-line' by entering a number.

When ARG is 4, negate the window scope determined by
`avy-all-windows'.

Otherwise, forward to `goto-line' with ARG."
  (interactive "p")
  (setq arg (or arg 1))
  (if (not (memq arg '(1 4)))
      (progn
        (goto-char (point-min))
        (forward-line (1- arg)))
    (avy-with avy-goto-line
      (let* ((avy-handler-old avy-handler-function)
             (avy-handler-function
              (lambda (char)
                (if (or (< char ?0)
                        (> char ?9))
                    (funcall avy-handler-old char)
                  (let ((line (read-from-minibuffer
                               "Goto line: " (string char))))
                    (when line
                      (avy-push-mark)
                      (save-restriction
                        (widen)
                        (goto-char (point-min))
                        (forward-line (1- (string-to-number line))))
                      (throw 'done 'exit))))))
             (r (avy--line (eq arg 4)))) ; <<<<<<<<<<<<<<<<<<<<< THIS LINE
        (unless (eq r t)
          (avy-action-goto r))))))

That r evals to t in 25.1, but nil on master.

abo-abo commented 7 years ago

Just built master for like 20 minutes, a lot longer than what it used to take. Still no bug.

GNU Emacs 26.0.50 (build 1, x86_64-unknown-linux-gnu, GTK+ Version 2.24.23) of 2017-03-02
kaushalmodi commented 7 years ago

I can recreate the bug on emacs -Q using the master version:

You can eval the below in emacs -Q and then do M-x avy-goto-line 10.

(defmacro emacs-pkg-debug-setup (pkg-alist &rest body)
  "Install packages in PKG-ALIST and evaluate BODY.
Each element of PKG-ALIST has the form (((ID . LOCATION) . (PKG1 PKG2 ..)) ..).
The ID and LOCATION are the same as the ones in `package-archives'.
PKG1, PKG2, .. are package names from the ID archive.

Example usage:

1. Launch 'emacs -Q'.
2. Copy this macro definition to its scratch buffer and evaluate it.
3. Evaluate a minimum working example using this macro as below:
     (emacs-pkg-debug-setup '(;; Install hydra from GNU Elpa
                              (nil . (hydra))
                              ;; Install org from Org Elpa
                              ((\"org\" . \"http://orgmode.org/elpa/\") . (org)))
       ;; Then evaluate the below forms
       (org-mode)) "
  (declare (indent 1) (debug t))
  `(progn
     (require 'package)
     (setq user-emacs-directory (concat temporary-file-directory
                                        (getenv "USER") "/" ".emacs.d-debug/"))
     (setq package-user-dir (format "%selpa_%s/"
                                    user-emacs-directory emacs-major-version))
     (let (archive pkgs)
       (dolist (archive-alist ,pkg-alist)
     (setq archive (car archive-alist))
     (when archive
           (add-to-list 'package-archives archive :append))
         (setq pkgs (append pkgs (cdr archive-alist))))
       (package-initialize)
       (package-refresh-contents)

       (dolist (pkg pkgs)
         (when (and pkg (not (package-installed-p pkg)))
           (package-install pkg))
         (require pkg))

       ,@body)))

(emacs-pkg-debug-setup '((nil . (avy)))
  (message "Done!"))

BTW the compiled code of emacs 26.x is incompatible with 25.x. So I keep separate elpa dirs for major versions.

kaushalmodi commented 7 years ago

Interestingly, only the interactive call causes the error; M-: (avy-goto-line 10) works fine.

kaushalmodi commented 7 years ago

Here's the backtrace from emacs -Q:

Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil)
  avy-action-goto(nil)
  avy-goto-line(1)
  funcall-interactively(avy-goto-line 1)
  call-interactively(avy-goto-line)
  (let (archive pkgs) (let ((--dolist-tail-- (quote ((nil avy)))) archive-alist) (while --dolist-tail-- (setq archive-alist (car --dolist-tail--)) (setq archive (car archive-alist)) (if archive (progn (add-to-list (quote package-archives) archive :append))) (setq pkgs (append pkgs (cdr archive-alist))) (setq --dolist-tail-- (cdr --dolist-tail--)))) (package-initialize) (package-refresh-contents) (let ((--dolist-tail-- pkgs) pkg) (while --dolist-tail-- (setq pkg (car --dolist-tail--)) (if (and pkg (not (package-installed-p pkg))) (progn (package-install pkg))) (require pkg) (setq --dolist-tail-- (cdr --dolist-tail--)))) (call-interactively (function avy-goto-line)))
  (progn (require (quote package)) (setq user-emacs-directory (concat temporary-file-directory (getenv "USER") "/" ".emacs.d-debug/")) (setq package-user-dir (concat user-emacs-directory "elpa/")) (let (archive pkgs) (let ((--dolist-tail-- (quote ((nil avy)))) archive-alist) (while --dolist-tail-- (setq archive-alist (car --dolist-tail--)) (setq archive (car archive-alist)) (if archive (progn (add-to-list (quote package-archives) archive :append))) (setq pkgs (append pkgs (cdr archive-alist))) (setq --dolist-tail-- (cdr --dolist-tail--)))) (package-initialize) (package-refresh-contents) (let ((--dolist-tail-- pkgs) pkg) (while --dolist-tail-- (setq pkg (car --dolist-tail--)) (if (and pkg (not (package-installed-p pkg))) (progn (package-install pkg))) (require pkg) (setq --dolist-tail-- (cdr --dolist-tail--)))) (call-interactively (function avy-goto-line))))
  eval((progn (require (quote package)) (setq user-emacs-directory (concat temporary-file-directory (getenv "USER") "/" ".emacs.d-debug/")) (setq package-user-dir (concat user-emacs-directory "elpa/")) (let (archive pkgs) (let ((--dolist-tail-- (quote (...))) archive-alist) (while --dolist-tail-- (setq archive-alist (car --dolist-tail--)) (setq archive (car archive-alist)) (if archive (progn (add-to-list ... archive :append))) (setq pkgs (append pkgs (cdr archive-alist))) (setq --dolist-tail-- (cdr --dolist-tail--)))) (package-initialize) (package-refresh-contents) (let ((--dolist-tail-- pkgs) pkg) (while --dolist-tail-- (setq pkg (car --dolist-tail--)) (if (and pkg (not ...)) (progn (package-install pkg))) (require pkg) (setq --dolist-tail-- (cdr --dolist-tail--)))) (call-interactively (function avy-goto-line)))) nil)
  elisp--eval-last-sexp(nil)
  eval-last-sexp(nil)
  funcall-interactively(eval-last-sexp nil)
  call-interactively(eval-last-sexp nil nil)
  command-execute(eval-last-sexp)
kaushalmodi commented 7 years ago

Seems like re-compiling avy.el fixed it.. don't know how.

UPDATE: Re-compiling does not 'fix' it.. the problem goes away because I re-eval avy--process. After I restart emacs (with or without -Q), the problem comes back.

I then again tried the above method to recreate the problem in emacs -Q and I was able to do it.

Then when I re-evaluated just avy--process, this problem went away. Do you see anything unique in the way avy--process behaviour could change when compiled vs when eval'd?

kaushalmodi commented 7 years ago

I confirmed that this issue doesn't occur on emacs 25.2.1 too. Filed bug report.

abo-abo commented 7 years ago

Then when I re-evaluated just avy--process, this problem went away. Do you see anything unique in the way avy--process behaviour could change when compiled vs when eval'd?

So if you remove avy.elc and reload with emacs -Q on master, the problem goes away?

kaushalmodi commented 7 years ago

Yes, that's correct.

kaushalmodi commented 7 years ago

Turns out this bug was introduced by a different byte compilation implementation of cond/pcase/cl-case forms. As noted here, setting the byte-compile-cond-use-jump-table to nil also acts as a workaround to prevent this issue.

Developer of this new feature, @vibhavp, has a patch which I have verified to have resolved this issue: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25941#14

So closing this ticket.

Thanks @vibhavp!

kaushalmodi commented 7 years ago

I confirmed the fix in http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=092071345f265efcd3abd6de01552ebe95ffb9a1

To the people who happen to come to this thread as they still see this problem: Make sure you byte-recompile the .el files.. I did that for all packages.

(byte-recompile-directory package-user-dir nil :force)