bug#24542: 25.1.50; The symbol `@' and sexp scanning

bug#24542: 25.1.50; The symbol `@' and sexp scanning

[Michael Heerdegen]

Hi,

the following breaks my "el-search" package when searching in
"skeleton.el".  It assumes that calling `scan-sexps' works at the start
of any sexp.

Recipe: in emacs -Q,

insert

    (eq element '@)

in an elisp mode buffer (e.g. scratch).  Put point at the quote or the
"@".  Eval

    (goto-char (scan-sexps (point) 1))

and you get an error like:


Debugger entered--Lisp error: (scan-error "Containing expression ends prematurely" 15 16)
  scan-sexps(13 1)
  (goto-char (scan-sexps (point) 1))
  eval((goto-char (scan-sexps (point) 1)) nil)
  eval-expression((goto-char (scan-sexps (point) 1)) nil)
  funcall-interactively(eval-expression (goto-char (scan-sexps (point) 1)) nil)
  call-interactively(eval-expression nil nil)
  command-execute(eval-expression)


I think `scan-sexps' should not error.  FWIW, if I add one more sexp to
the list after "'@" (different from "'@"), there is no error.


TIA,

Michael.




In GNU Emacs 25.1.50.2 (x86_64-pc-linux-gnu, GTK+ Version 3.21.5)
 of 2016-09-23 built on drachen
Repository revision: 14c36d76df035faa127580d706a0564f4e496991
Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
System Description:	Debian GNU/Linux testing (stretch)

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND DBUS GSETTINGS NOTIFY
LIBXML2 FREETYPE XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LC_ALL: de_DE.utf8
  value of $LC_COLLATE: C
  value of $LC_TIME: C
  value of $LANG: de_DE.utf8
  locale-coding-system: utf-8-unix

Major mode: Emacs-Lisp

