elisp--local-variables triggers an error on special input

elisp--local-variables triggers an error on special input

[Evgeny Zajcev]

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

I've noticed that sometimes company raises an error in my *scratch*, I've
shrinked the case and got:

(with-current-buffer (get-buffer-create "pwp")
  (erase-buffer)
  (insert "((eg \"\C-,\")")
  (elisp--local-variables))

Evaluating this sexp raises "Invalid modifier in string" error

Not sure this is intended, I think elisp--local-variables expects
`invalid-read-syntax` error for this input, but got another error and does
not handle it preventing error from happening

-- 
lg

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

Re: elisp--local-variables triggers an error on special input

[Joost Kremers]

On Tue, Dec 01 2020, Evgeny Zajcev wrote:
> I've noticed that sometimes company raises an error in my *scratch*, I've
> shrinked the case and got:
>
> (with-current-buffer (get-buffer-create "pwp")
>   (erase-buffer)
>   (insert "((eg \"\C-,\")")
>   (elisp--local-variables))
>
> Evaluating this sexp raises "Invalid modifier in string" error

You get that error by just eval'ing:

    (insert "((eg \"\C-,\")")

in the *scratch* buffer. So it doesn't seem to be related to
`elisp--local-variables`.

-- 
Joost Kremers
Life has its moments

Re: elisp--local-variables triggers an error on special input

[tomas]

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

On Tue, Dec 01, 2020 at 02:15:48PM +0100, Joost Kremers wrote:
> 
> On Tue, Dec 01 2020, Evgeny Zajcev wrote:
> > I've noticed that sometimes company raises an error in my *scratch*, I've
> > shrinked the case and got:
> >
> > (with-current-buffer (get-buffer-create "pwp")
> >   (erase-buffer)
> >   (insert "((eg \"\C-,\")")
> >   (elisp--local-variables))
> >
> > Evaluating this sexp raises "Invalid modifier in string" error
> 
> You get that error by just eval'ing:
> 
>     (insert "((eg \"\C-,\")")
> 
> in the *scratch* buffer.

