GNU bug report logs - #72561
31.0.50; Scan error in ert--pp-with-indentation-and-newline

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 72561 in the body.
You can then email your comments to 72561 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: bug-gnu-emacs <at> gnu.org
Subject: 31.0.50; Scan error in ert--pp-with-indentation-and-newline
Date: Sat, 10 Aug 2024 06:48:11 -0700

[Message part 1 (text/plain, inline)]

Tags: patch

This apparently began after

  2f181d60323bd9e0196775828de633100479f4c2
  Author:     Stefan Monnier <monnier <at> iro.umontreal.ca>
  AuthorDate: Fri Jun 16 13:35:06 2023 -0400
  CommitDate: Sat Jun 17 17:24:38 2023 -0400

  pp.el (pp-fill): New default pp function
  
  1 file changed, 90 insertions(+), 1 deletion(-)
  lisp/emacs-lisp/pp.el | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++-

So I guess it also affects Emacs 30. To reproduce, put this in
test/lisp/emacs-lisp/ert-tests.el:

   (ert-deftest ert--pp-with-indentation-and-newline ()
     (should (equal '((:one "1" :three "3" :two "2")) '((:one "1")))))

Then run something like:

   $ make -C test \
     SELECTOR=ert--pp-with-indentation-and-newline \
     lisp/emacs-lisp/ert-tests.log

   Error: scan-error ("Containing expression ends prematurely" 221 221)
     forward-sexp-default-function(-1)
     forward-sexp(-1)
     calculate-lisp-indent((5 221 238 nil nil nil 4 nil nil (5 27 161 175
     lisp-indent-calc-next(#s(lisp-indent-state :stack (50 16 6 5 nil) :p
     indent-sexp()
     ert--pp-with-indentation-and-newline((ert-test-failed ((should (equa
     #f(compiled-function (event-type &rest event-args) #<bytecode 0x16b6
     #f(compiled-function () #<bytecode -0xb4ce749a56118cd>)()
     ert-run-or-rerun-test(#s(ert--stats :selector ert--pp-with-indentati
   
   Aborted: Ran 1 tests, 0 results as expected, 1 unexpected, -1 skipped
   
No idea if there's a deeper issue at play here, maybe something in
pp.el that a patch like the attached would just be papering over.


In GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.43, cairo version 1.18.0) of 2024-08-09 built on localhost
Repository revision: 944e45db53cb173c5eadd4794081c133e8649d67
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12401002
System Description: Fedora Linux 40 (Workstation Edition)

Configured using:
 'configure --enable-check-lisp-object-type --enable-checking=yes,glyphs
 'CFLAGS=-O0 -g3'
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP
NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr compile comint ansi-osc ansi-color ring comp-run
bytecomp byte-compile comp-common rx emacsbug message mailcap yank-media
puny dired dired-loaddefs rfc822 mml mml-sec password-cache epa derived
epg rfc6068 epg-config gnus-util text-property-search time-date subr-x
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils rmc iso-transl tooltip cconv eldoc paren
electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd touch-screen tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo gtk
x-toolkit xinput2 x multi-tty move-toolbar make-network-process
native-compile emacs)

Memory information:
((conses 16 58857 9185) (symbols 48 6741 0) (strings 32 16558 3964)
 (string-bytes 1 477530) (vectors 16 11257)
 (vector-slots 8 135809 8706) (floats 8 21 4) (intervals 56 249 0)
 (buffers 984 11))

[0001-Indent-failed-ERT-should-forms-with-Emacs-Lisp-synta.patch (text/x-patch, attachment)]

Message #8 received at 72561 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "J.P." <jp <at> neverwas.me>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 72561 <at> debbugs.gnu.org
Subject: Re: bug#72561: 31.0.50;
 Scan error in ert--pp-with-indentation-and-newline
Date: Thu, 15 Aug 2024 11:33:32 +0300

> From: "J.P." <jp <at> neverwas.me>
> Date: Sat, 10 Aug 2024 06:48:11 -0700
> 
> Tags: patch
> 
> This apparently began after
> 
>   2f181d60323bd9e0196775828de633100479f4c2
>   Author:     Stefan Monnier <monnier <at> iro.umontreal.ca>
>   AuthorDate: Fri Jun 16 13:35:06 2023 -0400
>   CommitDate: Sat Jun 17 17:24:38 2023 -0400
> 
>   pp.el (pp-fill): New default pp function
>   
>   1 file changed, 90 insertions(+), 1 deletion(-)
>   lisp/emacs-lisp/pp.el | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> 
> So I guess it also affects Emacs 30. To reproduce, put this in
> test/lisp/emacs-lisp/ert-tests.el:
> 
>    (ert-deftest ert--pp-with-indentation-and-newline ()
>      (should (equal '((:one "1" :three "3" :two "2")) '((:one "1")))))
> 
> Then run something like:
> 
>    $ make -C test \
>      SELECTOR=ert--pp-with-indentation-and-newline \
>      lisp/emacs-lisp/ert-tests.log
> 
>    Error: scan-error ("Containing expression ends prematurely" 221 221)
>      forward-sexp-default-function(-1)
>      forward-sexp(-1)
>      calculate-lisp-indent((5 221 238 nil nil nil 4 nil nil (5 27 161 175
>      lisp-indent-calc-next(#s(lisp-indent-state :stack (50 16 6 5 nil) :p
>      indent-sexp()
>      ert--pp-with-indentation-and-newline((ert-test-failed ((should (equa
>      #f(compiled-function (event-type &rest event-args) #<bytecode 0x16b6
>      #f(compiled-function () #<bytecode -0xb4ce749a56118cd>)()
>      ert-run-or-rerun-test(#s(ert--stats :selector ert--pp-with-indentati
>    
>    Aborted: Ran 1 tests, 0 results as expected, 1 unexpected, -1 skipped
>    
> No idea if there's a deeper issue at play here, maybe something in
> pp.el that a patch like the attached would just be papering over.

Adding Stefan.

Message #11 received at 72561 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: "J.P." <jp <at> neverwas.me>
Cc: 72561 <at> debbugs.gnu.org
Subject: Re: bug#72561: 31.0.50; Scan error in
 ert--pp-with-indentation-and-newline
Date: Wed, 21 Aug 2024 17:41:38 -0400

>    Error: scan-error ("Containing expression ends prematurely" 221 221)
>      forward-sexp-default-function(-1)
>      forward-sexp(-1)
>      calculate-lisp-indent((5 221 238 nil nil nil 4 nil nil (5 27 161 175
>      lisp-indent-calc-next(#s(lisp-indent-state :stack (50 16 6 5 nil) :p
>      indent-sexp()
>      ert--pp-with-indentation-and-newline((ert-test-failed ((should (equa

Hmm...

> --- a/lisp/emacs-lisp/ert.el
> +++ b/lisp/emacs-lisp/ert.el
> @@ -1323,7 +1323,8 @@ ert--pp-with-indentation-and-newline
>      (unless (bolp) (insert "\n"))
>      (save-excursion
>        (goto-char begin)
> -      (indent-sexp))))
> +      (with-syntax-table emacs-lisp-mode-syntax-table
> +        (indent-sexp)))))

Your patch makes sense: indeed, looking at the code of `indent-sexp`,
I see that it uses `lisp-indent*` functions in a way which presumes that
we're looking at Lisp code and would require a list-mode syntax-table.
I wonder why this has not bitten us earlier in other circumstances.

But I also wonder why `ert--pp-with-indentation-and-newline` calls
`indent-sexp`, since `pp` should have done that for us already, so I'd
be tempted to just remove that call.  Or maybe the purpose is to "shift"
the text when `begin` is not in column 0?
If so, maybe `indent-rigidly` is a better way to get the same result?


        Stefan

Message #14 received at 72561 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 72561 <at> debbugs.gnu.org
Subject: Re: bug#72561: 31.0.50; Scan error in
 ert--pp-with-indentation-and-newline
Date: Wed, 21 Aug 2024 17:28:45 -0700

[Message part 1 (text/plain, inline)]

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

> Your patch makes sense: indeed, looking at the code of `indent-sexp`,
> I see that it uses `lisp-indent*` functions in a way which presumes that
> we're looking at Lisp code and would require a list-mode syntax-table.
> I wonder why this has not bitten us earlier in other circumstances.
>
> But I also wonder why `ert--pp-with-indentation-and-newline` calls
> `indent-sexp`, since `pp` should have done that for us already, so I'd
> be tempted to just remove that call.  Or maybe the purpose is to "shift"
> the text when `begin` is not in column 0?
> If so, maybe `indent-rigidly` is a better way to get the same result?

Right, I don't think `begin' is ever in column 0 (as currently used). So
I guess the intention is indeed to shift all but the first line of the
`pp' result by `current-column', meaning it's probably cleaner (as you
say) to do a dumb, uniform shift.

[0001-Indent-ERT-failure-explanations-rigidly.patch (text/x-patch, attachment)]

Message #17 received at 72561 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 72561 <at> debbugs.gnu.org
Subject: Re: bug#72561: 31.0.50; Scan error in
 ert--pp-with-indentation-and-newline
Date: Wed, 21 Aug 2024 18:21:11 -0700

[Message part 1 (text/plain, inline)]

"J.P." <jp <at> neverwas.me> writes:

Actually, that can probably be simplified a bit.

[0001-Indent-ERT-failure-explanations-rigidly.patch (text/x-patch, attachment)]

Message #20 received at 72561 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: "J.P." <jp <at> neverwas.me>
Cc: 72561 <at> debbugs.gnu.org
Subject: Re: bug#72561: 31.0.50; Scan error in
 ert--pp-with-indentation-and-newline
Date: Thu, 22 Aug 2024 09:43:14 -0400

> Actually, that can probably be simplified a bit.

BTW, it could be argued that the `indent-rigidly` should take place in
`pp` (i.e. consider it as a bug in `pp`).

The patch looks good to me, tho I have the following nitpicks:

>    (let ((begin (point))
> +        (cols (- (point) (line-beginning-position)))
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                 (current-column)

>          (pp-escape-newlines t)
>          (print-escape-control-characters t))
>      (pp object (current-buffer))
>      (unless (bolp) (insert "\n"))
> -    (save-excursion
> -      (goto-char begin)
> -      (indent-sexp))))
> +    (indent-rigidly begin (point-max) cols)))
>                            ^^^^^^^^^^^
                             (point)

We arguably know that (point) is the same as (point-max) here, so it's
not really important, but (point) is shorter and conceptually more
correct since we wouldn't want to shift text that was there before.


        Stefan

Message #23 received at 72561 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 72561 <at> debbugs.gnu.org
Subject: Re: bug#72561: 31.0.50; Scan error in
 ert--pp-with-indentation-and-newline
Date: Thu, 22 Aug 2024 18:02:20 -0700

[Message part 1 (text/plain, inline)]

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

>> Actually, that can probably be simplified a bit.
>
> BTW, it could be argued that the `indent-rigidly` should take place in
> `pp` (i.e. consider it as a bug in `pp`).
>
> The patch looks good to me, tho I have the following nitpicks:
>
>>    (let ((begin (point))
>> +        (cols (- (point) (line-beginning-position)))
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>                  (current-column)

Oh, TABs and stuff, right.

>
>>          (pp-escape-newlines t)
>>          (print-escape-control-characters t))
>>      (pp object (current-buffer))
>>      (unless (bolp) (insert "\n"))
>> -    (save-excursion
>> -      (goto-char begin)
>> -      (indent-sexp))))
>> +    (indent-rigidly begin (point-max) cols)))
>>                            ^^^^^^^^^^^
>                              (point)
>
> We arguably know that (point) is the same as (point-max) here, so it's
> not really important, but (point) is shorter and conceptually more
> correct since we wouldn't want to shift text that was there before.

Makes sense. Might as well have the function be more generally useful.
Thanks.

And since the issue is new on the release branch, I'm guessing that's
where a patch should go (Cc. Eli)?

[0001-Indent-ERT-failure-explanations-rigidly.patch (text/x-patch, attachment)]

Message #26 received at 72561 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "J.P." <jp <at> neverwas.me>
Cc: 72561 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#72561: 31.0.50; Scan error in
 ert--pp-with-indentation-and-newline
Date: Fri, 23 Aug 2024 09:31:24 +0300

> From: "J.P." <jp <at> neverwas.me>
> Cc: 72561 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
> Date: Thu, 22 Aug 2024 18:02:20 -0700
> 
> And since the issue is new on the release branch, I'm guessing that's
> where a patch should go (Cc. Eli)?

If you run the entire test suite, including the expensive tests, and
this doesn't change the results, then yes, please install on the
release branch.

Thanks.

Message #31 received at 72561-done <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: 72561-done <at> debbugs.gnu.org
Cc: Eli Zaretskii <eliz <at> gnu.org>, monnier <at> iro.umontreal.ca
Subject: Re: bug#72561: 31.0.50; Scan error in
 ert--pp-with-indentation-and-newline
Date: Wed, 28 Aug 2024 20:48:49 -0700

Eli Zaretskii <eliz <at> gnu.org> writes:

> If you run the entire test suite, including the expensive tests, and
> this doesn't change the results, then yes, please install on the
> release branch.

Results for check-expensive were stable, running locally in EMBA's
inotify and native-comp containers. The changes were therefore installed
on the release branch. Thanks and closing.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.