bug#37045: lisp-indent-region hangs in lisp-indent-calc-next when indenting the three characters "|#\n"

bug#37045: lisp-indent-region hangs in lisp-indent-calc-next when indenting the three characters "|#\n"

[eschulte]

[-- Attachment #1.1: Type: text/plain, Size: 363 bytes --]

Hi,

Indent-region sometimes hangs when indenting a lisp file.  A minimal
example is the file just containing the characters "|#" followed by a
newline.  This hang is due to an infinite loop in the while loop in the
`lisp-indent-calc-next' function.  The following version of this
function fixes this problem, but is certainly not the appropriate
long-term fix.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: lisp-indent-calc-next-workaround.lisp --]
[-- Type: text/x-lisp, Size: 3495 bytes --]

;;; Work around a bug in `lisp-indent-calc-next' in which it can hang
;;; on a dangling '|#'.  The change in the code below against the
;;; original is the addition of a block and a throw to this block when
;;; we're infinite looping (when the newly added last-last-sexp
;;; matches last-sexp).  On the off chance it is possible in normal
;;; execution for last-sexp to stay the same for some number of loop
;;; iterations this patch only aborts if they're equal 100 times in a
;;; row.
(defun lisp-indent-calc-next (state)
  "Move to next line and return calculated indent for it.
STATE is updated by side effect, the first state should be
created by `lisp-indent-initial-state'.  This function may move
by more than one line to cross a string literal."
  (pcase-let* (((cl-struct lisp-indent-state
                           (stack indent-stack) ppss ppss-point)
                state)
               (indent-depth (car ppss)) ; Corresponding to indent-stack.
               (depth indent-depth)
               (last-last-sexp nil) (counter 0))
    (block nil
      ;; Parse this line so we can learn the state to indent the
      ;; next line.
      (while (let ((last-sexp (nth 2 ppss)))
               (setq ppss (parse-partial-sexp
                           ppss-point (progn (end-of-line) (point))
                           nil nil ppss))
               ;; Preserve last sexp of state (position 2) for
               ;; `calculate-lisp-indent', if we're at the same depth.
               (if (and (not (nth 2 ppss)) (= depth (car ppss)))
                   (setf (nth 2 ppss) last-sexp)
                 (setq last-sexp (nth 2 ppss)))
               (setq depth (car ppss))
               (if (and (>= counter 100) (equal last-sexp last-last-sexp))
                   (progn (setf indent-stack nil) (return))
                 (setq last-last-sexp last-sexp
                       counter (1+ counter)))
               ;; Skip over newlines within strings.
               (nth 3 ppss))
        (let ((string-start (nth 8 ppss)))
          (setq ppss (parse-partial-sexp (point) (point-max)
                                         nil nil ppss 'syntax-table))
          (setf (nth 2 ppss) string-start) ; Finished a complete string.
          (setq depth (car ppss)))
        (setq ppss-point (point)))
      (setq ppss-point (point))
      (let* ((depth-delta (- depth indent-depth)))
        (cond ((< depth-delta 0)
               (setq indent-stack (nthcdr (- depth-delta) indent-stack)))
              ((> depth-delta 0)
               (setq indent-stack (nconc (make-list depth-delta nil)
                                         indent-stack))))))

    (prog1
        (let (indent)
          (cond ((= (forward-line 1) 1) nil)
                ;; Negative depth, probably some kind of syntax error.
                ((null indent-stack)
                 ;; Reset state.
                 (setq ppss (parse-partial-sexp (point) (point))))
                ((car indent-stack))
                ((integerp (setq indent (calculate-lisp-indent ppss)))
                 (setf (car indent-stack) indent))
                ((consp indent)       ; (COLUMN CONTAINING-SEXP-START)
                 (car indent))
                ;; This only happens if we're in a string.
                (t (error "This shouldn't happen"))))
      (setf (lisp-indent-state-stack state) indent-stack)
      (setf (lisp-indent-state-ppss-point state) ppss-point)
      (setf (lisp-indent-state-ppss state) ppss))))