Hm. I don't. The string just gets inserted:

  ((eg "C-,")

(as I'd expected).

It surely looks a bit funny (if you wanted that to be
valid lisp, there's one left paren too much), but a string
is a string is a string :-)

> So it doesn't seem to be related to `elisp--local-variables`.

What does happen if you toggle-debug-on-error before doing
that?

Cheers
 - t

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: elisp--local-variables triggers an error on special input

[Joost Kremers]

On Tue, Dec 01 2020, tomas@tuxteam.de wrote:
> On Tue, Dec 01, 2020 at 02:15:48PM +0100, Joost Kremers wrote:
>> You get that error by just eval'ing:
>> 
>>     (insert "((eg \"\C-,\")")
>> 
>> in the *scratch* buffer.
>
> Hm. I don't. The string just gets inserted:
>
>   ((eg "C-,")
>
> (as I'd expected).

What Emacs version are you using?

Mine is:

"GNU Emacs 27.1.50 (build 9, x86_64-pc-linux-gnu, GTK+ Version 3.22.30, cairo version 1.15.10)
 of 2020-11-14"

>> So it doesn't seem to be related to `elisp--local-variables`.
>
> What does happen if you toggle-debug-on-error before doing
> that?

Nothing very interesting:

```
Debugger entered--Lisp error: (error "Invalid modifier in string")
  read(#<buffer test.el>)
  elisp--preceding-sexp()
  elisp--eval-last-sexp(nil)
  eval-last-sexp(nil)
  funcall-interactively(eval-last-sexp nil)
  call-interactively(eval-last-sexp nil nil)
  command-execute(eval-last-sexp)
```

-- 
Joost Kremers
Life has its moments

Re: elisp--local-variables triggers an error on special input

[Gregory Heytings via Emacs development discussions.]

>> You get that error by just eval'ing:
>>
>>     (insert "((eg \"\C-,\")")
>>
>> in the *scratch* buffer.
>
> Hm. I don't. The string just gets inserted:
>
>  ((eg "C-,")
>
> (as I'd expected).
>

You should get an "Invalid modifier in string" error, at least that's what 
you get with emacs -Q.  The \C-, is not valid.  ASCII characters 0x00 to 
0x3E are not allowed after \C, with the exception of \0x20 (space).  That 
includes the comma, colon, semicolong, digits, ...

Re: elisp--local-variables triggers an error on special input

[Stefan Monnier]

> I've noticed that sometimes company raises an error in my *scratch*, I've
> shrinked the case and got:
> (with-current-buffer (get-buffer-create "pwp")
>   (erase-buffer)
>   (insert "((eg \"\C-,\")")
>   (elisp--local-variables))
> Evaluating this sexp raises "Invalid modifier in string" error
> Not sure this is intended, I think elisp--local-variables expects
> `invalid-read-syntax` error for this input, but got another error and does
> not handle it preventing error from happening

Clearly, `elisp--local-variables` should catch this error, but as you
point out, maybe the better way to do that is to change the error that
is signaled so that it is a child of `invalid-read-syntax`.


        Stefan

Re: elisp--local-variables triggers an error on special input

[tomas]

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

On Tue, Dec 01, 2020 at 03:36:47PM +0100, Joost Kremers wrote:
> 
> On Tue, Dec 01 2020, tomas@tuxteam.de wrote:
> > On Tue, Dec 01, 2020 at 02:15:48PM +0100, Joost Kremers wrote:
> >> You get that error by just eval'ing:
> >> 
> >>     (insert "((eg \"\C-,\")")
> >> 
> >> in the *scratch* buffer.
> >
> > Hm. I don't. The string just gets inserted:
> >
> >   ((eg "C-,")
> >
> > (as I'd expected).
> 
> What Emacs version are you using?
> 
> Mine is:
> 
> "GNU Emacs 27.1.50 (build 9, x86_64-pc-linux-gnu, GTK+ Version 3.22.30, cairo version 1.15.10)
>  of 2020-11-14"

Mine:

  "28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 2.24.32, cairo version 1.16.0)
 of 2020-11-30"

I win :-)

> >> So it doesn't seem to be related to `elisp--local-variables`.
> >
> > What does happen if you toggle-debug-on-error before doing
> > that?
> 
> Nothing very interesting:
> 
> ```
> Debugger entered--Lisp error: (error "Invalid modifier in string")
>   read(#<buffer test.el>)
>   elisp--preceding-sexp()
>   elisp--eval-last-sexp(nil)
>   eval-last-sexp(nil)
>   funcall-interactively(eval-last-sexp nil)
>   call-interactively(eval-last-sexp nil nil)
>   command-execute(eval-last-sexp)
> ```

Oh, wait! I forgot the backslash before the 'C'. Now I get the error.

Just evaluating "\C-," gets you the same error. The '\C-,' isn't valid
char escape syntax (AFAIK it only works for ASCII SPACE through SPACE+32).

Sorry for the confusion.

Cheers
 - t

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: elisp--local-variables triggers an error on special input

[Evgeny Zajcev]

[-- Attachment #1.1: Type: text/plain, Size: 902 bytes --]

вт, 1 дек. 2020 г. в 18:45, Stefan Monnier <monnier@iro.umontreal.ca>:

> > I've noticed that sometimes company raises an error in my *scratch*, I've
> > shrinked the case and got:
> > (with-current-buffer (get-buffer-create "pwp")
> >   (erase-buffer)
> >   (insert "((eg \"\C-,\")")
> >   (elisp--local-variables))
> > Evaluating this sexp raises "Invalid modifier in string" error
> > Not sure this is intended, I think elisp--local-variables expects
> > `invalid-read-syntax` error for this input, but got another error and
> does
> > not handle it preventing error from happening
>
> Clearly, `elisp--local-variables` should catch this error, but as you
> point out, maybe the better way to do that is to change the error that
> is signaled so that it is a child of `invalid-read-syntax`.
>
>
I've composed a patch that fixes this, could you please apply it.

-- 
lg

[-- Attachment #1.2: Type: text/html, Size: 1394 bytes --]

[-- Attachment #2: 0002-Make-Invalid-modifier-in-string-ordinary-invalid-rea.patch --]
[-- Type: text/x-patch, Size: 908 bytes --]

From 90647406fbf21d34c5ddd52303f0dc7685862d58 Mon Sep 17 00:00:00 2001
From: Zajcev Evgeny <zevlg@yandex.ru>
Date: Thu, 17 Dec 2020 11:27:20 +0300
Subject: [PATCH 2/2] Make "Invalid modifier in string" ordinary
 invalid-read-syntax error

* src/lread.ec (read1): Raise "Invalid modifier in string" error as
  `invalid-read-syntax'.  This fixes raise of unhandled error in
  `elisp--local-variables'
---
 src/lread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/lread.c b/src/lread.c
index a3d5fd7bb8..3ef874039a 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -3438,7 +3438,7 @@ read1 (Lisp_Object readcharfun, int *pch, bool first_in_list)
 
 		/* Any modifiers remaining are invalid.  */
 		if (modifiers)
-		  error ("Invalid modifier in string");
+		  invalid_syntax ("Invalid modifier in string");
 		p += CHAR_STRING (ch, (unsigned char *) p);
 	      }
 	    else
-- 
2.25.1

Re: elisp--local-variables triggers an error on special input

[Stefan Monnier]

> I've composed a patch that fixes this, could you please apply it.

Done.


        Stefan

Re: elisp--local-variables triggers an error on special input

[Evgeny Zajcev]

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

`elisp--local-variables'

чт, 17 дек. 2020 г. в 18:43, Stefan Monnier <monnier@iro.umontreal.ca>:

> > I've composed a patch that fixes this, could you please apply it.
>
> Done.
>
>
This patch fixes this particular problem with lisp syntax reader, however
`elisp--local-variables' remains *so* fragile, for example here I've got
next errors from it:

In these cases lisp syntax is ok, but `elisp--local-variables-1' fails on
it:

(with-current-buffer (get-buffer-create "bb")
  (erase-buffer)
  (insert "(let cc ")
  (elisp--local-variables))
==> Lisp error: (wrong-type-argument listp cc)

(with-current-buffer (get-buffer-create "bb")
  (erase-buffer)
  (insert "(let [a b]\n")
  (elisp--local-variables))
==> Lisp error: (wrong-type-argument listp [a b])

Should we fix all these cases one by one, or maybe just run
`elisp--local-variables` under `ignore-errors`, what do you think?

-- 
lg

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

Re: elisp--local-variables triggers an error on special input

[Stefan Monnier]

> In these cases lisp syntax is ok, but `elisp--local-variables-1' fails on
> it:
>
> (with-current-buffer (get-buffer-create "bb")
>   (erase-buffer)
>   (insert "(let cc ")
>   (elisp--local-variables))
> ==> Lisp error: (wrong-type-argument listp cc)
>
> (with-current-buffer (get-buffer-create "bb")
>   (erase-buffer)
>   (insert "(let [a b]\n")
>   (elisp--local-variables))
> ==> Lisp error: (wrong-type-argument listp [a b])
>
> Should we fix all these cases one by one,

I think we should try to, yes.

> or maybe just run `elisp--local-variables` under `ignore-errors`, what
> do you think?

I think it's best to let the caller ignore (or demote) errors if it wants to.


        Stefan