Giving "text quotes" syntax in font-lock-syntax-table only

Giving "text quotes" syntax in font-lock-syntax-table only

[Ikumi Keita]

[-- Attachment #1: Type: text/plain, Size: 7030 bytes --]

Dear Emacs developers,
I was recommended to post to emacs-devel by Tassilo, AUCTeX maintainer.

I'd like to ask about SYNTAX-ALIST entry of `font-lock-defaults'. It
enables to redefine syntax table entries temporarily during font lock,
but is it allowed to assign "text quotes" syntax to a particular char
(dollar sign)?

Background:
In AUCTeX's TeX and LaTeX modes, $ normally has "paired delimiter ('$')"
syntax. But during font lock (by font-latex.el) we give it "string quote
('"')" syntax via the SYNTAX-ALIST in order to apply the right faces
for math (treated as strings here) through syntactic fontification.

However, we recently discovered that leads to the tricky bug#33139 where
under some special circumstances the syntax recognition is confused and
after a certain position of buffer math and non-math is fontified in
inverted way.

We think that the problem comes from running `syntax-ppss'. We
demonstrate by the attached toy example how font-latex.el implements the
syntactic fontification of math expression "$...$" and that
`syntax-ppss' actually interferes badly with that method. Here, the
example file hw.txt is taken and adopted from the report of bug#33139
mentioned above. Open it by
emacs -Q -geometry 140x35 -l bug-repro.el hw.txt
and type C-v three times. You will see that the paragraph beginning with
"Let $B$ be the subset ..." has inverted fontification.
(In case you didn't, do
xterm -geometry 80x25
and in that terminal
emacs -Q -nw -l bug-repro.el hw.txt
and type C-v eight times. The part beginning with "N$ and $c^{\mathcal
M}..." will be showed in inverted style.)
If the line adding `syntax-ppss' to `pre-command-hook' is deleted from
bug-repro.el, the above problem doesn't appear.

We think that the reason is that `syntax-ppss' is called with different
syntax-tables and that leads to some faulty cache entry:
(a) In font lock `font-lock-fontify-syntactically-region' calls
    `syntax-ppss' within
(with-syntax-table (or syntax-ppss-table (syntax-table)) ... ).
    Here, (syntax-table) returns the `font-lock-syntax-table' which
    gives "string quote" syntax to "$" according to SYNTAX-ALIST.
(b) On the other hand, `pre-command-hook' calls `syntax-ppss' with
    `bug-repro-mode-syntax-table', which gives "paired delimiter" syntax
    to "$".
Thus the two `syntax-ppss' can return different results conflicting to
each other which are cached deep in the guts of syntax.el.

Of course, the repro is contrieved but it seems something similar
happens to many users during normal LaTeX editing work. I suspect that
the bug#40930 and examples listed in its report are the same as the one
discussed now. I guess that user-enabled minor modes (or packages) use
syntax-aware functions extensibly in those examples, typically in
`pre-command-hook', and produce syntax ppss caches which conflict with
font-latex.el.

If we give $ string-quote syntax also in the mode's syntax-table, the
bug cannot be reproduced anymore.  But that's not something we want to
do in actual AUCTeX, as it has other unwanted effects.  Obviously,
inline math is not really a string.

It is certain that the key is the syntax ppss cache, which I could
confirm by the following try in the above example:
----------------------------------------------------------------------
--- bug-repro.el~	2020-05-19 13:17:23.000000000 +0900
+++ bug-repro.el	2020-05-19 21:41:12.232250000 +0900
@@ -18,4 +18,17 @@
   (setq font-lock-defaults
         '(nil nil nil ((?$ . "\""))
               (font-lock-syntactic-face-function
-               . bug-repro-syntactic-face-function))))
+               . bug-repro-syntactic-face-function)
+	      (font-lock-fontify-region-function
+	       . bug-repro-fontify-region))))
+
+(defvar bug-repro-syntax-ppss-cache (list nil nil -1))
+(make-variable-buffer-local 'bug-repro-syntax-ppss-cache)
+
+(defun bug-repro-fontify-region (beg end loudly)
+  (let ((syntax-ppss-wide (pop bug-repro-syntax-ppss-cache))
+	(syntax-ppss-narrow (pop bug-repro-syntax-ppss-cache))
+	(syntax-propertize--done (pop bug-repro-syntax-ppss-cache)))
+    (font-lock-default-fontify-region beg end loudly)
+    (setq bug-repro-syntax-ppss-cache
+	  (list syntax-ppss-wide syntax-ppss-narrow syntax-propertize--done))))
----------------------------------------------------------------------
This change attempts to use separate cache for syntax ppss during font
lock, and it really eliminates the wrong fontification for the above
example. However, it is unsatisfactory by at least two reasons:

1. When a similar attempt is adopted in AUCTeX (font-latex.el), editing
   the LaTeX document is fragile.  font-latex.el uses syntax-based
   fontification to fontify the argument of the \verb macro also, but it
   stops giving the right face when I insert "\verb|xyz|" with the above
   change. (But typing "$" just after it immediately makes "|xyz|"
   fontified.)
   
   I suppose the reason of this trouble is that the new cache isn't
   flushed properly. Whereas sytax.el uses `syntax-ppss-flush-cache' to
   update cache as the editing in the buffer proceeds, it doesn't touch
   the new cache hided in `*-syntax-ppss-cache' in the above patch.
2. It depends heavily on the implementation of caching of syntax.el.
   The above patch assumes that syntax ppss cache is managed by three
   variables `syntax-ppss-wide', `syntax-ppss-narrow' and
   `syntax-propertize--done'. But I don't know whether this assumption
   continues to hold in future nor it is appropriate for even current
   syntax.el.

   If the caching mechanism of syntax.el is not uniform across emacs
   versions, the maintenance cost of such codes would be much
   expensive.

   And even if syntax.el is stable enough, it is still not good to use
   such dependent code, especially to use internal variable with "--" in
   its name.

So I'd like to ask that:
(1) It doesn't work reliably to give "string quotes" syntax to $ in
    SYNTAX-ALIST. Is this an emacs bug or an intented restriction?
(2) If not a bug, is it reasonable to ask to extend the font lock
    framework to allow syntactic fontification of $...$ form? E.g.
    [A] Implement some kind of "separate cache" in syntax-ppss and
        make it available from font lock.
    [B] Extend syntax parse state to include information about "the
        position is in-math state or not" and make
	`font-lock-fontify-syntactically-region' responsive to the
        in-math state in addition to in-comment and in-string states.
        This way, it would no longer be necessary to redefine syntax of
        $ in SYNTAX-ALIST and we can naturally implement syntactic
        fontification of $...$ via `font-lock-syntactic-face-function'.
        This approach would be beneficial for standard tex-mode.el, and
        potentially other program modes as well, if realized.
(3) Or is there some smart way to achieve syntactic fontification of
    $...$ in the current font lock scheme?

I'm not on emacs-devel list, so please keep me in CC when replying.

Best regards,
Ikumi Keita


[-- Attachment #2: test kit --]
[-- Type: application/x-gzip, Size: 6199 bytes --]

Re: Giving "text quotes" syntax in font-lock-syntax-table only

[Stefan Monnier]

> We think that the problem comes from running `syntax-ppss'.

Yes, the problem is that `syntax-ppss` is affected by the current
syntax-table yet it doesn't pay attention to syntax-table changes when
it comes to enforcing that its cache is not stale.

The arguably better way to solve the problem is to not change the
syntax-table used inside `syntax-ppss` and `syntax-propertize`.  This is
especially true for `syntax-propertize` if you use `syntax-ppss` in your
`syntax-propertize-function`.

The best way to do that is to set `syntax-ppss-table` to the
syntax-table that should be used.


        Stefan

Re: Giving "text quotes" syntax in font-lock-syntax-table only

[Ikumi Keita]

Hi!

>>>>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> The best way to do that is to set `syntax-ppss-table` to the
> syntax-table that should be used.

I suppose that you are suggesting that something like this:
----------------------------------------------------------------------
(defvar bug-repro-mode-syntax-table
  (let ((st (make-syntax-table)))
    (modify-syntax-entry ?$ "$" st)
    (modify-syntax-entry ?% "<" st)
    (modify-syntax-entry ?\n ">" st)
    st))

(defun bug-repro-syntactic-face-function (state)
  (let ((char (nth 3 state)))
    (if char
        font-lock-string-face
      font-lock-comment-face)))

(define-derived-mode bug-repro-mode nil "BugRepro"
  :syntax-table bug-repro-mode-syntax-table
  (add-hook 'pre-command-hook #'syntax-ppss nil t)
  (setq font-lock-multiline t)
  (setq syntax-ppss-table
	(let ((st (make-syntax-table bug-repro-mode-syntax-table)))
	  (modify-syntax-entry ?$ "\"" st)
	  st))
  (setq font-lock-defaults
        '(nil nil nil nil ; ((?$ . "\""))
              (font-lock-syntactic-face-function
               . bug-repro-syntactic-face-function))))
