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

On Sat, Jun 08, 2019 at 13:17:24 +0000, Alan Mackenzie wrote:
> Hello, Emacs.

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

> -- 
> Alan Mackenzie (Nuremberg, Germany).

The following patch is simpler than my first proposal, following
feedback from Eli.  It works for me.

Stefan, could you look at this, please?


Make syntax-ppss react to changes in syntax-table text properties

In particular, it trims its cache to the point of such a change.  This fixes
bug #36136.

* src/textprop.c (syntax-propertize--done): New buffer local variable.
(set_properties, add_properties, remove_properties): when a syntax-table text
property is being changed, reduce syntax-propertize--done to the buffer
position.

* lisp/emacs-lisp/syntax.el (syntax-ppss--trim-cache): New function extracted
from syntax-ppss-flush-cache.
(syntax-ppss-flush-cache): Now only modifies syntax-propertize--done and
syntax-ppss--done.
(syntax-ppss): Calls syntax-ppss--trim-cache and sets syntax-propertize--done.


diff --git a/lisp/emacs-lisp/syntax.el b/lisp/emacs-lisp/syntax.el
index 9c6d5b5829..6ca27d8e83 100644
--- a/lisp/emacs-lisp/syntax.el
+++ b/lisp/emacs-lisp/syntax.el
@@ -404,13 +404,10 @@ syntax-ppss-narrow
 (defvar-local syntax-ppss-narrow-start nil
   "Start position of the narrowing for `syntax-ppss-narrow'.")
 
-(define-obsolete-function-alias 'syntax-ppss-after-change-function
-  #'syntax-ppss-flush-cache "27.1")
-(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.
-  (setq syntax-propertize--done (min beg syntax-propertize--done))
-  ;; Flush invalid cache entries.
+(defun syntax-ppss--trim-cache (beg)
+  "Flush the cache specific to `syntax-ppss' from position BEG.
+
+This doesn't include the cache for `syntax-propertize'."
   (dolist (cell (list syntax-ppss-wide syntax-ppss-narrow))
     (pcase cell
       (`(,last . ,cache)
@@ -432,8 +429,16 @@ syntax-ppss-flush-cache
        ;; (unless cache
        ;;   (remove-hook 'before-change-functions #'syntax-ppss-flush-cache t))
        (setcar cell last)
-       (setcdr cell cache)))
-    ))
+       (setcdr cell cache)))))
+
+(define-obsolete-function-alias 'syntax-ppss-after-change-function
+  #'syntax-ppss-flush-cache "27.1")
+(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.
+  (setq syntax-propertize--done (min beg syntax-propertize--done))
+  ;; Flush invalid cache entries.
+  (setq syntax-ppss--done (min beg syntax-ppss--done)))
 
 ;;; FIXME: Explain this variable.  Currently only its last (5th) slot is used.
 ;;; Perhaps the other slots should be removed?
@@ -477,8 +482,10 @@ syntax-ppss
 running the hook."
   ;; Default values.
   (unless pos (setq pos (point)))
+  (syntax-ppss--trim-cache syntax-ppss--done)
   (syntax-propertize pos)
   ;;
+  (setq syntax-ppss--done (max syntax-ppss--done pos))
   (with-syntax-table (or syntax-ppss-table (syntax-table))
   (let* ((cell (syntax-ppss--data))
          (ppss-last (car cell))
diff --git a/src/textprop.c b/src/textprop.c
index ae42c44185..be9e34c889 100644
--- a/src/textprop.c
+++ b/src/textprop.c
@@ -334,6 +334,9 @@ 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)
+                && (interval->position < syntax_ppss__done))
+              syntax_ppss__done = interval->position;
 	  }
 
       /* For each new property that has no value at all in the old plist,
@@ -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)
+                && (interval->position < syntax_ppss__done))
+              syntax_ppss__done = interval->position;
 	  }
     }
 
@@ -400,6 +406,9 @@ 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)
+                    && (i->position < syntax_ppss__done))
+                  syntax_ppss__done = i->position;
 	      }
 
 	    /* I's property has a different value -- change it */
@@ -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)
+                  && (i->position < syntax_ppss__done))
+                syntax_ppss__done = 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)
+                  && (i->position < syntax_ppss__done))
+                syntax_ppss__done = 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)
+                      && (i->position < syntax_ppss__done))
+                    syntax_ppss__done = i->position;
+                }
 
 	      Fsetcdr (XCDR (tail2), XCDR (XCDR (this)));
 	      changed = true;
@@ -2319,6 +2341,11 @@ inherits it if NONSTICKINESS is nil.  The `front-sticky' and
   Vtext_property_default_nonsticky
     = list2 (Fcons (Qsyntax_table, Qt), Fcons (Qdisplay, Qt));
 
+  DEFVAR_INT ("syntax-ppss--done", syntax_ppss__done,
+              doc: /* Position up to which the `syntax-ppss' cache is valid.  */);
+  syntax_ppss__done = -1;
+  Fmake_variable_buffer_local (intern ("syntax_ppss__done"));
+
   interval_insert_behind_hooks = Qnil;
   interval_insert_in_front_hooks = Qnil;
   staticpro (&interval_insert_behind_hooks);


-- 
Alan Mackenzie (Nuremberg, Germany).