From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#36136: [PATCH]: Re: bug#36136: syntax-ppss fails to invalidate its cache on changes to syntax-table text properties Date: Sun, 09 Jun 2019 08:56:45 +0300 Message-ID: <83ef43cndu.fsf@gnu.org> References: <20190608131724.GA4643@ACM> <20190608203639.GA28722@ACM> Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="65870"; mail-complaints-to="usenet@blaine.gmane.org" Cc: 36136@debbugs.gnu.org To: Alan Mackenzie Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun Jun 09 07:57:16 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1hZqpQ-000Gw1-6y for geb-bug-gnu-emacs@m.gmane.org; Sun, 09 Jun 2019 07:57:16 +0200 Original-Received: from localhost ([::1]:33906 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hZqpJ-0003hn-SE for geb-bug-gnu-emacs@m.gmane.org; Sun, 09 Jun 2019 01:57:09 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:33255) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hZqpD-0003hV-Dy for bug-gnu-emacs@gnu.org; Sun, 09 Jun 2019 01:57:04 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hZqpC-0001OG-7c for bug-gnu-emacs@gnu.org; Sun, 09 Jun 2019 01:57:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:40550) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hZqpC-0001Nz-42 for bug-gnu-emacs@gnu.org; Sun, 09 Jun 2019 01:57:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hZqpC-0002Lk-0K for bug-gnu-emacs@gnu.org; Sun, 09 Jun 2019 01:57:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 09 Jun 2019 05:57:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 36136 X-GNU-PR-Package: emacs Original-Received: via spool by 36136-submit@debbugs.gnu.org id=B36136.15600598219029 (code B ref 36136); Sun, 09 Jun 2019 05:57:01 +0000 Original-Received: (at 36136) by debbugs.gnu.org; 9 Jun 2019 05:57:01 +0000 Original-Received: from localhost ([127.0.0.1]:54094 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hZqpB-0002LV-0C for submit@debbugs.gnu.org; Sun, 09 Jun 2019 01:57:01 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:52733) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hZqp8-0002LI-KK for 36136@debbugs.gnu.org; Sun, 09 Jun 2019 01:57:00 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:55113) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hZqp3-0001IJ-1t; Sun, 09 Jun 2019 01:56:53 -0400 Original-Received: from [176.228.60.248] (port=2466 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1hZqp2-0000Wu-Ea; Sun, 09 Jun 2019 01:56:52 -0400 In-reply-to: <20190608203639.GA28722@ACM> (message from Alan Mackenzie on Sat, 8 Jun 2019 20:36:39 +0000) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 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.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:160253 Archived-At: > Date: Sat, 8 Jun 2019 20:36:39 +0000 > From: Alan Mackenzie > > > Suggested fix: the functions set_properties, add_properties, > > remove_properties in textprop.c should check for changes to, > > specifically, syntax-table properties. When these changes are detected, > > a hook called something like syntax-table-props-change-alert-hook should > > be called (with some appropriate position parameters, tbd). > > syntax-ppss-flush-cache will be added to this hook. > > Here is a first draught of a fix to this bug. I have no opinion about the issue and the idea of its proposed solution, but I do have some comments to the implementation. > diff --git a/lisp/emacs-lisp/syntax.el b/lisp/emacs-lisp/syntax.el > index 9c6d5b5829..673d598de6 100644 > --- a/lisp/emacs-lisp/syntax.el > +++ b/lisp/emacs-lisp/syntax.el > @@ -43,6 +43,10 @@ > > (eval-when-compile (require 'cl-lib)) > > +;; Ensure that the cache gets invalidated when `syntax-table' text > +;; properties get set, even when `inhibit-modification-hooks' is non-nil. > +(add-hook 'syntax-table-props-change-alert-functions #'syntax-ppss-flush-cache) This means that whenever syntax.el is loaded (i.e., always, since syntax.el is preloaded), this hook will be non-nil. Is that a good idea? I mean, if we always call this function, why do this via a hook? > DEFVAR_BOOL ("inhibit-modification-hooks", inhibit_modification_hooks, > - doc: /* Non-nil means don't run any of the hooks that respond to buffer changes. > + doc: /* Non-nil means don't run most of the hooks that respond to buffer changes. > This affects `before-change-functions' and `after-change-functions', > -as well as hooks attached to text properties and overlays. > +as well as most hooks attached to text properties and overlays. > +However the hook `syntax-table-props-change-alert-functions' gets run > +regardless of the setting of this variable. If you go to such lengths, please be sure to mention that the new hook will NOT run if run-hooks is nil. > + if (EQ (sym, Qsyntax_table)) > + CALLN (Frun_hook_with_args, Qsyntax_table_props_change_alert_functions, > + make_fixnum (interval->position)); These calls make set/add/remove_properties slower. AFAIK, those functions are hotspots in Emacs, so please make these calls as efficient as possible. In particular, I would call run_hook_with_args directly, bypassing an extra Frun_hook_with_args function call. Better still, if we abandon the idea of doing this via a hook, calling what is now a hook function directly will be more efficient still. Also, what about the safety of this call? what if the hook signals an error or the user presses C-g while the hook runs? IOW, should you use safe_call or some of its variants, and should you inhibit QUIT? and if so, whether and how should you handle in the code the case when the hook does signal an error? > + DEFVAR_LISP ("syntax-table-props-change-alert-functions", > + Vsyntax_table_props_change_alert_functions, > + doc: /* List of functions to call each time a `syntax-table' text property > +gets added, removed, or changed. The first line of a doc string should be a complete sentence. > +Each function is passed a single parameter, the buffer position of the > +start of the property change. I think you also know the length of the text where you call the hook, so maybe pass that as well? > + These functions get called even when > +`inhibit-modification-hooks' is non-nil. But not when run-hooks is nil. > +Note that these functions may NOT themselves make any buffer changes, > +including text property changes. */); And how do we enforce this? > + Vsyntax_table_props_change_alert_functions = Qnil; This should have a comment saying the value is assigned in syntax.el, i.e. it is always non-nil in a dumped Emacs.