----------------------------------------------------------------------
This resembles the method to modify the major mode's syntax table
(without setting `syntax-ppss-table'). It does eliminate the wrong
fontification, but brings another problem in actual use cases of AUCTeX.

When `syntax-ppss-table' is non-nil, all functions using `syntax-ppss'
obey that syntax table, so they regard math expression $...$ as string
constant. This can be problematic for cases like this:
----------------------------------------------------------------------
\documentclass{article}
\begin{document}
$x^{2}+y^{2} % syntax-ppss wouldn't regard me as a comment.
=z^{2}$
\end{document}
----------------------------------------------------------------------

In other words, what we want to carry out is:
- to use modified syntax table only within font lock
- while keeping to use major mode's syntax table for all other purposes.

If we temporarily bind `syntax-ppss-table' during font lock by `let',
the problem of conflicting cache emerges again and it causes the
inverted fontification, if I understand correctly.

I can think of no way to achieve our intention in the current framework
of syntax.el and font lock.

I'm speaking without knowing the detail of syntax.el, so please correct
me if I understand something wrongly.

Regards,
Ikumi Keita

Re: Giving "text quotes" syntax in font-lock-syntax-table only

[Ikumi Keita]

Hi emacs developers,

>>>>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> The best way to do that is to set `syntax-ppss-table` to the
>> syntax-table that should be used.

Thanks for your advice, but unfortunately I couldn't find a good
solution yet.

1. As I wrote previously, another problem occurs if we set constant
`syntax-ppss-table' in the major mode.

2. I actually tried the following implementation:
>>>>> Ikumi Keita <ikumi@ikumi.que.jp> writes:
> If we temporarily bind `syntax-ppss-table' during font lock by `let',
> the problem of conflicting cache emerges again and it causes the
> inverted fontification, if I understand correctly.
----------------------------------------------------------------------
(defvar bug-repro-mode-syntax-table
  (let ((st (make-syntax-table)))
    (modify-syntax-entry ?$ "$" st)
    (modify-syntax-entry ?% "<" st)
    (modify-syntax-entry ?\n ">" st)
    st))

(defvar bug-repro-syntax-ppss-table
  (let ((st (make-syntax-table bug-repro-mode-syntax-table)))
    (modify-syntax-entry ?$ "\"" st)
    st))

(defun bug-repro-syntactic-face-function (state)
  (let ((char (nth 3 state)))
    (if char
        font-lock-string-face
      font-lock-comment-face)))

(define-derived-mode bug-repro-mode nil "BugRepro"
  :syntax-table bug-repro-mode-syntax-table
  (add-hook 'pre-command-hook #'syntax-ppss nil t)
  (setq font-lock-multiline t)
  (setq font-lock-defaults
        '(nil nil nil nil ; ((?$ . "\""))
              (font-lock-syntactic-face-function
               . bug-repro-syntactic-face-function)
	      (font-lock-fontify-region-function
	       . bug-repro-fontify-region))))

