bug#21747: 25.0.50; while-no-input breaks kbd event handling when called from post-command-hook

[Tassilo Horn <tsdh@gnu.org>]

Tassilo Horn <tsdh@gnu.org> writes:

> By the way, I cannot reproduce an infloop in re-search-backward when
> LIMIT is zero.  Instead, with the current unchanged master code I also
> have pretty much the same infloop on the SMIE lisp level as in the
> previous mail where I patched the code in order not to run in LIMIT =
> 0 cases:
>
> 1 -> (aggressive-indent--indent-if-changed)

[snip]

> | | | 4 -> (sh-smie-sh-forward-token)
> | | | | 5 -> (sh-smie--newline-semi-p)
> | | | | | 6 -> (sh-smie-sh-backward-token)
> | | | | | | 7 -> (sh-smie--default-backward-token)
> | | | | | | 7 <- sh-smie--default-backward-token: "if"
> | | | | | | 7 -> (sh-smie--sh-keyword-p "if")
> | | | | | | | 8 -> (sh-smie--keyword-p)
> | | | | | | | | 9 -> (sh-smie-sh-backward-token)
> | | | | | | | | | 10 -> (sh-smie--default-backward-token)
> | | | | | | | | | 10 <- sh-smie--default-backward-token: ""
> | | | | | | | | 9 <- sh-smie-sh-backward-token: ""
> | | | | | | | 8 <- sh-smie--keyword-p: t
> | | | | | | 7 <- sh-smie--sh-keyword-p: t
> | | | | | 6 <- sh-smie-sh-backward-token: "if"
> | | | | 5 <- sh-smie--newline-semi-p: nil
> | | | 4 <- sh-smie-sh-forward-token: ";"

Ok, that loop is caused by the call to `end-of-defun' in
`aggressive-indent-indent-region-and-on':

          ;; And then we indent each following line until nothing happens.
          (forward-line 1)
          (skip-chars-forward "[:blank:]\n\r\xc")
          (let* ((eod (ignore-errors
                        (save-excursion (end-of-defun)
                                        (point-marker))))

end-of-defun calls beginning-of-defun-raw which works/terminates, and
then it calls end-of-defun-function which just does (forward-sexp 1)
which calls forward-sexp-function being smie-forward-sexp-command which
in turn calls smie-forward-sexp.  Well, and that calls smie-next-sexp
which loops because eventually the token returned by (funcall
next-token) where next-token is a function which calls
sh-smie-sh-forward-token returns ";".  That adds (38 38) to toklevels
and later to levels which is again popped somewhere below and then gets
re-added, popped again, re-added...

Hm, in one branch there's already a test if calling next-token actually
moved point and if not, it throws 'return to terminate the loop.  When I
move that out of the branch, that seems to fix the issue.

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/emacs-lisp/smie.el b/lisp/emacs-lisp/smie.el
index f305025..70ab6f3 100644
--- a/lisp/emacs-lisp/smie.el
+++ b/lisp/emacs-lisp/smie.el
@@ -706,6 +706,9 @@ smie-next-sexp
           (let* ((pos (point))
                  (token (funcall next-token))
                  (toklevels (cdr (assoc token smie-grammar))))
+            (if (eq pos (point))
+                ;; We did not move, so let's abort the loop.
+                (throw 'return (list t (point))))
             (cond
              ((null toklevels)
               (when (zerop (length token))
@@ -719,10 +722,7 @@ smie-next-sexp
                             (list t epos
                                   (buffer-substring-no-properties
                                    epos
-                                   (+ epos (if (< (point) epos) -1 1))))))))
-                (if (eq pos (point))
-                    ;; We did not move, so let's abort the loop.
-                    (throw 'return (list t (point))))))
+                                   (+ epos (if (< (point) epos) -1 1))))))))))
              ((not (numberp (funcall op-back toklevels)))
               ;; A token like a paren-close.
               (cl-assert (numberp  ; Otherwise, why mention it in smie-grammar.
--8<---------------cut here---------------end--------------->8---

But I'm really not sure if that's the right thing to do.  Maybe it's
better to fix sh-smie-sh-forward-token so that it doesn't return ";"
when actually no movement did occur because we already started out at
EOB.  That would be this:

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index fbb4a90..baed27b 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1920,10 +1920,11 @@ sh-smie-sh-forward-token
             ;; Pretend the here-document is a "newline representing a
             ;; semi-colon", since the here-doc otherwise covers the newline(s).
             ";")
-        (let ((semi (sh-smie--newline-semi-p)))
-          (forward-line 1)
-          (if (or semi (eobp)) ";"
-            (sh-smie-sh-forward-token))))
+        (unless (eobp)
+          (let ((semi (sh-smie--newline-semi-p)))
+            (forward-line 1)
+            (if (or semi (eobp)) ";"
+              (sh-smie-sh-forward-token)))))
     (forward-comment (point-max))
     (cond
      ((looking-at "\\\\\n") (forward-line 1) (sh-smie-sh-forward-token))
--8<---------------cut here---------------end--------------->8---

I actually can't judge what's the right fix.  I'd tend towards the more
general change in smie.el because I had this delayed display/100% CPU
issue also several times with clojure-mode though I haven't debugged it
in detail there so cannot say if it has been the same issue there.  Of
course, the right fix might be something completely different, too.

Bye,
Tassilo