bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Noam Postavsky]

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

On the emacs-25 and master branches, changing the syntax-table
temporarily affects the table used during syntax-propertize. This does
not happen in 24.5 (unless the difference is just a timing thing?).
This triggers an infinite loop when expanding certain snippets with
yasnippet in haskell-mode (see [1] and [2]).

Attached is a simplified reproducer, run as

    emacs -Q -l synprop-bug.el

In Emacs 25+, this results in the message

Error during redisplay: (internal--syntax-propertize 2) signaled
(error "syntax table is changed to standard-syntax-table during syntax
propertize")


In Emacs 24.5, it runs without a problem. It's not entirely clear to
me who should be reponsible for ensuring the right syntax-table during
syntax-propertize.

[1]: https://github.com/capitaomorte/yasnippet/issues/687
[2]: https://github.com/haskell/haskell-mode/issues/1315

[-- Attachment #2: synprop-bug.el --]
[-- Type: text/x-emacs-lisp, Size: 824 bytes --]

(defun test-check-syntax-table (begin end)
  (unless (eq (syntax-table) test-syntax-prop-mode-syntax-table)
    (error "syntax table is changed to %s during syntax propertize"
           (if (eq (syntax-table) (standard-syntax-table))
               'standard-syntax-table
             'other-syntax-table))))

(define-derived-mode test-syntax-prop-mode prog-mode "TestSyntaxProp"
  "A mode for testing syntax-propertize bug(?)."
  (setq-local syntax-propertize-function #'test-check-syntax-table)
  (setq-local font-lock-defaults
              '(nil nil nil nil nil
                (font-lock-syntactic-face-function . ignore)
                (parse-sexp-lookup-properties . t))))

(switch-to-buffer "tt")
(test-syntax-prop-mode)
(erase-buffer)
(insert "foo")
(with-syntax-table (standard-syntax-table)
  (scan-sexps 1 1))

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Dmitry Gutov]

Hi Noam,

On 05/04/2016 01:41 PM, Noam Postavsky wrote:
> In Emacs 24.5, it runs without a problem. It's not entirely clear to
> me who should be reponsible for ensuring the right syntax-table during
> syntax-propertize.

IIUC, the difference is that in Emacs 25 scan-sexps and other low-level 
primitives started calling syntax-propertize when appropriate. And in 
24.5 test-check-syntax-table is only called after (probably by font-lock).

In that's the reason, the new behavior is the correct one.

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Noam Postavsky]

On Sat, May 7, 2016 at 8:58 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> Hi Noam,
>
> On 05/04/2016 01:41 PM, Noam Postavsky wrote:
>>
>> In Emacs 24.5, it runs without a problem. It's not entirely clear to
>> me who should be reponsible for ensuring the right syntax-table during
>> syntax-propertize.
>
>
> IIUC, the difference is that in Emacs 25 scan-sexps and other low-level
> primitives started calling syntax-propertize when appropriate. And in 24.5
> test-check-syntax-table is only called after (probably by font-lock).
>
> In that's the reason, the new behavior is the correct one.

So yasnippet mustn't use scan-sexps whilst the syntax-table has been changed?

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Dmitry Gutov]

On 05/08/2016 09:51 PM, Noam Postavsky wrote:

> So yasnippet mustn't use scan-sexps whilst the syntax-table has been changed?

I don't know the whole issue, but it should be possible to reorganize 
the code that when scan-sexps is called, the current syntax table is the 
appropriate one.

But I don't know why you're changing it in the first place.

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Noam Postavsky]

On Sun, May 8, 2016 at 2:53 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> On 05/08/2016 09:51 PM, Noam Postavsky wrote:
>
>> So yasnippet mustn't use scan-sexps whilst the syntax-table has been
>> changed?
>
>
> I don't know the whole issue, but it should be possible to reorganize the
> code that when scan-sexps is called, the current syntax table is the
> appropriate one.
>
> But I don't know why you're changing it in the first place.

That scan-sexps is being to parse the snippet syntax, which is why
it's not using the major-mode's syntax table. I suppose the proper way
to fix this is to parse the whole snippet before inserting it into the
buffer, but I was hoping to put off the code reorganization that would
entail for a while longer.

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Dmitry Gutov]

On 05/08/2016 09:58 PM, Noam Postavsky wrote:

> That scan-sexps is being to parse the snippet syntax, which is why
> it's not using the major-mode's syntax table. I suppose the proper way
> to fix this is to parse the whole snippet before inserting it into the
> buffer,

It seems so.

> but I was hoping to put off the code reorganization that would
> entail for a while longer.

Here's a possible stopgap:

- Narrow the buffer to the snippet (just to be safe).
- Bind syntax-propertize-function to nil.
- Call scan-sexps.
- Call (syntax-ppss-flush-cache snippet-beginning-position).

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Andreas Röhler]

On 08.05.2016 02:58, Dmitry Gutov wrote:
> Hi Noam,
>
> On 05/04/2016 01:41 PM, Noam Postavsky wrote:
>> In Emacs 24.5, it runs without a problem. It's not entirely clear to
>> me who should be reponsible for ensuring the right syntax-table during
>> syntax-propertize.
>
> IIUC, the difference is that in Emacs 25 scan-sexps and other 
> low-level primitives started calling syntax-propertize when appropriate. 

