GNU bug report logs - #68546
29.1.90; end-of-file has incorrect data when signaled within a load

Previous Next

Package: emacs;

Reported by: Spencer Baugh <sbaugh <at> janestreet.com>

Date: Wed, 17 Jan 2024 19:05:02 UTC

Severity: normal

Found in version 29.1.90

Done: Sean Whitton <spwhitton <at> spwhitton.name>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Wed, 17 Jan 2024 19:05:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Spencer Baugh <sbaugh <at> janestreet.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Wed, 17 Jan 2024 19:05:02 GMT) Full text and rfc822 format available.

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: bug-gnu-emacs <at> gnu.org
Cc: dmitry <at> gutov.dev
Subject: 29.1.90; end-of-file has incorrect data when signaled within a load
Date: Wed, 17 Jan 2024 14:04:09 -0500
The end-of-file error signaled by read has incorrect data if it's
signaled during a load, by a read unrelated to that load.

1. Create a file "~/read-empty.el" containing:
(read "")
2. (load "~/read-empty.el")
3. Observe the error message:
End of file during parsing: /home/sbaugh/read-empty.el

This error message suggests that there's a syntax error in
read-empty.el, when in fact the syntax error is in something else
entirely.  This is quite confusing.

This happens because end_of_file_error uses load-true-file-name if it's
non-nil, even if the actual read call is unrelated.

One possible fix: a new variable read-file-name could be introduced, and
end_of_file_error could use that instead of load-true-file-name, and
load can just bind read-file-name around read.

As one particular example of the confusing current behavior, a user had
corrupted their ~/.emacs.d/projects so that reading it failed.  Also,
they had a call to (project-forget-zombie-projects) in their init.el.
In combination, this meant Emacs startup errored with:

End of file during parsing: /home/user/.emacs.d/init.el

even though there was no syntax error in init.el at all.

To resolve that, perhaps project--read-project-list could bind
read-file-name to project-list-file, so we'd get an error message which
properly mentions the project file instead.


In GNU Emacs 29.1.90 (build 8, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2024-01-04 built on
 igm-qws-u22796a
Repository revision: 57fa5a53f74e489702825045832f52730c5d550f
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Rocky Linux 8.9 (Green Obsidian)

Configured using:
 'configure 'CFLAGS=-O0 -g3' --with-gif=ifavailable
 --with-x-toolkit=lucid'

Configured features:
CAIRO DBUS FREETYPE GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG
SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM
XINPUT2 XPM LUCID ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  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
  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 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 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
dynamic-setting system-font-setting font-render-setting cairo x-toolkit
xinput2 x multi-tty make-network-process emacs)

Memory information:
((conses 16 63231 9564)
 (symbols 48 9475 0)
 (strings 32 22767 1134)
 (string-bytes 1 677438)
 (vectors 16 9316)
 (vector-slots 8 148900 13842)
 (floats 8 34 25)
 (intervals 56 241 0)
 (buffers 976 10))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Wed, 17 Jan 2024 20:56:02 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>, 68546 <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when signaled
 within a load
Date: Wed, 17 Jan 2024 22:55:41 +0200
On 17/01/2024 21:04, Spencer Baugh wrote:
> As one particular example of the confusing current behavior, a user had
> corrupted their ~/.emacs.d/projects so that reading it failed.  Also,
> they had a call to (project-forget-zombie-projects) in their init.el.
> In combination, this meant Emacs startup errored with:
> 
> End of file during parsing:/home/user/.emacs.d/init.el
> 
> even though there was no syntax error in init.el at all.

Would something like this help with this particular sub-problem?

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index a6f14a0865c..196a82757b2 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1694,7 +1694,9 @@ project--read-project-list
                  (let ((name (car elem)))
                    (list (if (file-remote-p name) name
                            (abbreviate-file-name name)))))
