Re: master 4b39b74: python.el: don't syntax-propertize single/double quoted strings

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: master 4b39b74: python.el: don't syntax-propertize single/double quoted strings

Glenn Morris-3
Stefan Monnier wrote:

> branch: master
> commit 4b39b741f1949ebad1dfccc5032dfce521bedc2a

>     python.el: don't syntax-propertize single/double quoted strings

This causes python-tests--python-nav-end-of-statement--infloop to fail.
Ref eg https://hydra.nixos.org/build/91945586

Reply | Threaded
Open this post in threaded view
|

Re: master 4b39b74: python.el: don't syntax-propertize single/double quoted strings

Stefan Monnier
>> branch: master
>> commit 4b39b741f1949ebad1dfccc5032dfce521bedc2a
>
>>     python.el: don't syntax-propertize single/double quoted strings
>
> This causes python-tests--python-nav-end-of-statement--infloop to fail.
> Ref eg https://hydra.nixos.org/build/91945586

Hmm... I can't find a better way than to disable it, sorry.
So .... "Fixed!"


        Stefan

Reply | Threaded
Open this post in threaded view
|

Re: master 4b39b74: python.el: don't syntax-propertize single/double quoted strings

Clément Pit-Claudel
On 2019-04-10 10:41, Stefan Monnier wrote:

>>> branch: master
>>> commit 4b39b741f1949ebad1dfccc5032dfce521bedc2a
>>
>>>     python.el: don't syntax-propertize single/double quoted strings
>>
>> This causes python-tests--python-nav-end-of-statement--infloop to fail.
>> Ref eg https://hydra.nixos.org/build/91945586
>
> Hmm... I can't find a better way than to disable it, sorry.
> So .... "Fixed!"

But that test was added to prevent regression of an actual bug, right?
Did your commit reintroduce the bug? If so, disabling the test doesn't sound ideal…

Clément.

Reply | Threaded
Open this post in threaded view
|

Re: master 4b39b74: python.el: don't syntax-propertize single/double quoted strings

Stefan Monnier
> But that test was added to prevent regression of an actual bug, right?
> Did your commit reintroduce the bug? If so, disabling the test doesn't sound ideal…

Here was my answer in the actual commit,


        Stefan


diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index 94c846ecb1..999cf8dc7a 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -5345,13 +5345,23 @@ python-tests-shell-interpreter
 (ert-deftest python-tests--python-nav-end-of-statement--infloop ()
   "Checks that `python-nav-end-of-statement' doesn't infloop in a
 buffer with overlapping strings."
+  ;; FIXME: The treatment of strings has changed in the mean time, and the
+  ;; test below now neither signals an error nor inf-loops.
+  ;; The description of the problem it's trying to catch is not clear enough
+  ;; to be able to see if the underlying problem is really fixed, sadly.
+  ;; E.g. I don't know what is meant by "overlap", really.
+  (skip-unless nil)
   (python-tests-with-temp-buffer "''' '\n''' ' '\n"
     (syntax-propertize (point-max))
     ;; Create a situation where strings nominally overlap.  This
     ;; shouldn't happen in practice, but apparently it can happen when
     ;; a package calls `syntax-ppss' in a narrowed buffer during JIT
     ;; lock.
+    ;; FIXME: 4-5 is the SPC right after the opening triple quotes: why
+    ;; put a string-fence syntax on it?
     (put-text-property 4 5 'syntax-table (string-to-syntax "|"))
+    ;; FIXME: 8-9 is the middle quote in the closing triple quotes:
+    ;; it shouldn't have any syntax-table property to remove anyway!
     (remove-text-properties 8 9 '(syntax-table nil))
     (goto-char 4)
     (setq-local syntax-propertize-function nil)


Reply | Threaded
Open this post in threaded view
|

Re: master 4b39b74: python.el: don't syntax-propertize single/double quoted strings

Clément Pit-Claudel
On 2019-04-10 12:23, Stefan Monnier wrote:
>> But that test was added to prevent regression of an actual bug, right?
>> Did your commit reintroduce the bug? If so, disabling the test doesn't sound ideal…
>
> Here was my answer in the actual commit,

Ah, thanks.


Reply | Threaded
Open this post in threaded view
|

Re: master 4b39b74: python.el: don't syntax-propertize single/double quoted strings

Noam Postavsky
In reply to this post by Clément Pit-Claudel
On Wed, 10 Apr 2019 at 11:18, Clément Pit-Claudel <[hidden email]> wrote:

> >> This causes python-tests--python-nav-end-of-statement--infloop to fail.

> > Hmm... I can't find a better way than to disable it, sorry.

> But that test was added to prevent regression of an actual bug, right?
> Did your commit reintroduce the bug? If so, disabling the test doesn't sound ideal…

I think the related report is Bug#30964, but there was never a clear
reproducer for it. The commit that introduced the test is [1: 4fbd330fae],
whose commit message says: "[...] Unfortunately it is
impossible to reproduce without manually destroying the syntactic
information in the Python buffer, but it has been observed in
practice [...]".

[1: 4fbd330fae]: 2017-03-23 23:05:19 +0100
  Protect against an infloop in python-mode
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=4fbd330fae54a9c45d4a717127aa86d75e9938d5

Reply | Threaded
Open this post in threaded view
|

Re: master 4b39b74: python.el: don't syntax-propertize single/double quoted strings

Andreas Röhler-2


On 11.04.19 01:59, Noam Postavsky wrote:

> On Wed, 10 Apr 2019 at 11:18, Clément Pit-Claudel <[hidden email]> wrote:
>
>>>> This causes python-tests--python-nav-end-of-statement--infloop to fail.
>>> Hmm... I can't find a better way than to disable it, sorry.
>> But that test was added to prevent regression of an actual bug, right?
>> Did your commit reintroduce the bug? If so, disabling the test doesn't sound ideal…
> I think the related report is Bug#30964, but there was never a clear
> reproducer for it. The commit that introduced the test is [1: 4fbd330fae],
> whose commit message says: "[...] Unfortunately it is
> impossible to reproduce without manually destroying the syntactic
> information in the Python buffer, but it has been observed in
> practice [...]".
>
> [1: 4fbd330fae]: 2017-03-23 23:05:19 +0100
>    Protect against an infloop in python-mode
>    https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=4fbd330fae54a9c45d4a717127aa86d75e9938d5
>

Hi,

if syntax-ppss is the cause, than that kind of bug is sourced in other
places/modes too. As fixing syntax-ppss seems difficult, being
interested to learn about cases, where it can't be replaced by calls to
parse-partial-sexp.

Best,

Andreas



Reply | Threaded
Open this post in threaded view
|

Re: master 4b39b74: python.el: don't syntax-propertize single/double quoted strings

Noam Postavsky
On Fri, 26 Apr 2019 at 04:12, Andreas Röhler <[hidden email]> wrote:

> if syntax-ppss is the cause,

Apparently the actual cause was the regex code not being reentrant:
https://debbugs.gnu.org/30964#38

I guess the test attempts to reproduce the incorrect syntax info which
gets produced in that case. So maybe there is not much sense in
keeping it now that the regexp code is reentrant.