bug#36136: [PATCH]: Re: bug#36136: syntax-ppss fails to invalidate its cache on changes to syntax-table text properties

[Alan Mackenzie <acm@muc.de>]

Hello, Emacs.

On Sat, Jun 08, 2019 at 13:17:24 +0000, Alan Mackenzie wrote:
> The syntax-ppss cache is not invalidated when syntax-table text
> properties are set or cleared.  This is because the invalidation
> function, syntax-ppss-flush-cache is invoked only as a before-change
> function, but typical (?all) syntax-table property changes happen when
> before-change-functions is inactive.

> This is a bug.

> In my debugging of a CC Mode scenario, a buffer change causes a
> syntax-table text property change at an earlier part of the buffer.  This
> is to do with the change making previously non-matching C++ raw string
> identifiers match up.  Font lock follows the syntax-ppss state, which
> spuriously records that the end part of the buffer is still in a string.
> Hence the non-string part of the buffer is still fontified with
> font-lock-string-face.

> 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.

It creates a new abnormal hook,
syntax-table-props-change-alert-functions, whose member functions are
called each time a syntax-table text property is changed.

syntax-ppss-flush-cache is inserted into this new hook at loading time
for syntax.el, thus invalidating the syntax-ppss cache each time a
syntax-table text property is changed.

There is a consequential amendment to the doc string of
inhibit-modification-hooks.

This fixes the original problem.

Comments?


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)
+
 ;;; Applying syntax-table properties where needed.
 
 (defvar syntax-propertize-function nil
diff --git a/src/insdel.c b/src/insdel.c
index 85fffd8fd1..5e978d1b29 100644
--- a/src/insdel.c
+++ b/src/insdel.c
@@ -2360,9 +2360,11 @@ syms_of_insdel (void)
   Vcombine_after_change_calls = Qnil;
 
   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.
 Setting this variable non-nil also inhibits file locks and checks
 whether files are locked by another Emacs session, as well as
 handling of the active region per `select-active-regions'.  */);
diff --git a/src/textprop.c b/src/textprop.c
index ae42c44185..050bb7b435 100644
--- a/src/textprop.c
+++ b/src/textprop.c
@@ -334,7 +334,10 @@ set_properties (Lisp_Object properties, INTERVAL interval, Lisp_Object object)
 	    record_property_change (interval->position, LENGTH (interval),
 				    XCAR (sym), XCAR (value),
 				    object);
-	  }
+            if (EQ (sym, Qsyntax_table))
+              CALLN (Frun_hook_with_args, Qsyntax_table_props_change_alert_functions,
+                     make_fixnum (interval->position));
+          }
 
       /* For each new property that has no value at all in the old plist,
 	 make an undo record binding it to nil, so it will be removed.  */
@@ -346,6 +349,9 @@ set_properties (Lisp_Object properties, INTERVAL interval, Lisp_Object object)
 	    record_property_change (interval->position, LENGTH (interval),
 				    XCAR (sym), Qnil,
 				    object);
+            if (EQ (sym, Qsyntax_table))
+              CALLN (Frun_hook_with_args, Qsyntax_table_props_change_alert_functions,
+                     make_fixnum (interval->position));
 	  }
     }
 
@@ -400,7 +406,10 @@ add_properties (Lisp_Object plist, INTERVAL i, Lisp_Object object,
 	      {
 		record_property_change (i->position, LENGTH (i),
 					sym1, Fcar (this_cdr), object);
-	      }
+                if (EQ (sym1, Qsyntax_table))
+                  CALLN (Frun_hook_with_args, Qsyntax_table_props_change_alert_functions,
+                         make_fixnum (i->position));
+              }
 
 	    /* I's property has a different value -- change it */
 	    if (set_type == TEXT_PROPERTY_REPLACE)
@@ -436,6 +445,9 @@ add_properties (Lisp_Object plist, INTERVAL i, Lisp_Object object,
 	    {
 	      record_property_change (i->position, LENGTH (i),
 				      sym1, Qnil, object);
+              if (EQ (sym1, Qsyntax_table))
+                CALLN (Frun_hook_with_args, Qsyntax_table_props_change_alert_functions,
+                       make_fixnum (i->position));
 	    }
 	  set_interval_plist (i, Fcons (sym1, Fcons (val1, i->plist)));
 	  changed = true;
@@ -470,9 +482,14 @@ remove_properties (Lisp_Object plist, Lisp_Object list, INTERVAL i, Lisp_Object
       while (CONSP (current_plist) && EQ (sym, XCAR (current_plist)))
 	{
 	  if (BUFFERP (object))
-	    record_property_change (i->position, LENGTH (i),
-				    sym, XCAR (XCDR (current_plist)),
-				    object);
+            {
+              record_property_change (i->position, LENGTH (i),
+                                      sym, XCAR (XCDR (current_plist)),
+                                      object);
+              if (EQ (sym, Qsyntax_table))
+                CALLN (Frun_hook_with_args, Qsyntax_table_props_change_alert_functions,
+                       make_fixnum (i->position));
+            }
 
 	  current_plist = XCDR (XCDR (current_plist));
 	  changed = true;
@@ -486,8 +503,13 @@ remove_properties (Lisp_Object plist, Lisp_Object list, INTERVAL i, Lisp_Object
 	  if (CONSP (this) && EQ (sym, XCAR (this)))
 	    {
 	      if (BUFFERP (object))
-		record_property_change (i->position, LENGTH (i),
-					sym, XCAR (XCDR (this)), object);
+                {
+                  record_property_change (i->position, LENGTH (i),
+                                          sym, XCAR (XCDR (this)), object);
+                  if (EQ (sym, Qsyntax_table))
+                    CALLN (Frun_hook_with_args, Qsyntax_table_props_change_alert_functions,
+                           make_fixnum (i->position));
+                }
 
 	      Fsetcdr (XCDR (tail2), XCDR (XCDR (this)));
 	      changed = true;
@@ -2319,6 +2341,20 @@ inherits it if NONSTICKINESS is nil.  The `front-sticky' and
   Vtext_property_default_nonsticky
     = list2 (Fcons (Qsyntax_table, Qt), Fcons (Qdisplay, Qt));
 
+  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.
+
+Each function is passed a single parameter, the buffer position of the
+start of the property change.  The current buffer is the one the
+change has been made in.  These functions get called even when
+`inhibit-modification-hooks' is non-nil.
+
+Note that these functions may NOT themselves make any buffer changes,
+including text property changes.  */);
+  Vsyntax_table_props_change_alert_functions = Qnil;
+
   interval_insert_behind_hooks = Qnil;
   interval_insert_in_front_hooks = Qnil;
   staticpro (&interval_insert_behind_hooks);
@@ -2337,6 +2373,7 @@ inherits it if NONSTICKINESS is nil.  The `front-sticky' and
   DEFSYM (Qrear_nonsticky, "rear-nonsticky");
   DEFSYM (Qmouse_face, "mouse-face");
   DEFSYM (Qminibuffer_prompt, "minibuffer-prompt");
+  DEFSYM (Qsyntax_table_props_change_alert_functions, "syntax-table-props-change-alert-functions");
 
   /* Properties that text might use to specify certain actions.  */
 


-- 
Alan Mackenzie (Nuremberg, Germany).