(defun bug-repro-fontify-region (beg end loudly)
  (let ((syntax-ppss-table bug-repro-syntax-ppss-table))
    (font-lock-default-fontify-region beg end loudly)))
----------------------------------------------------------------------

But the result was just what I anticipated.

So may I ask the following questions again?
(1) It doesn't work reliably to give "string quotes" syntax to $ in
    SYNTAX-ALIST. Is this an emacs bug or an intented restriction?
(2) If not a bug, is it reasonable to ask to extend the font lock
    framework to allow syntactic fontification of $...$ form? E.g.
    [A] Implement some kind of "separate cache" in syntax-ppss and
        make it available from font lock.
    [B] Extend syntax parse state to include information about "the
        position is in-math state or not" and make
	`font-lock-fontify-syntactically-region' responsive to the
        in-math state in addition to in-comment and in-string states.
        This way, it would no longer be necessary to redefine syntax of
        $ in SYNTAX-ALIST and we can naturally implement syntactic
        fontification of $...$ via `font-lock-syntactic-face-function'.
        This approach would be beneficial for standard tex-mode.el, and
        potentially other program modes as well, if realized.
(3) Or is there some smart way to achieve syntactic fontification of
    $...$ in the current font lock scheme?

Best regards,
Ikumi Keita

Re: Giving "text quotes" syntax in font-lock-syntax-table only