[-- Attachment #1.3: Type: text/plain, Size: 427 bytes --]


Thanks,
Eric

P.S. I hope this isn't a duplicate report, I tried to search both the
archive https://lists.gnu.org/archive/html/bug-gnu-emacs/ and the
previous bug reports https://debbugs.gnu.org/ but I didn't find anything
relevant.


-- 
Eric Schulte, Ph.D.
Director of Automated Software Engineering
GrammaTech, Inc.
PGP Fingerprint: FA8D C2C3 E8A0 A749 34CD  9DCF 3C1B 8581 614C A05D
Pronouns: he, him, his

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#37045: lisp-indent-region hangs in lisp-indent-calc-next when indenting the three characters "|#\n"

[Noam Postavsky]

[-- Attachment #1: Type: text/plain, Size: 1223 bytes --]

found 37045 26.1
tags 37045 + patch
quit

eschulte@grammatech.com writes:

> Indent-region sometimes hangs when indenting a lisp file.  A minimal
> example is the file just containing the characters "|#" followed by a
> newline.

It's actually any unfinished string literal (in Common Lisp, |...| is
treated by Emacs as a string literal, though technically it's an escaped
symbol).

> This hang is due to an infinite loop in the while loop in the
> `lisp-indent-calc-next' function.  The following version of this
> function fixes this problem, but is certainly not the appropriate
> long-term fix.

I think the right fix is just to check for end of buffer:

@@ -810,7 +810,7 @@ lisp-indent-calc-next
                (setq last-sexp (nth 2 ppss)))
              (setq depth (car ppss))
              ;; Skip over newlines within strings.
-             (nth 3 ppss))
+             (and (not (eobp)) (nth 3 ppss)))
       (let ((string-start (nth 8 ppss)))
         (setq ppss (parse-partial-sexp (point) (point-max)
                                        nil nil ppss 'syntax-table))

Full patch with test (and some other comment updates) attached below.
Is this okay for emacs-26?  The bug is a regression from Emacs 25.


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 3226 bytes --]

From ab3f0d0d09163862c62efa4d67e4e56cdef60716 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Fri, 16 Aug 2019 07:26:40 -0400
Subject: [PATCH] Fix lisp indent infloop on unfinished strings (Bug#37045)

* lisp/emacs-lisp/lisp-mode.el (lisp-indent-calc-next): Stop trying to
skip over strings if we've hit the end of buffer.
* test/lisp/emacs-lisp/lisp-mode-tests.el
(lisp-indent-unfinished-string): New test.
---
 lisp/emacs-lisp/lisp-mode.el            | 15 ++++++++++-----
 test/lisp/emacs-lisp/lisp-mode-tests.el |  6 +++++-
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index 74bf0c87c5..bde0a4ea6d 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -810,7 +810,7 @@ lisp-indent-calc-next
                (setq last-sexp (nth 2 ppss)))
              (setq depth (car ppss))
              ;; Skip over newlines within strings.
-             (nth 3 ppss))
+             (and (not (eobp)) (nth 3 ppss)))
       (let ((string-start (nth 8 ppss)))
         (setq ppss (parse-partial-sexp (point) (point-max)
                                        nil nil ppss 'syntax-table))
@@ -826,17 +826,22 @@ lisp-indent-calc-next
                                        indent-stack)))))
     (prog1
         (let (indent)
-          (cond ((= (forward-line 1) 1) nil)
-                ;; Negative depth, probably some kind of syntax error.
+          (cond ((= (forward-line 1) 1)
+                 ;; Can't move to the next line, apparently end of buffer.
+                 nil)
                 ((null indent-stack)
-                 ;; Reset state.
+                 ;; Negative depth, probably some kind of syntax
+                 ;; error.  Reset the state.
                  (setq ppss (parse-partial-sexp (point) (point))))
                 ((car indent-stack))
                 ((integerp (setq indent (calculate-lisp-indent ppss)))
                  (setf (car indent-stack) indent))
                 ((consp indent)       ; (COLUMN CONTAINING-SEXP-START)
                  (car indent))
-                ;; This only happens if we're in a string.
+                ;; This only happens if we're in a string, but the
+                ;; loop should always skip over strings (unless we hit
+                ;; end of buffer, which is taken care of by the first
+                ;; clause).
                 (t (error "This shouldn't happen"))))
       (setf (lisp-indent-state-stack state) indent-stack)
       (setf (lisp-indent-state-ppss-point state) ppss-point)
diff --git a/test/lisp/emacs-lisp/lisp-mode-tests.el b/test/lisp/emacs-lisp/lisp-mode-tests.el
index 63632449ca..e4ba929ecb 100644
--- a/test/lisp/emacs-lisp/lisp-mode-tests.el
+++ b/test/lisp/emacs-lisp/lisp-mode-tests.el
@@ -284,7 +284,11 @@ lisp-indent-with-read-only-field
     (lisp-indent-line)
     (should (equal (buffer-string) "prompt> foo"))))
 
-
+(ert-deftest lisp-indent-unfinished-string ()
+  "Don't infloop on unfinished string (Bug#37045)."
+  (with-temp-buffer
+    (insert "\"\n")
+    (lisp-indent-region (point-min) (point-max))))
 
 (provide 'lisp-mode-tests)
 ;;; lisp-mode-tests.el ends here
-- 
2.11.0

bug#37045: lisp-indent-region hangs in lisp-indent-calc-next when indenting the three characters "|#\n"

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@gmail.com>
> Date: Fri, 16 Aug 2019 07:47:20 -0400
> Cc: 37045@debbugs.gnu.org
> 
> Is this okay for emacs-26?  The bug is a regression from Emacs 25.

Yes, thanks.

bug#37045: lisp-indent-region hangs in lisp-indent-calc-next when indenting the three characters "|#\n"

[Noam Postavsky]

tags 37045 fixed
close 37045 26.3
quit

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Noam Postavsky <npostavs@gmail.com>
>> Date: Fri, 16 Aug 2019 07:47:20 -0400
>> Cc: 37045@debbugs.gnu.org
>> 
>> Is this okay for emacs-26?  The bug is a regression from Emacs 25.
>
> Yes, thanks.

Pushed to emacs-26.

bcd0115e4d 2019-08-17T09:42:34-04:00 "Fix lisp indent infloop on unfinished strings (Bug#37045)"
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=bcd0115e4db49791a77566b80fabc4384d9ebf57