From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Spencer Baugh via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: 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-ID: References: <86il2nv9xp.fsf@gnu.org> Reply-To: Spencer Baugh Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="13614"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: dmitry@gutov.dev, Stefan Monnier , 68546@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Oct 15 21:17:00 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1t0n2R-0003PR-SK for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 15 Oct 2024 21:17:00 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1t0n2D-00059H-M6; Tue, 15 Oct 2024 15:16:45 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1t0n2B-00058r-Ly for bug-gnu-emacs@gnu.org; Tue, 15 Oct 2024 15:16:43 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1t0n2B-0001Ea-DD for bug-gnu-emacs@gnu.org; Tue, 15 Oct 2024 15:16:43 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=MIME-Version:Date:References:In-Reply-To:From:To:Subject; bh=4pDmav0+V8BSichL52AvvCJXFfjrd8aTweaUgAONqRo=; b=Hfe/0YdzLIUPYs6n9yAUFg/78Tnfsz6mrX1W79JmwfAXSxtJGV1lyz5jaiTBKMdOEKCo7wg2sKKhCNk197DwE6XakocfzlI3TD4MYJGCtZ/vxap6NK60S42d1L2QPK7CiIxT6O7LIGA9P/uTA/xj1yVSXFZ8hrn0oZhjVAlMEd3XqlvyeRiZPpqRr9JMSQf6xzoCokroqZ9mv5cwy5cyr57wiCLFzGvDvSIeG26rgO6QHmshYr9HIst9RnEpd9ZEXKYLtBlFxi9wTbNhjv3a60ugX99zGmtElmG8vmrKf9eMmN5/XIHndVTjgR8DgRImWTH6PL0acfK1Os43byLBPg==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1t0n2T-0004cv-Q2 for bug-gnu-emacs@gnu.org; Tue, 15 Oct 2024 15:17:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 15 Oct 2024 19:17:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 68546 X-GNU-PR-Package: emacs Original-Received: via spool by 68546-submit@debbugs.gnu.org id=B68546.172901978717674 (code B ref 68546); Tue, 15 Oct 2024 19:17:01 +0000 Original-Received: (at 68546) by debbugs.gnu.org; 15 Oct 2024 19:16:27 +0000 Original-Received: from localhost ([127.0.0.1]:57505 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t0n1u-0004az-Ru for submit@debbugs.gnu.org; Tue, 15 Oct 2024 15:16:27 -0400 Original-Received: from mxout6.mail.janestreet.com ([64.215.233.21]:41753) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t0n1s-0004aV-C6 for 68546@debbugs.gnu.org; Tue, 15 Oct 2024 15:16:25 -0400 In-Reply-To: <86il2nv9xp.fsf@gnu.org> (Eli Zaretskii's message of "Sat, 17 Feb 2024 09:14:58 +0200") DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1729019760; bh=4pDmav0+V8BSichL52AvvCJXFfjrd8aTweaUgAONqRo=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=1td6fK38n+5DCyxS76WSrnen2RfzoTa17c9JrsuBnFnjAl7SdOZ0QrxtJ9xg+EuoR 9mOOQgoEng7eVUck3GzpzSUi2wxwLnNZtkXgmBmAI85d04CxYbVHhr05jdBByV7tNV M0F+VW4+bVhmnDok54Wmp0esFPHfdJ1402BR2tPldktPoq/tuwG34PHXycpGairfwU HHZq+/g731045SNe1IBofVhYoXIs/0I1Ja7vQjsrUundC7sXTkAOdwdcG+LhoeXe9/ Vh9mHSsboZK2l0PEuXrDNQhWbJ2hsfQOFQceEdQRGXjux+EiPhlNdLZPHtcL3G2s3d cSb1FfNBioazg== X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:293640 Archived-At: --=-=-= Content-Type: text/plain Eli Zaretskii writes: >> Cc: dmitry@gutov.dev >> From: Spencer Baugh >> 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 >> 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. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Prevent-incorrect-error-message-when-calling-read-in.patch >From 8b308258a0deafcfa3846c52efc651f09154ea9e Mon Sep 17 00:00:00 2001 From: Spencer Baugh 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 --=-=-=--