[Stefan Monnier]

Sorry for not getting back to you sooner.
I see you (plural) decided to switch to highlighting math elements using
regexps rather than the syntax-table.  That sounds like a wise choice.

> This resembles the method to modify the major mode's syntax table
> (without setting `syntax-ppss-table'). It does eliminate the wrong
> fontification, but brings another problem in actual use cases of AUCTeX.
> When `syntax-ppss-table' is non-nil, all functions using `syntax-ppss'
> obey that syntax table, so they regard math expression $...$ as string
> constant. This can be problematic for cases like this:

Indeed.  What you could do if you still want to use syntax-tables rather
than regexp, is to add an entry to the beginning of `font-lock-keywords`
which does the equivalent of `font-lock-fontify-syntactically-region`
but without using `syntax-ppss` and using its own syntax-table.

Maybe this actually argues that `font-lock-fontify-syntactically-region`
should use `font-lock-syntax-table` rather than `syntax-ppss-table` and
should refrain from using `syntax-ppss` when that syntax-table is
different from that of syntax-ppss.


        Stefan

Re: Giving "text quotes" syntax in font-lock-syntax-table only

[Ikumi Keita]

Hi Stefan,

>>>>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> I see you (plural) decided to switch to highlighting math elements using
> regexps rather than the syntax-table.  That sounds like a wise choice.

Thanks. It is lucky that we have texmathp.el, a smart utility created by
Carsten Dominik. It gives us a way practical enough to identify whether
a given position of latex document buffer is in math mode or not.

> Maybe this actually argues that `font-lock-fontify-syntactically-region`
> should use `font-lock-syntax-table` rather than `syntax-ppss-table` and
> should refrain from using `syntax-ppss` when that syntax-table is
> different from that of syntax-ppss.

Maybe. I suppose that it should be documented, for the time being, that
SYNTAX-ALIST entry of `font-lock-defaults' may not introduce dratistic
change in syntax table such as changing assignments of "string quotes"
syntax.

Regards,
Ikumi Keita

Re: Giving "text quotes" syntax in font-lock-syntax-table only

[Stefan Monnier]

>> We think that the problem comes from running `syntax-ppss'.
>
> Yes, the problem is that `syntax-ppss` is affected by the current
> syntax-table yet it doesn't pay attention to syntax-table changes when
> it comes to enforcing that its cache is not stale.
>
> The arguably better way to solve the problem is to not change the
> syntax-table used inside `syntax-ppss` and `syntax-propertize`.  This is
> especially true for `syntax-propertize` if you use `syntax-ppss` in your
> `syntax-propertize-function`.
>
> The best way to do that is to set `syntax-ppss-table` to the
> syntax-table that should be used.

While I think the new approach using `font-lock-keywords` instead of
`font-lock-syntax-table` is a better solution, I think we should try and
avoid this breakage.

Could you test the patch below to see if it makes the old code
work correctly?


        Stefan


diff --git a/lisp/font-lock.el b/lisp/font-lock.el
index e0955b74abc..f4ac00d14bd 100644
--- a/lisp/font-lock.el
+++ b/lisp/font-lock.el
@@ -575,6 +575,7 @@ font-lock-syntax-table
   "Non-nil means use this syntax table for fontifying.
 If this is nil, the major mode's syntax table is used.
 This is normally set via `font-lock-defaults'.")
+(defvar-local font-lock--syntax-table-affects-ppss nil)
 
 (defvar font-lock-mark-block-function nil
   "Non-nil means use this function to mark a block of text.
@@ -1610,7 +1611,15 @@ font-lock-fontify-syntactically-region
 	       (regexp-quote
 	        (replace-regexp-in-string "^ *" "" comment-end))))
           ;; Find the `start' state.
-          (state (syntax-ppss start))
+          (state (if (or syntax-ppss-table
+                         (not font-lock--syntax-table-affects-ppss))
+                     (syntax-ppss start)
+                   ;; If `syntax-ppss' doesn't have its own syntax-table and
+                   ;; we have installed out own syntax-table which
+                   ;; differs from the standard one in ways which affects PPSS,
+                   ;; then we can't use `syntax-ppss' since that would pollute
+                   ;; and be polluted by its cache.
+                   (parse-partial-sexp (point-min) start)))
           face beg)
       (if loudly (message "Fontifying %s... (syntactically...)" (buffer-name)))
       ;;
