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.