bug#24542: 25.1.50; The symbol `@' and sexp scanning

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> insert
>
>     (eq element '@)
>
> in an elisp mode buffer (e.g. scratch).  Put point at the quote or the
> "@".  Eval
>
>     (goto-char (scan-sexps (point) 1))
>
> and you get an error like:
>
>
> Debugger entered--Lisp error: (scan-error "Containing expression ends
> prematurely" 15 16)

It seems that you can replace the symbol `@' in this recipe with any
symbol whose name has the form "@+", i.e. `@@' or `@@@' or...  When the
symbol contains only one character different from "@", it doesn't seem
to happen.


Michael.

bug#24542: 25.1.50; The symbol `@' and sexp scanning

[npostavs]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> insert
>>
>>     (eq element '@)
>>
>> in an elisp mode buffer (e.g. scratch).  Put point at the quote or the
>> "@".  Eval
>>
>>     (goto-char (scan-sexps (point) 1))
>>
>> and you get an error like:
>>
>>
>> Debugger entered--Lisp error: (scan-error "Containing expression ends
>> prematurely" 15 16)
>
> It seems that you can replace the symbol `@' in this recipe with any
> symbol whose name has the form "@+", i.e. `@@' or `@@@' or...  When the
> symbol contains only one character different from "@", it doesn't seem
> to happen.

`@' has the prefix syntax flag, the elisp manual says `(elisp) Syntax Flags':

   * `p' identifies an additional prefix character for Lisp syntax.
     These characters are treated as whitespace when they appear between
                          ^^^^^^^^^^^^^^^^^^^^^
     expressions.  When they appear within an expression, they are
     handled according to their usual syntax classes.

So (eq element '@) acts the same as (eq element ' ).  This could be
changed with something the following patch, but I'm not sure if it's the
right thing for non-Lisp languages...

diff --git i/src/syntax.c w/src/syntax.c
index 0ee1c74..253d3fb 100644
--- i/src/syntax.c
+++ w/src/syntax.c
@@ -2681,7 +2681,16 @@ scan_lists (EMACS_INT from, EMACS_INT count, EMACS_INT depth, bool sexpflag)
 	    }
 
 	  if (prefix)
-	    continue;
+            {
+              int next_c = FETCH_CHAR_AS_MULTIBYTE (from_byte);
+              int next_syntax = SYNTAX_WITH_FLAGS (next_c);
+              bool next_prefix = SYNTAX_FLAGS_PREFIX (next_syntax);
+              enum syntaxcode next_code = syntax_multibyte (next_c, multibyte_symbol_p);
+              if (next_prefix
+                  || next_code == Ssymbol
+                  || next_code == Sword)
+                continue;
+            }
 
 	  switch (code)
 	    {

bug#24542: 25.1.50; The symbol `@' and sexp scanning

[Michael Heerdegen]

npostavs@users.sourceforge.net writes:

> >> insert
> >>
> >>     (eq element '@)
> >>
> >> in an elisp mode buffer (e.g. scratch).  Put point at the quote or the
> >> "@".  Eval
> >>
> >>     (goto-char (scan-sexps (point) 1))
> >>
> >> and you get an error like:
> >>
> >>
> >> Debugger entered--Lisp error: (scan-error "Containing expression ends
> >> prematurely" 15 16)
> >
> > It seems that you can replace the symbol `@' in this recipe with any
> > symbol whose name has the form "@+", i.e. `@@' or `@@@' or...  When the
> > symbol contains only one character different from "@", it doesn't seem
> > to happen.
>
> `@' has the prefix syntax flag, the elisp manual says `(elisp) Syntax Flags':
>
>    * `p' identifies an additional prefix character for Lisp syntax.
>      These characters are treated as whitespace when they appear between
>                           ^^^^^^^^^^^^^^^^^^^^^
>      expressions.  When they appear within an expression, they are
>      handled according to their usual syntax classes.
>
> So (eq element '@) acts the same as (eq element ' ).

I don't agree with your interpretation.  `@' is an (textual
representation of an) expression in the above example, so "these
characters" don't "appear between expressions" in this case.  Could be
that this wording led to a wrong implementation, however.

> This could be changed with something the following patch, but I'm not
> sure if it's the right thing for non-Lisp languages...

> diff --git i/src/syntax.c w/src/syntax.c
> index 0ee1c74..253d3fb 100644
> --- i/src/syntax.c
> +++ w/src/syntax.c
> @@ -2681,7 +2681,16 @@ scan_lists (EMACS_INT from, EMACS_INT count, EMACS_INT depth, bool sexpflag)
>  	    }
>  
>  	  if (prefix)
> -	    continue;
> +            {
> +              int next_c = FETCH_CHAR_AS_MULTIBYTE (from_byte);
> +              int next_syntax = SYNTAX_WITH_FLAGS (next_c);
> +              bool next_prefix = SYNTAX_FLAGS_PREFIX (next_syntax);
> +              enum syntaxcode next_code = syntax_multibyte (next_c, multibyte_symbol_p);
> +              if (next_prefix
> +                  || next_code == Ssymbol
> +                  || next_code == Sword)
> +                continue;
> +            }
>  
>  	  switch (code)
>  	    {

Thanks for the patch.  Can anyone judge whether we can do this?


Thanks,

Michael. 

bug#24542: 25.1.50; The symbol `@' and sexp scanning

[npostavs]

Michael Heerdegen <michael_heerdegen@web.de> writes:
>> `@' has the prefix syntax flag, the elisp manual says `(elisp) Syntax Flags':
>>
>>    * `p' identifies an additional prefix character for Lisp syntax.
>>      These characters are treated as whitespace when they appear between
>>                           ^^^^^^^^^^^^^^^^^^^^^
>>      expressions.  When they appear within an expression, they are
>>      handled according to their usual syntax classes.
>>
>> So (eq element '@) acts the same as (eq element ' ).
>
> I don't agree with your interpretation.  `@' is an (textual
> representation of an) expression in the above example, so "these
> characters" don't "appear between expressions" in this case.  Could be
> that this wording led to a wrong implementation, however.

Well, it's a bit ambiguous whether a character sequence composed soley
of prefix characters should be considered an expression or not.
Obviously the end result is wrong for Lisp, i.e., does not correspond
with what `read' thinks.

bug#24542: 25.1.50; The symbol `@' and sexp scanning

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Thanks for the patch.  Can anyone judge whether we can do this?

I tried the patch, and it fixes the

(setq a '@)

problem for me.  I don't really have a great overview of where
scan_lists is used for -- it seems like a very general solution to a
very specific problem.

Does anybody have any comments here?

diff --git a/src/syntax.c b/src/syntax.c
index 7f0fc341f6..10912dd5f2 100644
--- a/src/syntax.c
+++ b/src/syntax.c
@@ -2693,7 +2693,17 @@ scan_lists (EMACS_INT from0, EMACS_INT count, EMACS_INT depth, bool sexpflag)
 	    }
 
 	  if (prefix)
-	    continue;
+            {
+              int next_c = FETCH_CHAR_AS_MULTIBYTE (from_byte);
+              int next_syntax = SYNTAX_WITH_FLAGS (next_c);
+              bool next_prefix = SYNTAX_FLAGS_PREFIX (next_syntax);
+              enum syntaxcode next_code =
+		syntax_multibyte (next_c, multibyte_symbol_p);
+              if (next_prefix
+                  || next_code == Ssymbol
+                  || next_code == Sword)
+                continue;
+            }
 
 	  switch (code)
 	    {


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#24542: 25.1.50; The symbol `@' and sexp scanning

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Fri, 04 Sep 2020 15:05:36 +0200
> Cc: 24542@debbugs.gnu.org, npostavs@users.sourceforge.net
> 
> I tried the patch, and it fixes the
> 
> (setq a '@)
> 
> problem for me.  I don't really have a great overview of where
> scan_lists is used for -- it seems like a very general solution to a
> very specific problem.
> 
> Does anybody have any comments here?

Stefan, any comments?

bug#24542: 25.1.50; The symbol `@' and sexp scanning

[Stefan Monnier]

>> Does anybody have any comments here?
> Stefan, any comments?

Checking for an immediately following word or symbol syntax doesn't see
sufficient if we consider cases like:

   '(sgasfg)

or

   , (sfgsdf)

or

   ,@ ;; entertained yet?
   \)

and then we have the really fun stuff like:

   (foo '@ bar)

vs

   (foo ,@ bar)

So maybe a better solution is a syntax-propertize rule which
distinguishes "@ after comma" from "@ not after comma"?


        Stefan

bug#24542: 25.1.50; The symbol `@' and sexp scanning

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

>     (eq element '@)
>
> in an elisp mode buffer (e.g. scratch).  Put point at the quote or the
> "@".  Eval
>     (goto-char (scan-sexps (point) 1))
> and you get an error like:
> Debugger entered--Lisp error: (scan-error "Containing expression ends prematurely" 15 16)

The patch below seems to fix it.
Any comments/objections?


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: bug24542.diff --]
[-- Type: text/x-diff, Size: 1338 bytes --]

diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 9b4c3f994cd..10d7f01ebaf 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -40,9 +40,8 @@ 'emacs-lisp-mode-abbrev-table
 
 (defvar emacs-lisp-mode-syntax-table
   (let ((table (make-syntax-table lisp-data-mode-syntax-table)))
-    ;; These are redundant, now.
-    ;;(modify-syntax-entry ?\[ "(]  " table)
-    ;;(modify-syntax-entry ?\] ")[  " table)
+    ;; `syntax-propertize'. takes care of `,@'.
+    (modify-syntax-entry ?@ "_" table)
     table)
   "Syntax table used in `emacs-lisp-mode'.")
 
diff --git a/test/lisp/progmodes/elisp-mode-tests.el b/test/lisp/progmodes/elisp-mode-tests.el
index 1d1ef9981e5..3ef5a28e460 100644
--- a/test/lisp/progmodes/elisp-mode-tests.el
+++ b/test/lisp/progmodes/elisp-mode-tests.el
@@ -1131,5 +1131,14 @@ test-indentation
                         (emacs-lisp-mode)
                         (indent-region (point-min) (point-max)))))
 
+(ert-deftest elisp-tests-syntax-propertize ()
+  (with-temp-buffer
+    (emacs-lisp-mode)
+    (insert "(a '@)")
+    (should (equal (scan-sexps (+ (point-min) 3) 1) (1- (point-max))))
+    (erase-buffer)
+    (insert "(a ,@)")
+    (should-error (scan-sexps (+ (point-min) 3) 1))))
+
 (provide 'elisp-mode-tests)
 ;;; elisp-mode-tests.el ends here

bug#24542: 25.1.50; The symbol `@' and sexp scanning

[Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>  (defvar emacs-lisp-mode-syntax-table
>    (let ((table (make-syntax-table lisp-data-mode-syntax-table)))
> -    ;; These are redundant, now.
> -    ;;(modify-syntax-entry ?\[ "(]  " table)
> -    ;;(modify-syntax-entry ?\] ")[  " table)
> +    ;; `syntax-propertize'. takes care of `,@'.
> +    (modify-syntax-entry ?@ "_" table)
>      table)
>    "Syntax table used in `emacs-lisp-mode'.")

This seems to give good results for me - so far at least.  Also Isearch
symbol searching doesn't seem to be affected in any negative way (I
think there is lost "." in your comment, however).

Thanks,

Michael.

bug#24542: 25.1.50; The symbol `@' and sexp scanning

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>  (defvar emacs-lisp-mode-syntax-table
>>    (let ((table (make-syntax-table lisp-data-mode-syntax-table)))
>> -    ;; These are redundant, now.
>> -    ;;(modify-syntax-entry ?\[ "(]  " table)
>> -    ;;(modify-syntax-entry ?\] ")[  " table)
>> +    ;; `syntax-propertize'. takes care of `,@'.
>> +    (modify-syntax-entry ?@ "_" table)
>>      table)
>>    "Syntax table used in `emacs-lisp-mode'.")
>
> This seems to give good results for me - so far at least.

Thanks, pushed to `master`, closing.


        Stefan

bug#24542: 25.1.50; The symbol `@' and sexp scanning

[Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> Thanks, pushed to `master`, closing.

Thank you.

[ Is the omission of the test your original patch contained
intentional? ]


Michael.

bug#24542: 25.1.50; The symbol `@' and sexp scanning

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> [ Is the omission of the test your original patch contained
> intentional? ]

No, indeed, thanks,


        Stefan