Package: emacs;
Reported by: Boruch Baum <boruch_baum <at> gmx.com>
Date: Tue, 6 Dec 2016 02:21:02 UTC
Severity: minor
Tags: fixed, patch
Found in versions 24.4.50, 24.5
Fixed in version 26.1
Done: npostavs <at> users.sourceforge.net
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: npostavs <at> users.sourceforge.net To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> Cc: 25122 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>, Boruch Baum <boruch_baum <at> gmx.com> Subject: bug#25122: 24.5; function describe-variable hangs on large variables Date: Mon, 13 Mar 2017 10:01:13 -0400
[Message part 1 (text/plain, inline)]
npostavs <at> users.sourceforge.net writes: > npostavs <at> users.sourceforge.net writes: > >> Also, this doesn't really solve the performance problem, it just makes >> it much less likely to occur, e.g., (pp (list load-history)) is still >> slow. > > Okay, I think I found the real fix now: Missed a corner case.
[v2-0001-Don-t-reparse-the-sexp-in-indent-sexp-Bug-25122.patch (text/x-diff, inline)]
From b8dd372f65ce8979324c00b12a9ae767c1ffabda Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sun, 12 Mar 2017 23:59:19 -0400
Subject: [PATCH v2] Don't reparse the sexp in indent-sexp (Bug#25122)
* lisp/emacs-lisp/lisp-mode.el (calculate-lisp-indent): Let
PARSE-START be a parse state that can be reused.
(indent-sexp): Pass the running parse state to calculate-lisp-indent
instead of the sexp beginning position. Saving the
CONTAINING-SEXP-START returned by `calculate-lisp-indent' is no longer
needed.
* test/lisp/emacs-lisp/lisp-mode-tests.el (indent-sexp): Add blank
line to test case.
---
lisp/emacs-lisp/lisp-mode.el | 71 ++++++++++++++++++---------------
test/lisp/emacs-lisp/lisp-mode-tests.el | 5 ++-
2 files changed, 42 insertions(+), 34 deletions(-)
diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index eb07c18b03..3fefb69066 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -781,6 +781,10 @@ calculate-lisp-indent
If the value is nil, that means don't change the indentation
because the line starts inside a string.
+PARSE-START may be a buffer position to start parsing from, or a
+parse state as returned by calling `parse-partial-sexp' up to the
+beginning of the current line.
+
The value can also be a list of the form (COLUMN CONTAINING-SEXP-START).
This means that following lines at the same level of indentation
should not necessarily be indented the same as this line.
@@ -794,12 +798,14 @@ calculate-lisp-indent
(desired-indent nil)
(retry t)
calculate-lisp-indent-last-sexp containing-sexp)
- (if parse-start
- (goto-char parse-start)
- (beginning-of-defun))
- ;; Find outermost containing sexp
- (while (< (point) indent-point)
- (setq state (parse-partial-sexp (point) indent-point 0)))
+ (cond ((or (markerp parse-start) (integerp parse-start))
+ (goto-char parse-start))
+ ((null parse-start) (beginning-of-defun))
+ (t (setq state parse-start)))
+ (unless state
+ ;; Find outermost containing sexp
+ (while (< (point) indent-point)
+ (setq state (parse-partial-sexp (point) indent-point 0))))
;; Find innermost containing sexp
(while (and retry
state
@@ -1070,11 +1076,6 @@ indent-sexp
ENDPOS is encountered."
(interactive)
(let* ((indent-stack (list nil))
- ;; If ENDPOS is non-nil, use beginning of defun as STARTING-POINT.
- ;; If ENDPOS is nil, it is safe not to scan before point
- ;; since every line we indent is more deeply nested than point is.
- (starting-point (save-excursion (if endpos (beginning-of-defun))
- (point)))
;; Use `syntax-ppss' to get initial state so we don't get
;; confused by starting inside a string. We don't use
;; `syntax-ppss' in the loop, because this is measurably
@@ -1093,16 +1094,21 @@ indent-sexp
(save-excursion
(while (< (point) endpos)
;; Parse this line so we can learn the state to indent the
- ;; next line.
- (while (progn
- (setq state (parse-partial-sexp
- last-syntax-point (progn (end-of-line) (point))
- nil nil state))
- ;; Skip over newlines within strings.
- (nth 3 state))
- (setq state (parse-partial-sexp (point) (point-max)
- nil nil state 'syntax-table))
- (setq last-syntax-point (point)))
+ ;; next line. Preserve element 2 of the state (last sexp) for
+ ;; `calculate-lisp-indent'.
+ (let ((last-sexp (nth 2 state)))
+ (while (progn
+ (setq state (parse-partial-sexp
+ last-syntax-point (progn (end-of-line) (point))
+ nil nil state))
+ (setq last-sexp (or (nth 2 state) last-sexp))
+ ;; Skip over newlines within strings.
+ (nth 3 state))
+ (setq state (parse-partial-sexp (point) (point-max)
+ nil nil state 'syntax-table))
+ (setq last-sexp (or (nth 2 state) last-sexp))
+ (setq last-syntax-point (point)))
+ (setf (nth 2 state) last-sexp))
(setq next-depth (car state))
;; If the line contains a comment indent it now with
;; `indent-for-comment'.
@@ -1115,6 +1121,8 @@ indent-sexp
(make-list (- init-depth next-depth) nil))
last-depth (- last-depth next-depth)
next-depth init-depth))
+ ;; Now indent the next line according to what we learned from
+ ;; parsing the previous one.
(forward-line 1)
(when (< (point) endpos)
(let ((depth-delta (- next-depth last-depth)))
@@ -1124,28 +1132,25 @@ indent-sexp
(setq indent-stack (nconc (make-list depth-delta nil)
indent-stack))))
(setq last-depth next-depth))
- ;; Now indent the next line according
- ;; to what we learned from parsing the previous one.
- (skip-chars-forward " \t")
;; But not if the line is blank, or just a comment (we
;; already called `indent-for-comment' above).
+ (skip-chars-forward " \t")
(unless (or (eolp) (eq (char-syntax (char-after)) ?<))
- (let ((this-indent (car indent-stack)))
- (when (listp this-indent)
- (let ((val (calculate-lisp-indent
- (or (car this-indent) starting-point))))
- (setq
- this-indent
+ (indent-line-to
+ (or (car indent-stack)
+ ;; The state here is actually to the end of the
+ ;; previous line, but that's fine for our purposes.
+ ;; And parsing over the newline would only destroy
+ ;; element 2 (last sexp position).
+ (let ((val (calculate-lisp-indent state)))
(cond ((integerp val)
(setf (car indent-stack) val))
((consp val) ; (COLUMN CONTAINING-SEXP-START)
- (setf (car indent-stack) (cdr val))
(car val))
;; `calculate-lisp-indent' only returns nil
;; when we're in a string, but this won't
;; happen because we skip strings above.
- (t (error "This shouldn't happen!"))))))
- (indent-line-to this-indent))))))))
+ (t (error "This shouldn't happen!"))))))))))))
(defun indent-pp-sexp (&optional arg)
"Indent each line of the list starting just after point, or prettyprint it.
diff --git a/test/lisp/emacs-lisp/lisp-mode-tests.el b/test/lisp/emacs-lisp/lisp-mode-tests.el
index 2801f23df6..848dd7ca3e 100644
--- a/test/lisp/emacs-lisp/lisp-mode-tests.el
+++ b/test/lisp/emacs-lisp/lisp-mode-tests.el
@@ -31,6 +31,9 @@
1
2)
2)
+ (fun arg1
+
+ arg2)
(1
\"string
noindent\" (\"string2
@@ -58,7 +61,7 @@
(save-excursion
(let ((n 0))
(while (not (eobp))
- (unless (looking-at "noindent")
+ (unless (looking-at "noindent\\|^[[:blank:]]*$")
(insert (make-string n ?\s)))
(cl-incf n)
(forward-line))))
--
2.11.1
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.