-               (read (current-buffer))))))
+               (condition-case nil
+                   (read (current-buffer))
+                 (end-of-file (warn "Failed to read the projects list 
file")))))))
     (unless (seq-every-p
              (lambda (elt) (stringp (car-safe elt)))
              project--list)





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Thu, 25 Jan 2024 14:06:01 GMT) Full text and rfc822 format available.

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 68546 <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when signaled
 within a load
Date: Thu, 25 Jan 2024 09:05:19 -0500
[Message part 1 (text/plain, inline)]
On Wed, Jan 17, 2024 at 3:55 PM Dmitry Gutov <dmitry <at> gutov.dev> wrote:

> On 17/01/2024 21:04, Spencer Baugh wrote:
> > As one particular example of the confusing current behavior, a user had
> > corrupted their ~/.emacs.d/projects so that reading it failed.  Also,
> > they had a call to (project-forget-zombie-projects) in their init.el.
> > In combination, this meant Emacs startup errored with:
> >
> > End of file during parsing:/home/user/.emacs.d/init.el
> >
> > even though there was no syntax error in init.el at all.
>
> Would something like this help with this particular sub-problem?
>
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index a6f14a0865c..196a82757b2 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -1694,7 +1694,9 @@ project--read-project-list
>                    (let ((name (car elem)))
>                      (list (if (file-remote-p name) name
>                              (abbreviate-file-name name)))))
> -               (read (current-buffer))))))
> +               (condition-case nil
> +                   (read (current-buffer))
> +                 (end-of-file (warn "Failed to read the projects list
> file")))))))
>       (unless (seq-every-p
>                (lambda (elt) (stringp (car-safe elt)))
>                project--list)
>
>
Yes, I think that would be great.  Especially because this allows startup
to continue.  If it's okay with you too, I think it's reasonable to push.

This de-facto resets the contents of the projects list file (since the
corrupted version will get saved over), but I think that's fine -  it's not
especially hard information to rebuild, and if it's corrupt anyway then
it's probably already at least partially lost (in my case, a user had an
empty projects file for some reason, not sure why).  Oh and I guess we
already reset the contents if it's in the wrong format.  So yeah, this
seems great.
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Thu, 25 Jan 2024 14:14:02 GMT) Full text and rfc822 format available.

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 68546 <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when signaled
 within a load
Date: Thu, 25 Jan 2024 09:12:50 -0500
[Message part 1 (text/plain, inline)]
On Thu, Jan 25, 2024 at 9:05 AM Spencer Baugh <sbaugh <at> janestreet.com> wrote:

> On Wed, Jan 17, 2024 at 3:55 PM Dmitry Gutov <dmitry <at> gutov.dev> wrote:
>
>> On 17/01/2024 21:04, Spencer Baugh wrote:
>> > As one particular example of the confusing current behavior, a user had
>> > corrupted their ~/.emacs.d/projects so that reading it failed.  Also,
>> > they had a call to (project-forget-zombie-projects) in their init.el.
>> > In combination, this meant Emacs startup errored with:
>> >
>> > End of file during parsing:/home/user/.emacs.d/init.el
>> >
>> > even though there was no syntax error in init.el at all.
>>
>> Would something like this help with this particular sub-problem?
>>
>> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>> index a6f14a0865c..196a82757b2 100644
>> --- a/lisp/progmodes/project.el
>> +++ b/lisp/progmodes/project.el
>> @@ -1694,7 +1694,9 @@ project--read-project-list
>>                    (let ((name (car elem)))
>>                      (list (if (file-remote-p name) name
>>                              (abbreviate-file-name name)))))
>> -               (read (current-buffer))))))
>> +               (condition-case nil
>> +                   (read (current-buffer))
>> +                 (end-of-file (warn "Failed to read the projects list
>> file")))))))
>>       (unless (seq-every-p
>>                (lambda (elt) (stringp (car-safe elt)))
>>                project--list)
>>
>>
> Yes, I think that would be great.  Especially because this allows startup
> to continue.  If it's okay with you too, I think it's reasonable to push.
>
> This de-facto resets the contents of the projects list file (since the
> corrupted version will get saved over), but I think that's fine -  it's not
> especially hard information to rebuild, and if it's corrupt anyway then
> it's probably already at least partially lost (in my case, a user had an
> empty projects file for some reason, not sure why).  Oh and I guess we
> already reset the contents if it's in the wrong format.  So yeah, this
> seems great.
>

Ah, I think the cause (in at least one case) is that the user ran out of
disk space, and so the write-region call in project--write-project-list
failed when writing, which happens after truncating the existing file.  I
guess there should be some atomic version of write-region which writes to a
temp file and renames it over the original.
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Thu, 25 Jan 2024 14:30:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: dmitry <at> gutov.dev, 68546 <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90;
 end-of-file has incorrect data when signaled within a load
Date: Thu, 25 Jan 2024 16:29:29 +0200
> Cc: 68546 <at> debbugs.gnu.org
> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Date: Thu, 25 Jan 2024 09:05:19 -0500
> 
>  Would something like this help with this particular sub-problem?
> 
>  diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>  index a6f14a0865c..196a82757b2 100644
>  --- a/lisp/progmodes/project.el
>  +++ b/lisp/progmodes/project.el
>  @@ -1694,7 +1694,9 @@ project--read-project-list
>                     (let ((name (car elem)))
>                       (list (if (file-remote-p name) name
>                               (abbreviate-file-name name)))))
>  -               (read (current-buffer))))))
>  +               (condition-case nil
>  +                   (read (current-buffer))
>  +                 (end-of-file (warn "Failed to read the projects list 
>  file")))))))
>        (unless (seq-every-p
>                 (lambda (elt) (stringp (car-safe elt)))
>                 project--list)
> 
> Yes, I think that would be great.  Especially because this allows startup to continue.  If it's okay with
> you too, I think it's reasonable to push.

I don't object to the change, of course, but perhaps we could make the
error message friendlier?  For example:

   Failed to read the projects list file due to unexpected EOF




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Fri, 26 Jan 2024 00:46:02 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>, Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 68546 <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when signaled
 within a load
Date: Fri, 26 Jan 2024 02:45:28 +0200
On 25/01/2024 16:29, Eli Zaretskii wrote:
>> Cc:68546 <at> debbugs.gnu.org
>> From: Spencer Baugh<sbaugh <at> janestreet.com>
>> Date: Thu, 25 Jan 2024 09:05:19 -0500
>>
>>   Would something like this help with this particular sub-problem?
>>
>>   diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>>   index a6f14a0865c..196a82757b2 100644
>>   --- a/lisp/progmodes/project.el
>>   +++ b/lisp/progmodes/project.el
>>   @@ -1694,7 +1694,9 @@ project--read-project-list
>>                      (let ((name (car elem)))
>>                        (list (if (file-remote-p name) name
>>                                (abbreviate-file-name name)))))
>>   -               (read (current-buffer))))))
>>   +               (condition-case nil
>>   +                   (read (current-buffer))
>>   +                 (end-of-file (warn "Failed to read the projects list
>>   file")))))))
>>         (unless (seq-every-p
>>                  (lambda (elt) (stringp (car-safe elt)))
>>                  project--list)
>>
>> Yes, I think that would be great.  Especially because this allows startup to continue.  If it's okay with
>> you too, I think it's reasonable to push.
> I don't object to the change, of course, but perhaps we could make the
> error message friendlier?  For example:
> 
>     Failed to read the projects list file due to unexpected EOF

Sure, the patch's message was more of a draft.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Fri, 26 Jan 2024 00:55:01 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 68546 <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when signaled
 within a load
Date: Fri, 26 Jan 2024 02:54:25 +0200
On 25/01/2024 16:05, Spencer Baugh wrote:
> Yes, I think that would be great.  Especially because this allows 
> startup to continue.  If it's okay with you too, I think it's reasonable 
> to push.

Now pushed, thanks.

I'm not closing the bug, it's up to you to decide whether there's more 
to be done.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Fri, 26 Jan 2024 00:57:01 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 68546 <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when signaled
 within a load
Date: Fri, 26 Jan 2024 02:55:52 +0200
On 25/01/2024 16:12, Spencer Baugh wrote:
> Ah, I think the cause (in at least one case) is that the user ran out of 
> disk space, and so the write-region call in project--write-project-list 
> failed when writing, which happens after truncating the existing file.

Makes sense.

> I 
> guess there should be some atomic version of write-region which writes 
> to a temp file and renames it over the original.

Yep, that would be an improvement.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Fri, 16 Feb 2024 21:57:02 GMT) Full text and rfc822 format available.

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: 68546 <at> debbugs.gnu.org
Cc: dmitry <at> gutov.dev
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when
 signaled within a load
Date: Fri, 16 Feb 2024 16:56:26 -0500
[Message part 1 (text/plain, inline)]
Here is a straightforward fix.

[0001-Prevent-incorrect-error-message-when-calling-read-in.patch (text/x-patch, inline)]
From d850108fa309701e4899dfbcfd5a20d1e17f86af Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Fri, 16 Feb 2024 16:53:28 -0500
Subject: [PATCH] Prevent incorrect error message when calling read inside load

Previously, if `load' eval'd a `read' expression which raised
end-of-file, the error would include load-true-file-name, even
though the `read' may be reading something completely different.

Now, end-of-file errors raised by `read' will only include
load-true-file-name if it's actually reading that file.

We do this by having read include read-end-of-file-name in the error
instead of load-true-file-name, and only binding read-end-of-file-name
around the "read" parts of readevalloop, not the "eval" parts.
(load-true-file-name is still bound throughout)

Also, when reading a file (or some other source), it is now
possible to bind read-end-of-file-name so that end-of-file
errors raised by read will include the filename (or the string
of your choice).  Previously, an end-of-file error raised by
read outside of load would never include the filename.

* src/lread.c (syms_of_lread): Add read-end-of-file-name.
(readevalloop): Bind read-end-of-file-name to
load-true-file-name around read.
(end_of_file_error): Use read-end-of-file-name instead of
load-true-file-name.  (bug#68546)
---
 src/lread.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/lread.c b/src/lread.c
index 0e67a3f8879..54ae88c7eab 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -2385,8 +2385,8 @@ readevalloop_1 (int old)
 static AVOID
 end_of_file_error (void)
 {
-  if (STRINGP (Vload_true_file_name))
-    xsignal1 (Qend_of_file, Vload_true_file_name);
+  if (!NILP (Vread_end_of_file_name))
+    xsignal1 (Qend_of_file, Vread_end_of_file_name);
 
   xsignal0 (Qend_of_file);
 }
@@ -2490,6 +2490,8 @@ readevalloop (Lisp_Object readcharfun,
   while (continue_reading_p)
     {
       specpdl_ref count1 = SPECPDL_INDEX ();
+      if (NILP (Vread_end_of_file_name))
+	specbind (Qread_end_of_file_name, Vload_true_file_name);
 
       if (b != 0 && !BUFFER_LIVE_P (b))
 	error ("Reading from killed buffer");
@@ -2585,7 +2587,7 @@ readevalloop (Lisp_Object readcharfun,
       if (!NILP (start) && continue_reading_p)
 	start = Fpoint_marker ();
 
-      /* Restore saved point and BEGV.  */
+      /* Restore saved point and BEGV, and unbind read_stream_for_error.  */
       unbind_to (count1, Qnil);
 
       /* Now eval what we just read.  */
@@ -5843,6 +5845,12 @@ syms_of_lread (void)
 	       doc: /* Full name of file being loaded by `load'.  */);
   Vload_true_file_name = Qnil;
 
+  DEFVAR_LISP ("read-end-of-file-name", Vread_end_of_file_name,
+	       doc: /* String to be included when `read' signals `end-of-file'.
+When loading a file, this is bound to the filename.  */);
+  Vread_end_of_file_name = Qnil;
+  DEFSYM (Qread_end_of_file_name, "read-end-of-file-name");
+
   DEFVAR_LISP ("user-init-file", Vuser_init_file,
 	       doc: /* File name, including directory, of user's initialization file.
 If the file loaded had extension `.elc', and the corresponding source file
-- 
2.39.3


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Sat, 17 Feb 2024 07:18:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> janestreet.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: dmitry <at> gutov.dev, 68546 <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90;
 end-of-file has incorrect data when signaled within a load
Date: Sat, 17 Feb 2024 09:14:58 +0200
> Cc: dmitry <at> gutov.dev
> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Date: Fri, 16 Feb 2024 16:56:26 -0500
> 
> Here is a straightforward fix.

Adding Stefan to the discussion, since this is no longer specific to
project.el.  Stefan, any comments?

> >From d850108fa309701e4899dfbcfd5a20d1e17f86af Mon Sep 17 00:00:00 2001
> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Date: Fri, 16 Feb 2024 16:53:28 -0500
> Subject: [PATCH] Prevent incorrect error message when calling read inside load
> 
> Previously, if `load' eval'd a `read' expression which raised
> end-of-file, the error would include load-true-file-name, even
> though the `read' may be reading something completely different.
> 
> Now, end-of-file errors raised by `read' will only include
> load-true-file-name if it's actually reading that file.
> 
> We do this by having read include read-end-of-file-name in the error
> instead of load-true-file-name, and only binding read-end-of-file-name
> around the "read" parts of readevalloop, not the "eval" parts.
> (load-true-file-name is still bound throughout)
> 
> Also, when reading a file (or some other source), it is now
> possible to bind read-end-of-file-name so that end-of-file
> errors raised by read will include the filename (or the string
> of your choice).  Previously, an end-of-file error raised by
> read outside of load would never include the filename.

This needs the obvious documentation changes.

Also, I'm not sure I understand the rationale well enough: what other
use cases do we envision where read-end-of-file-name will be bound to
some other string than the name of the file being read?  IOW, this is
a general infrastructure change, but the use cases that justify such
generality are not clear to me.

> --- a/src/lread.c
> +++ b/src/lread.c
> @@ -2385,8 +2385,8 @@ readevalloop_1 (int old)
>  static AVOID
>  end_of_file_error (void)
>  {
> -  if (STRINGP (Vload_true_file_name))
> -    xsignal1 (Qend_of_file, Vload_true_file_name);
> +  if (!NILP (Vread_end_of_file_name))
> +    xsignal1 (Qend_of_file, Vread_end_of_file_name);

Why !NILP instead of STRINGP? read-end-of-file-name is documented to
take string values.

>  
>    xsignal0 (Qend_of_file);
>  }
> @@ -2490,6 +2490,8 @@ readevalloop (Lisp_Object readcharfun,
>    while (continue_reading_p)
>      {
>        specpdl_ref count1 = SPECPDL_INDEX ();
> +      if (NILP (Vread_end_of_file_name))
> +	specbind (Qread_end_of_file_name, Vload_true_file_name);
>  
>        if (b != 0 && !BUFFER_LIVE_P (b))
>  	error ("Reading from killed buffer");
> @@ -2585,7 +2587,7 @@ readevalloop (Lisp_Object readcharfun,
>        if (!NILP (start) && continue_reading_p)
>  	start = Fpoint_marker ();
>  
> -      /* Restore saved point and BEGV.  */
> +      /* Restore saved point and BEGV, and unbind read_stream_for_error.  */

read_stream_for_error or read-end-of-file-name?  If the former, I
don't understand the rationale for this hunk.

> +  DEFVAR_LISP ("read-end-of-file-name", Vread_end_of_file_name,
> +	       doc: /* String to be included when `read' signals `end-of-file'.
                              ^^^^^^^^^^^^^^
"included" where?  This sentence needs to be clarified, either in it
or in the following text.

Also, the general nature of the feature is not reflected in the
variable's name: if this can be any string, why does it have
"file-name" in its name?

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Tue, 15 Oct 2024 19:17:01 GMT) Full text and rfc822 format available.

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dmitry <at> gutov.dev, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 68546 <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when
 signaled within a load
Date: Tue, 15 Oct 2024 15:16:00 -0400
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:

>> Cc: dmitry <at> gutov.dev
>> From: Spencer Baugh <sbaugh <at> janestreet.com>
>> Date: Fri, 16 Feb 2024 16:56:26 -0500
>> 
>> Here is a straightforward fix.
>
> Adding Stefan to the discussion, since this is no longer specific to
> project.el.  Stefan, any comments?
>
>> >From d850108fa309701e4899dfbcfd5a20d1e17f86af Mon Sep 17 00:00:00 2001
>> From: Spencer Baugh <sbaugh <at> janestreet.com>
>> Date: Fri, 16 Feb 2024 16:53:28 -0500
>> Subject: [PATCH] Prevent incorrect error message when calling read inside load
>> 
>> Previously, if `load' eval'd a `read' expression which raised
>> end-of-file, the error would include load-true-file-name, even
>> though the `read' may be reading something completely different.
>> 
>> Now, end-of-file errors raised by `read' will only include
>> load-true-file-name if it's actually reading that file.
>> 
>> We do this by having read include read-end-of-file-name in the error
>> instead of load-true-file-name, and only binding read-end-of-file-name
>> around the "read" parts of readevalloop, not the "eval" parts.
>> (load-true-file-name is still bound throughout)
>> 
>> Also, when reading a file (or some other source), it is now
>> possible to bind read-end-of-file-name so that end-of-file
>> errors raised by read will include the filename (or the string
>> of your choice).  Previously, an end-of-file error raised by
>> read outside of load would never include the filename.
>
> This needs the obvious documentation changes.

Yes, will update the Elisp manual and NEWS in the next version if this
approach is acceptable.

> Also, I'm not sure I understand the rationale well enough: what other
> use cases do we envision where read-end-of-file-name will be bound to
> some other string than the name of the file being read?  IOW, this is
> a general infrastructure change, but the use cases that justify such
> generality are not clear to me.

Anything that uses read can use this to provide more useful error data
and error messages.

Picking a random example, package-load-descriptor calls read on a buffer
containing data from a specific file.  Right now, if that read
encounters an error, it will (signal end-of-file nil).  If
package-load-descriptor bound read-end-of-file-data to the filename
around the "read" call, read will (signal end-of-file the-filename),
which will provide a much more useful error message.

This could be done with condition-case and re-raising errors, but that
would make stack-traces and debugging worse.

Or we could do it by extracting the filename out of the buffer being
read, but that won't work for e.g. reading strings.

>> --- a/src/lread.c
>> +++ b/src/lread.c
>> @@ -2385,8 +2385,8 @@ readevalloop_1 (int old)
>>  static AVOID
>>  end_of_file_error (void)
>>  {
>> -  if (STRINGP (Vload_true_file_name))
>> -    xsignal1 (Qend_of_file, Vload_true_file_name);
>> +  if (!NILP (Vread_end_of_file_name))
>> +    xsignal1 (Qend_of_file, Vread_end_of_file_name);
>
> Why !NILP instead of STRINGP? read-end-of-file-name is documented to
> take string values.

True.  I've changed it to read-end-of-file-data in the latest version of
my patch, which seems more generally useful.

>>  
>>    xsignal0 (Qend_of_file);
>>  }
>> @@ -2490,6 +2490,8 @@ readevalloop (Lisp_Object readcharfun,
>>    while (continue_reading_p)
>>      {
>>        specpdl_ref count1 = SPECPDL_INDEX ();
>> +      if (NILP (Vread_end_of_file_name))
>> +	specbind (Qread_end_of_file_name, Vload_true_file_name);
>>  
>>        if (b != 0 && !BUFFER_LIVE_P (b))
>>  	error ("Reading from killed buffer");
>> @@ -2585,7 +2587,7 @@ readevalloop (Lisp_Object readcharfun,
>>        if (!NILP (start) && continue_reading_p)
>>  	start = Fpoint_marker ();
>>  
>> -      /* Restore saved point and BEGV.  */
>> +      /* Restore saved point and BEGV, and unbind read_stream_for_error.  */
>
> read_stream_for_error or read-end-of-file-name?  If the former, I
> don't understand the rationale for this hunk.

Fixed, yes, it should have been read-end-of-file-name.

>> +  DEFVAR_LISP ("read-end-of-file-name", Vread_end_of_file_name,
>> +	       doc: /* String to be included when `read' signals `end-of-file'.
>                               ^^^^^^^^^^^^^^
> "included" where?  This sentence needs to be clarified, either in it
> or in the following text.

Fixed.

> Also, the general nature of the feature is not reflected in the
> variable's name: if this can be any string, why does it have
> "file-name" in its name?

True, changed to read-end-of-file-data to make it more clear. 

[0001-Prevent-incorrect-error-message-when-calling-read-in.patch (text/x-patch, inline)]
From 8b308258a0deafcfa3846c52efc651f09154ea9e Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Tue, 15 Oct 2024 15:04:48 -0400
Subject: [PATCH] Prevent incorrect error message when calling read inside load

Previously, if `load' eval'd a `read' expression which raised
end-of-file, the error would include load-true-file-name, even
though the `read' may be reading something completely different.

Now, end-of-file errors raised by `read' will only include
load-true-file-name if it's actually reading that file.

We do this by having read include read-end-of-file-data in the error
instead of load-true-file-name, and only binding read-end-of-file-data
around the "read" parts of readevalloop, not the "eval" parts.
(load-true-file-name is still bound throughout)

Also, when reading a file (or some other source), it is now
possible to bind read-end-of-file-data so that end-of-file
errors raised by read will include the filename (or the string
of your choice).  Previously, an end-of-file error raised by
read outside of load would never include the filename.

* src/lread.c (syms_of_lread): Add read-end-of-file-data.
(readevalloop): Bind read-end-of-file-data to
load-true-file-name around read.
(end_of_file_error): Use read-end-of-file-data instead of
load-true-file-name.  (bug#68546)
---
 src/lread.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/lread.c b/src/lread.c
index 95c6891c205..ec0c68e1843 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -2329,8 +2329,8 @@ readevalloop_1 (int old)
 static AVOID
 end_of_file_error (void)
 {
-  if (STRINGP (Vload_true_file_name))
-    xsignal1 (Qend_of_file, Vload_true_file_name);
+  if (!NILP (Vread_end_of_file_data))
+    xsignal1 (Qend_of_file, Vread_end_of_file_data);
 
   xsignal0 (Qend_of_file);
 }
@@ -2434,6 +2434,8 @@ readevalloop (Lisp_Object readcharfun,
   while (continue_reading_p)
     {
       specpdl_ref count1 = SPECPDL_INDEX ();
+      if (NILP (Vread_end_of_file_data))
+	specbind (Qread_end_of_file_name, Vload_true_file_name);
 
       if (b != 0 && !BUFFER_LIVE_P (b))
 	error ("Reading from killed buffer");
@@ -2529,7 +2531,7 @@ readevalloop (Lisp_Object readcharfun,
       if (!NILP (start) && continue_reading_p)
 	start = Fpoint_marker ();
 
-      /* Restore saved point and BEGV.  */
+      /* Restore saved point and BEGV, and unbind read_end_of_file_data.  */
       unbind_to (count1, Qnil);
 
       /* Now eval what we just read.  */
@@ -2661,7 +2663,10 @@ DEFUN ("read", Fread, Sread, 0, 1, 0,
      call it with a char as argument to push a char back)
  a string (takes text from string, starting at the beginning)
  t (read text line using minibuffer and use it, or read from
-    standard input in batch mode).  */)
+    standard input in batch mode).
+
+If an unterminated list, vector, or string is encountered, signal
+`end-of-file' with `read-end-of-file-data'.  */)
   (Lisp_Object stream)
 {
   if (NILP (stream))
@@ -5963,6 +5968,12 @@ syms_of_lread (void)
 	       doc: /* Full name of file being loaded by `load'.  */);
   Vload_true_file_name = Qnil;
 
+  DEFVAR_LISP ("read-end-of-file-data", Vread_end_of_file_data,
+	       doc: /* When `read' signals `end-of-file', it passes this to `signal' as DATA.
+When loading a file, this is bound to `load-true-file-name'.  */);
+  Vread_end_of_file_data = Qnil;
+  DEFSYM (Qread_end_of_file_data, "read-end-of-file-data");
+
   DEFVAR_LISP ("user-init-file", Vuser_init_file,
 	       doc: /* File name, including directory, of user's initialization file.
 If the file loaded had extension `.elc', and the corresponding source file
-- 
2.39.3


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Thu, 17 Oct 2024 17:58:02 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: dmitry <at> gutov.dev, Eli Zaretskii <eliz <at> gnu.org>, 68546 <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when
 signaled within a load
Date: Thu, 17 Oct 2024 13:57:29 -0400
> diff --git a/src/lread.c b/src/lread.c
> index 95c6891c205..ec0c68e1843 100644
> --- a/src/lread.c
> +++ b/src/lread.c
> @@ -2329,8 +2329,8 @@ readevalloop_1 (int old)
>  static AVOID
>  end_of_file_error (void)
>  {
> -  if (STRINGP (Vload_true_file_name))
> -    xsignal1 (Qend_of_file, Vload_true_file_name);
> +  if (!NILP (Vread_end_of_file_data))
> +    xsignal1 (Qend_of_file, Vread_end_of_file_data);

Hmm... why call it `Vread_end_of_file_name`?
How 'bout something like `read--source`?  I suspect it could be useful to
include similar info in other read errors than `Qend_of_file`.
Also it could make sense to allow that var to indicate when we're
reading from a buffer (rather than a file).

>    xsignal0 (Qend_of_file);
>  }
> @@ -2434,6 +2434,8 @@ readevalloop (Lisp_Object readcharfun,
>    while (continue_reading_p)
>      {
>        specpdl_ref count1 = SPECPDL_INDEX ();
> +      if (NILP (Vread_end_of_file_data))
> +        specbind (Qread_end_of_file_name, Vload_true_file_name);

Why the `if` condition here?  Sounds like it could lead to problems for
nested loads and such.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Tue, 08 Apr 2025 21:19:02 GMT) Full text and rfc822 format available.

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: dmitry <at> gutov.dev, Eli Zaretskii <eliz <at> gnu.org>, 68546 <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when
 signaled within a load
Date: Tue, 08 Apr 2025 17:18:23 -0400
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>>    xsignal0 (Qend_of_file);
>>  }
>> @@ -2434,6 +2434,8 @@ readevalloop (Lisp_Object readcharfun,
>>    while (continue_reading_p)
>>      {
>>        specpdl_ref count1 = SPECPDL_INDEX ();
>> +      if (NILP (Vread_end_of_file_data))
>> +        specbind (Qread_end_of_file_name, Vload_true_file_name);
>
> Why the `if` condition here?  Sounds like it could lead to problems for
> nested loads and such.

Good point.

Actually, a good enough point that I completely reworked the patch in
response to it.  We don't need to add a new dynamically scoped variable
at all: we can just pass readcharfun to end_of_file_error and check
that.  Much simpler.

This should solve the original bug; while I'm here, I also added a
BUFFERP check, so that read on a buffer will signal end-of-file with the
buffer as data instead of just nil.

I tried including the line number and column number as data in the
BUFFERP case, but it seemed a bit noisy, since it's only useful in the
case where the buffer is narrowed (otherwise the error is just obviously
at the end of the buffer).

[0001-Signal-end-of-file-with-more-correct-data.patch (text/x-patch, inline)]
From 03e06a591896ed85f3c38908a0b3105b8bafc945 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Tue, 8 Apr 2025 17:13:08 -0400
Subject: [PATCH] Signal end-of-file with more correct data

end_of_file_error previously always signaled end-of-file with
load-true-file-name if that was non-nil (and a string).
However, this might be the wrong thing to do; for example, if a
file being loaded calls read on a buffer.

We can determine what's correct by also checking if readcharfun
is reading from a file.  Also, by checking that we can signal a
better error for read on a buffer.

* src/lread.c (end_of_file_error): Check readcharfun to
determine what data to signal with.  (bug#68546)
(read_char_escape, read_char_literal, read_string_literal)
(skip_space_and_comments, read0): Pass readcharfun to
end_of_file_error.
---
 src/lread.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/lread.c b/src/lread.c
index d39330bd0eb..2f18d763b9c 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -2342,12 +2342,14 @@ readevalloop_1 (int old)
    information.  */
 
 static AVOID
-end_of_file_error (void)
+end_of_file_error (Lisp_Object readcharfun)
 {
-  if (STRINGP (Vload_true_file_name))
+  if (FROM_FILE_P (readcharfun) && STRINGP (Vload_true_file_name))
     xsignal1 (Qend_of_file, Vload_true_file_name);
-
-  xsignal0 (Qend_of_file);
+  else if (BUFFERP (readcharfun))
+    xsignal1 (Qend_of_file, readcharfun);
+  else
+    xsignal0 (Qend_of_file);
 }
 
 static Lisp_Object
@@ -2858,7 +2860,7 @@ read_char_escape (Lisp_Object readcharfun, int next_char)
   switch (c)
     {
     case -1:
-      end_of_file_error ();
+      end_of_file_error (readcharfun);
 
     case 'a': chr = '\a'; break;
     case 'b': chr = '\b'; break;
@@ -3031,7 +3033,7 @@ read_char_escape (Lisp_Object readcharfun, int next_char)
           {
             int c = READCHAR;
             if (c < 0)
-              end_of_file_error ();
+              end_of_file_error (readcharfun);
             if (c == '}')
               break;
             if (c >= 0x80)
@@ -3201,7 +3203,7 @@ read_char_literal (Lisp_Object readcharfun)
 {
   int ch = READCHAR;
   if (ch < 0)
-    end_of_file_error ();
+    end_of_file_error (readcharfun);
 
   /* Accept `single space' syntax like (list ? x) where the
      whitespace character is SPC or TAB.
@@ -3347,7 +3349,7 @@ read_string_literal (Lisp_Object readcharfun)
     }
 
   if (ch < 0)
-    end_of_file_error ();
+    end_of_file_error (readcharfun);
 
   if (!force_multibyte && force_singlebyte)
     {
@@ -3777,7 +3779,7 @@ skip_space_and_comments (Lisp_Object readcharfun)
 	  c = READCHAR;
 	while (c >= 0 && c != '\n');
       if (c < 0)
-	end_of_file_error ();
+	end_of_file_error (readcharfun);
     }
   while (c <= 32 || c == NO_BREAK_SPACE);
   UNREAD (c);
@@ -3972,7 +3974,7 @@ read0 (Lisp_Object readcharfun, bool locate_syms)
   bool multibyte;
   int c = READCHAR_REPORT_MULTIBYTE (&multibyte);
   if (c < 0)
-    end_of_file_error ();
+    end_of_file_error (readcharfun);
 
   switch (c)
     {
@@ -4403,7 +4405,7 @@ read0 (Lisp_Object readcharfun, bool locate_syms)
 	      {
 		c = READCHAR;
 		if (c < 0)
-		  end_of_file_error ();
+		  end_of_file_error (readcharfun);
 		quoted = true;
 	      }
 
-- 
2.39.3


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Wed, 09 Apr 2025 01:23:01 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: dmitry <at> gutov.dev, Eli Zaretskii <eliz <at> gnu.org>, 68546 <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when
 signaled within a load
Date: Tue, 08 Apr 2025 21:21:58 -0400
> Actually, a good enough point that I completely reworked the patch in
> response to it.  We don't need to add a new dynamically scoped variable
> at all: we can just pass readcharfun to end_of_file_error and check
> that.  Much simpler.

Looks good to me.  Any objection?


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Mon, 11 Aug 2025 16:48:02 GMT) Full text and rfc822 format available.

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: dmitry <at> gutov.dev, Eli Zaretskii <eliz <at> gnu.org>, 68546 <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when
 signaled within a load
Date: Mon, 11 Aug 2025 12:47:05 -0400
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> Actually, a good enough point that I completely reworked the patch in
>> response to it.  We don't need to add a new dynamically scoped variable
>> at all: we can just pass readcharfun to end_of_file_error and check
>> that.  Much simpler.
>
> Looks good to me.  Any objection?

Since there are no objections to the patch, perhaps we can land it now?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Tue, 19 Aug 2025 08:10:02 GMT) Full text and rfc822 format available.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Spencer Baugh <sbaugh <at> janestreet.com>, Eli Zaretskii <eliz <at> gnu.org>,
 68546 <at> debbugs.gnu.org, dmitry <at> gutov.dev
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when
 signaled within a load
Date: Tue, 19 Aug 2025 09:09:26 +0100
Hello,

On Mon 11 Aug 2025 at 12:47pm -04, Spencer Baugh via "Bug reports for GNU Emacs, the Swiss army knife of text editors" wrote:

> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>>> Actually, a good enough point that I completely reworked the patch in
>>> response to it.  We don't need to add a new dynamically scoped variable
>>> at all: we can just pass readcharfun to end_of_file_error and check
>>> that.  Much simpler.
>>
>> Looks good to me.  Any objection?
>
> Since there are no objections to the patch, perhaps we can land it now?

I took a look.  It doesn't compile because FROM_FILE_P has been replaced
with from_file_p, so I think the patch needs an update.

Eli had mentioned updates to the manual.  I looked through and I think
your more minimal patch means no updates are needed, so we are good
there.

You've got a fairly substantive commit message but generally the
convention for Emacs is to push things out of commit messages and into
comments, reserving commit message content for remarks that only make
sense associated with the act of making the change, not the new code.

So while the "end_of_file_error always previously signaled ..."
paragraph makes sense, perhaps the next one could be made into a
comment.  E.g.

    /* Don't use Vload_true_file_name unless this error occurred while
       actually reading from that file.  E.g. if a file being loaded
       calls Fread on a buffer and the latter read hits an EOF, we don't
       want to claim that anything went wrong reading from
       Vload_true_file_name.  */

Finally, the changelog should mention that end_of_file_error has a new
argument.  E.g.

* src/lread.c (end_of_file_error) <readcharfun>: New argument.

-- 
Sean Whitton




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#68546; Package emacs. (Tue, 19 Aug 2025 14:57:02 GMT) Full text and rfc822 format available.

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: dmitry <at> gutov.dev, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 68546 <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when
 signaled within a load
Date: Tue, 19 Aug 2025 10:55:57 -0400
[Message part 1 (text/plain, inline)]
Sean Whitton <spwhitton <at> spwhitton.name> writes:
> Hello,
>
> On Mon 11 Aug 2025 at 12:47pm -04, Spencer Baugh via "Bug reports for GNU Emacs, the Swiss army knife of text editors" wrote:
>
>> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>>>> Actually, a good enough point that I completely reworked the patch in
>>>> response to it.  We don't need to add a new dynamically scoped variable
>>>> at all: we can just pass readcharfun to end_of_file_error and check
>>>> that.  Much simpler.
>>>
>>> Looks good to me.  Any objection?
>>
>> Since there are no objections to the patch, perhaps we can land it now?
>
> I took a look.  It doesn't compile because FROM_FILE_P has been replaced
> with from_file_p, so I think the patch needs an update.
>
> Eli had mentioned updates to the manual.  I looked through and I think
> your more minimal patch means no updates are needed, so we are good
> there.
>
> You've got a fairly substantive commit message but generally the
> convention for Emacs is to push things out of commit messages and into
> comments, reserving commit message content for remarks that only make
> sense associated with the act of making the change, not the new code.
>
> So while the "end_of_file_error always previously signaled ..."
> paragraph makes sense, perhaps the next one could be made into a
> comment.  E.g.
>
>     /* Don't use Vload_true_file_name unless this error occurred while
>        actually reading from that file.  E.g. if a file being loaded
>        calls Fread on a buffer and the latter read hits an EOF, we don't
>        want to claim that anything went wrong reading from
>        Vload_true_file_name.  */
>
> Finally, the changelog should mention that end_of_file_error has a new
> argument.  E.g.
>
> * src/lread.c (end_of_file_error) <readcharfun>: New argument.

All fixed, updated patch attached.

[0001-Signal-end-of-file-with-more-correct-data.patch (text/x-patch, inline)]
From 59ddc42b8598977ece1658a581e78ac09c3206b9 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Tue, 19 Aug 2025 10:52:44 -0400
Subject: [PATCH] Signal end-of-file with more correct data

end_of_file_error previously always signaled end-of-file with
load-true-file-name if that was non-nil (and a string).
However, this might be the wrong thing to do; for example, if a
file being loaded calls read on a buffer.

* src/lread.c (end_of_file_error) <source>: New argument; check it to
determine what data to signal with.  (bug#68546)
(read_char_escape, read_char_literal, read_string_literal)
(skip_space_and_comments, read0): Pass source to
end_of_file_error.
---
 src/lread.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/src/lread.c b/src/lread.c
index 57d3239e283..f1ba48eac34 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -2114,12 +2114,16 @@ build_load_history (Lisp_Object filename, bool entire)
    information.  */
 
 static AVOID
-end_of_file_error (void)
+end_of_file_error (source_t *source)
 {
-  if (STRINGP (Vload_true_file_name))
+  if (from_file_p (source))
+    /* Only Fload calls read on a file, and Fload always binds
+       load-true-file-name around the call. */
     xsignal1 (Qend_of_file, Vload_true_file_name);
-
-  xsignal0 (Qend_of_file);
+  else if (source->get == source_buffer_get)
+    xsignal1 (Qend_of_file, source->object);
+  else
+    xsignal0 (Qend_of_file);
 }
 
 static Lisp_Object
@@ -2604,7 +2608,7 @@ read_char_escape (source_t *source, int next_char)
   switch (c)
     {
     case -1:
-      end_of_file_error ();
+      end_of_file_error (source);
 
     case 'a': chr = '\a'; break;
     case 'b': chr = '\b'; break;
@@ -2777,7 +2781,7 @@ read_char_escape (source_t *source, int next_char)
           {
             int c = readchar (source);
             if (c < 0)
-              end_of_file_error ();
+              end_of_file_error (source);
             if (c == '}')
               break;
             if (c >= 0x80)
@@ -2819,7 +2823,7 @@ read_char_escape (source_t *source, int next_char)
       break;
     }
   if (chr < 0)
-    end_of_file_error ();
+    end_of_file_error (source);
   eassert (chr >= 0 && chr < (1 << CHARACTERBITS));
 
   /* Apply Control modifiers, using the rules:
@@ -2982,7 +2986,7 @@ read_char_literal (source_t *source)
 {
   int ch = readchar (source);
   if (ch < 0)
-    end_of_file_error ();
+    end_of_file_error (source);
 
   /* Accept `single space' syntax like (list ? x) where the
      whitespace character is SPC or TAB.
@@ -3118,7 +3122,7 @@ read_string_literal (source_t *source)
     }
 
   if (ch < 0)
-    end_of_file_error ();
+    end_of_file_error (source);
 
   if (!force_multibyte && force_singlebyte)
     {
@@ -3548,7 +3552,7 @@ skip_space_and_comments (source_t *source)
 	  c = readchar (source);
 	while (c >= 0 && c != '\n');
       if (c < 0)
-	end_of_file_error ();
+	end_of_file_error (source);
     }
   while (c <= 32 || c == NO_BREAK_SPACE);
   unreadchar (source, c);
@@ -3734,7 +3738,7 @@ read0 (source_t *source, bool locate_syms)
   Lisp_Object obj;
   int c = readchar (source);
   if (c < 0)
-    end_of_file_error ();
+    end_of_file_error (source);
 
   switch (c)
     {
@@ -4151,7 +4155,7 @@ read0 (source_t *source, bool locate_syms)
 	      {
 		c = readchar (source);
 		if (c < 0)
-		  end_of_file_error ();
+		  end_of_file_error (source);
 		quoted = true;
 	      }
 
-- 
2.43.7


Reply sent to Sean Whitton <spwhitton <at> spwhitton.name>:
You have taken responsibility. (Tue, 19 Aug 2025 16:01:02 GMT) Full text and rfc822 format available.

Notification sent to Spencer Baugh <sbaugh <at> janestreet.com>:
bug acknowledged by developer. (Tue, 19 Aug 2025 16:01:02 GMT) Full text and rfc822 format available.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: dmitry <at> gutov.dev, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 68546-done <at> debbugs.gnu.org
Subject: Re: bug#68546: 29.1.90; end-of-file has incorrect data when
 signaled within a load
Date: Tue, 19 Aug 2025 17:00:32 +0100
Hello,

Thank you, installed and closing.

-- 
Sean Whitton




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 17 Sep 2025 11:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 62 days ago.

Previous Next


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