@@ -1907,6 +1916,7 @@ font-lock-set-defaults
       ;; Case fold during regexp fontification?
       (setq-local font-lock-keywords-case-fold-search (nth 2 defaults))
       ;; Syntax table for regexp and syntactic fontification?
+      (kill-local-variable 'font-lock--syntax-table-affects-ppss)
       (if (null (nth 3 defaults))
           (setq-local font-lock-syntax-table nil)
 	(setq-local font-lock-syntax-table (copy-syntax-table (syntax-table)))
@@ -1916,7 +1926,14 @@ font-lock-set-defaults
 	    (dolist (char (if (numberp (car selem))
 			      (list (car selem))
 			    (mapcar #'identity (car selem))))
-	      (modify-syntax-entry char syntax font-lock-syntax-table)))))
+	      (unless (memq (car (aref font-lock-syntax-table char))
+	                    '(1 2 3))    ;"." "w" "_"
+	        (setq font-lock--syntax-table-affects-ppss t))
+	      (modify-syntax-entry char syntax font-lock-syntax-table)
+	      (unless (memq (car (aref font-lock-syntax-table char))
+	                    '(1 2 3))    ;"." "w" "_"
+	        (setq font-lock--syntax-table-affects-ppss t))
+	      ))))
       ;; (nth 4 defaults) used to hold `font-lock-beginning-of-syntax-function',
       ;; but that was removed in 25.1, so if it's a cons cell, we assume that
       ;; it's part of the variable alist.

Re: Giving "text quotes" syntax in font-lock-syntax-table only

[Ikumi Keita]

Hi Stefan,

>>>>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> Could you test the patch below to see if it makes the old code
> work correctly?

Yes, the fontification doesn't get scrambled with this modification.

If I understand correctly, fontification still can fail even with this
fix, if `font-lock-syntax-table' is modified after font lock mode is
enabled. So I think it's preferable to advertise that
`font-lock-syntax-table' should not modified directly, unless the
modification is restricted within translation between ".",  "w" and "_",
once after font lock mode is enabled.

Regards,
Ikumi Keita

Re: Giving "text quotes" syntax in font-lock-syntax-table only

[Stefan Monnier]

>> Could you test the patch below to see if it makes the old code
>> work correctly?
> Yes, the fontification doesn't get scrambled with this modification.

Thanks.  I pushed it to `master`.

> If I understand correctly, fontification still can fail even with this
> fix, if `font-lock-syntax-table' is modified after font lock mode is
> enabled.

Yes, if `font-lock-syntax-table` is modified/set directly rather than
via the 4th element of `font-lock-defaults`.

> So I think it's preferable to advertise that `font-lock-syntax-table'
> should not modified directly, unless the modification is restricted
> within translation between ".",  "w" and "_", once after font lock
> mode is enabled.

We should probably deprecate the use of `font-lock-syntax-table`, indeed.


        Stefan