From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#46713: 27.1; Variable binding depth exceeds max-specpdl-size in js-mode with js-indent-level 2 and indent-tabs-mode nil on new line Date: Sat, 25 Jun 2022 08:03:04 -0400 Message-ID: References: <871rd7enmz.fsf@gnus.org> <87ft1mdcei.fsf@gnus.org> <87k09cvjxx.fsf@gnus.org> Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="18063"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: Ryan Olson , 46713@debbugs.gnu.org To: Lars Ingebrigtsen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Jun 25 14:04:26 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1o54WX-0004RS-EF for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 25 Jun 2022 14:04:25 +0200 Original-Received: from localhost ([::1]:58872 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1o54WW-00071V-64 for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 25 Jun 2022 08:04:24 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:41886) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o54WA-00070l-Oy for bug-gnu-emacs@gnu.org; Sat, 25 Jun 2022 08:04:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:50411) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1o54WA-0003sq-Fj for bug-gnu-emacs@gnu.org; Sat, 25 Jun 2022 08:04:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1o54WA-0006ZM-C7 for bug-gnu-emacs@gnu.org; Sat, 25 Jun 2022 08:04:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 25 Jun 2022 12:04:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 46713 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: confirmed Original-Received: via spool by 46713-submit@debbugs.gnu.org id=B46713.165615860325175 (code B ref 46713); Sat, 25 Jun 2022 12:04:02 +0000 Original-Received: (at 46713) by debbugs.gnu.org; 25 Jun 2022 12:03:23 +0000 Original-Received: from localhost ([127.0.0.1]:44302 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1o54VW-0006Xz-MS for submit@debbugs.gnu.org; Sat, 25 Jun 2022 08:03:23 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:18441) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1o54VS-0006Xj-N3 for 46713@debbugs.gnu.org; Sat, 25 Jun 2022 08:03:21 -0400 Original-Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 0A0DE100241; Sat, 25 Jun 2022 08:03:13 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id C0C26100134; Sat, 25 Jun 2022 08:03:10 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1656158590; bh=IgG4en8d4eJ1wVcBVCg9N/I2q4gmKL7l2dPihCWovUE=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=ngsSI3fNiva4KkFznkqpZJkxyJofqFP8Unj68rRfAM5tqgh5YqcuzxjVO6Q64LmGy DhmN/Q5xdUBYcLcPXwPMRyMa4tFCwPGh0dx0xiowiW98NfCF1Y3kIGc6l2w7K3+NsD IKi0KevKLQYb3krrR31smz4CGloRNQ9+zpbkMUGrgq3D2viS4/EaeAOe7uYced13FH s1TnVRCgy/ez5wxgRdUeuv2Z0ORXjgYQB6uTg8n5LIlgW+HTsW12uYZqffFvPEtITa 3y6RZvLrIRQJO2R6NEpNOb4uarYpjOYTK8DWwUtKFXH18x1UI60rkqPp3/VTP48mRp IOgfXsJMMlMcg== Original-Received: from alfajor (smb-adp03.hotspot.hub-one.net [94.199.126.51]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id DEFB41204E9; Sat, 25 Jun 2022 08:03:09 -0400 (EDT) In-Reply-To: <87k09cvjxx.fsf@gnus.org> (Lars Ingebrigtsen's message of "Sun, 19 Jun 2022 15:53:14 +0200") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:235254 Archived-At: > Perhaps Stefan knows what's going on here; added to the CCs. [ The recipe works without the js-indent-level and indent-tabs-mode settings. ] I think the crux of the matter is the following: (run-hook-wrapped 'syntax-propertize-extend-region-functions (lambda (f) (let ((new (funcall f start end)) ;; Avoid recursion! (syntax-propertize--done most-positive-fixnum)) (if (or (null new) (and (>= (car new) start) (<= (cdr new) end))) nil Where the binding of `syntax-propertize--done` should be around the call to `f` (apparently I introduced this bug in commit 3928ef2dd5b8febf3b1d9c1bfb22af3698d16bea). But the bug also shows that it's difficult for syntax-propertize-functions to know when they use `syntax-ppss` since it can happen as a side-effect of forward-sexp and hence I'm beginning to think it's not practical to require them to call `syntax-ppss-flush-cache` manually. I'm thinking of installing the patch below on `master`. We could install a simpler patch on `emacs-28` which just swaps the two vars in the above `let` (and changes it into a `let*`). WDYT? Stefan * lisp/emacs-lisp/syntax.el: Rework the handling of nested calls. Nested calls to `syntax-ppss` and `syntax-propertize` can easily happen unexpectedly via ondemand propertizing or `forward-sexp`. Refine the handling of nested calls so we detect them more reliably (e.g. also within `syntax-propertize-extend-region-functions`) and so that the `syntax-ppss` cache is automatically flushed in case it might have been filled with data that's become obsolete since. (syntax-propertize--inhibit-flush): Delete var. (syntax-propertize--in-process-p): New function to replace it. (syntax-ppss-flush-cache): Use it. (syntax-ppss--updated-cache): New var. (syntax-propertize): Make `syntax-propertize--done` binding apply to `syntax-propertize-extend-region-functions` as well, as intended (fixes bug#46713). Use `syntax-ppss--updated-cache` to flush syntax-ppss cache at the end when needed. Don't bind `syntax-propertize--inhibit-flush` any more. (syntax-ppss): Set `syntax-ppss--updated-cache` when applicable. diff --git a/lisp/emacs-lisp/syntax.el b/lisp/emacs-lisp/syntax.el index a4d7beade13..36b0c56e953 100644 --- a/lisp/emacs-lisp/syntax.el +++ b/lisp/emacs-lisp/syntax.el @@ -345,10 +345,16 @@ syntax-propertize-via-font-lock (defvar-local syntax-ppss-table nil "Syntax-table to use during `syntax-ppss', if any.") -(defvar-local syntax-propertize--inhibit-flush nil - "If non-nil, `syntax-ppss-flush-cache' only flushes the ppss cache. -Otherwise it flushes both the ppss cache and the properties -set by `syntax-propertize'") +(defun syntax-propertize--in-process-p () + "Non-nil if we're inside `syntax-propertize'. +This is used to avoid infinite recursion as well as to handle cases where +`syntax-ppss' is called when the final `syntax-table' properties have not +yet been setup, in which case we may end up putting invalid info into the cache. +It's also used so that `syntax-ppss-flush-cache' can be used from within +`syntax-propertize' without ruining the `syntax-table' already set." + (eq syntax-propertize--done most-positive-fixnum)) + +(defvar-local syntax-ppss--updated-cache nil) (defun syntax-propertize (pos) "Ensure that syntax-table properties are set until POS (a buffer point)." @@ -370,21 +376,24 @@ syntax-propertize (with-silent-modifications (with-syntax-table (or syntax-ppss-table (syntax-table)) (make-local-variable 'syntax-propertize--done) ;Just in case! + ;; Make sure we let-bind it only buffer-locally. + (make-local-variable 'syntax-ppss--updated-cache) (let* ((start (max (min syntax-propertize--done (point-max)) (point-min))) (end (max pos (min (point-max) (+ start syntax-propertize-chunk-size)))) (first t) - (repeat t)) + (repeat t) + (syntax-ppss--updated-cache nil)) (while repeat (setq repeat nil) (run-hook-wrapped 'syntax-propertize-extend-region-functions (lambda (f) - (let ((new (funcall f start end)) - ;; Avoid recursion! - (syntax-propertize--done most-positive-fixnum)) + ;; Bind `syntax-propertize--done' to avoid recursion! + (let* ((syntax-propertize--done most-positive-fixnum) + (new (funcall f start end))) (if (or (null new) (and (>= (car new) start) (<= (cdr new) end))) nil @@ -399,20 +408,26 @@ syntax-propertize ;; Flush ppss cache between the original value of `start' and that ;; set above by syntax-propertize-extend-region-functions. (syntax-ppss-flush-cache start) - ;; Move the limit before calling the function, so the function - ;; can use syntax-ppss. + ;; Move the limit before calling the function, so it's + ;; done in case of errors. (setq syntax-propertize--done end) ;; (message "syntax-propertizing from %s to %s" start end) (remove-text-properties start end '(syntax-table nil syntax-multiline nil)) - ;; Make sure we only let-bind it buffer-locally. - (make-local-variable 'syntax-propertize--inhibit-flush) - ;; Let-bind `syntax-propertize--done' to avoid infinite recursion! - (let ((syntax-propertize--done most-positive-fixnum) - ;; Let `syntax-propertize-function' call - ;; `syntax-ppss-flush-cache' without worries. - (syntax-propertize--inhibit-flush t)) - (funcall syntax-propertize-function start end))))))))) + ;; Bind `syntax-propertize--done' to avoid recursion! + (let ((syntax-propertize--done most-positive-fixnum)) + (funcall syntax-propertize-function start end) + (when syntax-ppss--updated-cache + ;; `syntax-ppss' was called and updated the cache while we + ;; were propertizing so we need to flush the part of the + ;; cache that may have been rendered out-of-date by the new + ;; properties. + ;; We used to require syntax-propertize-functions to do that + ;; manually when applicable, but nowadays the `syntax-ppss' + ;; cache can be updated by too many functions, so the author + ;; of the syntax-propertize-function may not be aware it + ;; can happen. + (syntax-ppss-flush-cache start)))))))))) ;;; Link syntax-propertize with syntax.c. @@ -487,10 +502,10 @@ syntax-ppss-narrow-start (define-obsolete-function-alias 'syntax-ppss-after-change-function #'syntax-ppss-flush-cache "27.1") -(defun syntax-ppss-flush-cache (beg &rest ignored) +(defun syntax-ppss-flush-cache (beg &rest _ignored) "Flush the cache of `syntax-ppss' starting at position BEG." ;; Set syntax-propertize to refontify anything past beg. - (unless syntax-propertize--inhibit-flush + (unless (syntax-propertize--in-process-p) (setq syntax-propertize--done (min beg syntax-propertize--done))) ;; Flush invalid cache entries. (dolist (cell (list syntax-ppss-wide syntax-ppss-narrow)) @@ -517,10 +532,16 @@ syntax-ppss-flush-cache (setcdr cell cache))) )) -;;; FIXME: Explain this variable. Currently only its last (5th) slot is used. -;;; Perhaps the other slots should be removed? +;; FIXME: Explain this variable. Currently only its last (5th) slot is used. +;; Perhaps the other slots should be removed? +;; This variable is only used when `syntax-begin-function' is used and +;; will hence be removed together with `syntax-begin-function'. (defvar syntax-ppss-stats - [(0 . 0) (0 . 0) (0 . 0) (0 . 0) (0 . 0) (2 . 2500)]) + [(0 . 0) (0 . 0) (0 . 0) (0 . 0) (0 . 0) (2 . 2500)] + "Statistics about which case is more/less frequent in `syntax-ppss'. +The 5th slot drives the heuristic to use `syntax-begin-function'. +The rest is only useful if you're interested in tweaking the algorithm.") + (defun syntax-ppss-stats () (mapcar (lambda (x) (condition-case nil @@ -658,6 +679,7 @@ syntax-ppss ;; populate the cache so we won't need to do it again soon. (t (syntax-ppss--update-stats 3 pt-min pos) + (setq syntax-ppss--updated-cache t) ;; If `pt-min' is too far, add a few intermediate entries. (while (> (- pos pt-min) (* 2 syntax-ppss-max-span)) @@ -692,6 +714,7 @@ syntax-ppss (push pair ppss-cache) (setcar ppss-cache pair))))))))) + (setq syntax-ppss--updated-cache t) (setq ppss-last (cons pos ppss)) (setcar cell ppss-last) (setcdr cell ppss-cache)