AFAIS scan-sexps, which calls scan-lists internally, updates the 
syntax-table during scan.
Is this reasonable?

May you point me to the spot resp. discussion, when this change was made?

Thanks,

Andreas


> And in 24.5 test-check-syntax-table is only called after (probably by 
> font-lock).
>
> In that's the reason, the new behavior is the correct one.
>
>
>

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Dmitry Gutov]

On 05/10/2016 08:34 PM, Andreas Röhler wrote:

> AFAIS scan-sexps, which calls scan-lists internally, updates the
> syntax-table during scan.
> Is this reasonable?
>
> May you point me to the spot resp. discussion, when this change was made?

I don't recall there being a discussion. It was a longstanding problem 
which Stefan fixed (or at least improved on) during this development cycle.

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Andreas Röhler]

On 10.05.2016 22:42, Dmitry Gutov wrote:
> On 05/10/2016 08:34 PM, Andreas Röhler wrote:
>
>> AFAIS scan-sexps, which calls scan-lists internally, updates the
>> syntax-table during scan.
>> Is this reasonable?
>>
>> May you point me to the spot resp. discussion, when this change was 
>> made?
>
> I don't recall there being a discussion. It was a longstanding problem 
> which Stefan fixed (or at least improved on) during this development 
> cycle.

For me it looks like a bug-source rather than a fix. Do you recall what 
the mentioned longstanding problem was?

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Dmitry Gutov]

On 05/11/2016 08:20 AM, Andreas Röhler wrote:
> Do you recall what
> the mentioned longstanding problem was?

The problem was scan-lists not always honoring the syntax defined by 
syntax-propertize-function. Because it could behave differently 
depending on whether the nearby parts of the buffer had been already 
syntax-propertized.

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Andreas Röhler]

On 11.05.2016 11:20, Dmitry Gutov wrote:
> On 05/11/2016 08:20 AM, Andreas Röhler wrote:
>> Do you recall what
>> the mentioned longstanding problem was?
>
> The problem was scan-lists not always honoring the syntax defined by 
> syntax-propertize-function. Because it could behave differently 
> depending on whether the nearby parts of the buffer had been already 
> syntax-propertized.

Thanks for the info. Is there a test-case resp. bug-report describing 
the original problem?
IMO it needs a different solution than now.

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Dmitry Gutov]

On 05/11/2016 03:13 PM, Andreas Röhler wrote:

> Thanks for the info. Is there a test-case resp. bug-report describing
> the original problem?

There may be some, but I'm not aware of particular ones.

> IMO it needs a different solution than now.

I couldn't imagine a radically different solution.

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Noam Postavsky]

close 23443
tags 23443 + notabug
quit

On Sun, May 8, 2016 at 3:12 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> Here's a possible stopgap:
>
> - Narrow the buffer to the snippet (just to be safe).
> - Bind syntax-propertize-function to nil.
> - Call scan-sexps.
> - Call (syntax-ppss-flush-cache snippet-beginning-position).

Thanks, this seems to be working well (implemented in [1]).

[1]: https://github.com/capitaomorte/yasnippet/pull/695

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Andreas Röhler]

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

|reopen| bugnumber

The proposed solution, which has to bind syntax-propertize-function to 
nil before calling scan-sexp, reads like a work around. These things 
shouldn't be necessary.

Assume a fix must affect usage of syntax-propertize-function and probably is not a trivial one.
Its design seems worth further discussion.

For example reading:

"The specified function may call ‘syntax-ppss’ on any position
before END, but it should not call ‘syntax-ppss-flush-cache’,
which means that it should not call ‘syntax-ppss’ on some
position and later modify the buffer on some earlier position."

Emacs can do better.

On 12.05.2016 12:05, Noam Postavsky wrote:
> close 23443
> tags 23443 + notabug
> quit
>
> On Sun, May 8, 2016 at 3:12 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
>> Here's a possible stopgap:
>>
>> - Narrow the buffer to the snippet (just to be safe).
>> - Bind syntax-propertize-function to nil.
>> - Call scan-sexps.
>> - Call (syntax-ppss-flush-cache snippet-beginning-position).
> Thanks, this seems to be working well (implemented in [1]).
>
> [1]: https://github.com/capitaomorte/yasnippet/pull/695
>
>
>


[-- Attachment #2: Type: text/html, Size: 1854 bytes --]

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Andreas Röhler]

reopen 23443

The proposed solution, which has to bind syntax-propertize-function to 
nil before calling scan-sexp, reads like a work around. These things 
shouldn't be necessary.

Assume a fix must affect usage of syntax-propertize-function and probably is not a trivial one.
Its design seems worth further discussion.

For example reading:

"The specified function may call ‘syntax-ppss’ on any position
before END, but it should not call ‘syntax-ppss-flush-cache’,
which means that it should not call ‘syntax-ppss’ on some
position and later modify the buffer on some earlier position."

bug#23443: 25.0.93; Temporary change to syntax-table is in effect during syntax-propertize

[Dmitry Gutov]

On 05/13/2016 08:49 AM, Andreas Röhler wrote:
> |reopen| bugnumber

Please don't.

> The proposed solution, which has to bind syntax-propertize-function to
> nil before calling scan-sexp, reads like a work around. These things
> shouldn't be necessary.

It's not a solution (because there is no bug here), it's something that 
a poorly-behaving function can do to behave better.