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

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

[Spencer Baugh]

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))

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

[Dmitry Gutov]

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)

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

[Spencer Baugh]

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

On Wed, Jan 17, 2024 at 3:55 PM Dmitry Gutov <dmitry@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.

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

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

[Spencer Baugh]

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

On Thu, Jan 25, 2024 at 9:05 AM Spencer Baugh <sbaugh@janestreet.com> wrote:

> On Wed, Jan 17, 2024 at 3:55 PM Dmitry Gutov <dmitry@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.

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

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

[Eli Zaretskii]

> Cc: 68546@debbugs.gnu.org
> From: Spencer Baugh <sbaugh@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

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

[Dmitry Gutov]

On 25/01/2024 16:29, Eli Zaretskii wrote:
>> Cc:68546@debbugs.gnu.org
>> From: Spencer Baugh<sbaugh@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.

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

[Dmitry Gutov]

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.

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

[Dmitry Gutov]

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.

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

[Spencer Baugh]

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


Here is a straightforward fix.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Prevent-incorrect-error-message-when-calling-read-in.patch --]
[-- Type: text/x-patch, Size: 3151 bytes --]

From d850108fa309701e4899dfbcfd5a20d1e17f86af Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@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

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

[Eli Zaretskii]

> Cc: dmitry@gutov.dev
> From: Spencer Baugh <sbaugh@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@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.

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

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

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

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: dmitry@gutov.dev
>> From: Spencer Baugh <sbaugh@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@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. 


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Prevent-incorrect-error-message-when-calling-read-in.patch --]
[-- Type: text/x-patch, Size: 3669 bytes --]

From 8b308258a0deafcfa3846c52efc651f09154ea9e Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@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

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

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

> 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