25.3; compiled commands don't respect special interactive expressions

  • Open
  • quality assurance status badge
Details
7 participants
  • Drew Adams
  • Eli Zaretskii
  • Lars Ingebrigtsen
  • Stefan Monnier
  • Noam Postavsky
  • Glenn Morris
  • Allen Li
Owner
unassigned
Submitted by
Allen Li
Severity
normal
A
A
Allen Li wrote on 17 Nov 2017 07:17
(address . bug-gnu-emacs@gnu.org)
CAJr1M6fEjAkrvDuGZS=XvAm8w6raDmsE6TPyq89mQbJcewfUNw@mail.gmail.com
Repro:

1. emacs -Q
2. Mark some text in the buffer
3. M-x append-to-buffer RET *scratch* RET
4. C-x ESC ESC

Expected last command:

(append-to-buffer "*scratch*" (region-beginning) (region-end))

Actual last command:

(append-to-buffer "*scratch*" 1 145)

If you then go and re-evaluate append-to-buffer (thus loading the
source version instead of the compiled version, you get

(append-to-buffer "*scratch*" (region-beginning) (region-end))

Thus, it looks like the compiled command doesn't handle special
interactive forms correctly.
D
D
Drew Adams wrote on 17 Nov 2017 15:55
07ace48b-93b3-4b01-9f71-b69ed44492c6@default
Toggle quote (21 lines)
> 1. emacs -Q
> 2. Mark some text in the buffer
> 3. M-x append-to-buffer RET *scratch* RET
> 4. C-x ESC ESC
>
> Expected last command:
>
> (append-to-buffer "*scratch*" (region-beginning) (region-end))
>
> Actual last command:
>
> (append-to-buffer "*scratch*" 1 145)
>
> If you then go and re-evaluate append-to-buffer (thus loading the
> source version instead of the compiled version, you get
>
> (append-to-buffer "*scratch*" (region-beginning) (region-end))
>
> Thus, it looks like the compiled command doesn't handle special
> interactive forms correctly.

Yes, thank you! This is something that has bugged me
for a while. This change is actually a regression (or
else on purpose?), introduced in Emacs 24. In all
Emacs releases prior to 24 it works as a user expects.

Dunno whether this bug report might be a duplicate. It
seems unlikely that no one (including me) has reported
this before. It reduces the utility of `C-x ESC ESC'
considerably.

An inexperienced user will likely give up altogether
trying to use such a command with `C-x ESC ESC', if
not give up on `C-x ESC ESC' entirely, through not
fully understanding. And an experienced user has
the annoyance of having to edit the hard-coded values
to get the behavior expected.
G
G
Glenn Morris wrote on 21 Nov 2017 20:15
control message for bug 29334
(address . control@debbugs.gnu.org)
E1eHE1C-0005aE-Ll@fencepost.gnu.org
found 29334 24.4
G
G
Glenn Morris wrote on 21 Nov 2017 20:45
Re: bug#29334: 25.3; compiled commands don't respect special interactive expressions
(name . Allen Li)(address . vianchielfaura@gmail.com)(address . 29334@debbugs.gnu.org)
h4o9nvl8kk.fsf@fencepost.gnu.org
Allen Li wrote:

Toggle quote (13 lines)
> 1. emacs -Q
> 2. Mark some text in the buffer
> 3. M-x append-to-buffer RET *scratch* RET
> 4. C-x ESC ESC
>
> Expected last command:
>
> (append-to-buffer "*scratch*" (region-beginning) (region-end))
>
> Actual last command:
>
> (append-to-buffer "*scratch*" 1 145)

Bisected to a46481370, our old friend "Use lexical-binding".
N
N
Noam Postavsky wrote on 24 Nov 2017 04:11
(name . Glenn Morris)(address . rgm@gnu.org)
87efooz7z8.fsf@users.sourceforge.net
tags 29334 + patch
quit

Glenn Morris <rgm@gnu.org> writes:

Toggle quote (2 lines)
> Bisected to a46481370, our old friend "Use lexical-binding".

How about this:
From a8b43e98c592c84957ea304a0dc2d6423af9c5c5 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Thu, 23 Nov 2017 21:57:09 -0500
Subject: [PATCH] Fix command repetition with lexical-binding (Bug#29334)

`call-interactively' relies on analyzing the source of `interactive'
forms in order to preserve arguments like (region-end) in the command
history, rather than just storing the resulting position. However,
the byte-compiler does not preserve the source of the interactive form
when lexical-binding is in effect, because `call-interactively' would
evaluate the form with dynamic binding in that case.

To fix this, change `call-interactively' so that it checks compiled
functions for lexical-binding as well. Then the byte-compiler can
preserve the source of interactive forms regardless of the value of
lexical-binding.

* src/callint.c (Fcall_interactively): Functions compiled with
lexical-binding have their arglist encoded as an integer, use this to
choose the right kind of binding for compiled functions too.
* lisp/emacs-lisp/bytecomp.el (byte-compile-lambda): Preserve the
uncompiled form of the interactive form when lexical-binding is
enabled too.
---
lisp/emacs-lisp/bytecomp.el | 6 +-----
src/callint.c | 4 +++-
2 files changed, 4 insertions(+), 6 deletions(-)

Toggle diff (34 lines)
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 590db570c5..e16405f09b 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2823,11 +2823,7 @@ byte-compile-lambda
(while (consp (cdr form))
(setq form (cdr form)))
(setq form (car form)))
- (if (and (eq (car-safe form) 'list)
- ;; The spec is evalled in callint.c in dynamic-scoping
- ;; mode, so just leaving the form unchanged would mean
- ;; it won't be eval'd in the right mode.
- (not lexical-binding))
+ (if (eq (car-safe form) 'list)
nil
(setq int `(interactive ,newform)))))
((cdr int)
diff --git a/src/callint.c b/src/callint.c
index 5d88082e38..48ea9ba7a3 100644
--- a/src/callint.c
+++ b/src/callint.c
@@ -356,7 +356,9 @@ DEFUN ("call-interactively", Fcall_interactively, Scall_interactively, 1, 3, 0,
/* Compute the arg values using the user's expression. */
specs = Feval (specs,
CONSP (funval) && EQ (Qclosure, XCAR (funval))
- ? CAR_SAFE (XCDR (funval)) : Qnil);
+ ? CAR_SAFE (XCDR (funval))
+ : COMPILEDP (funval) && INTEGERP (AREF (funval, COMPILED_ARGLIST))
+ ? Qt : Qnil);
if (events != num_input_events || !NILP (record_flag))
{
/* We should record this command on the command history. */
--
2.11.0
E
E
Eli Zaretskii wrote on 24 Nov 2017 08:58
(name . Noam Postavsky)(address . npostavs@users.sourceforge.net)
83vai09kgq.fsf@gnu.org
Toggle quote (25 lines)
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Thu, 23 Nov 2017 22:11:23 -0500
> Cc: 29334@debbugs.gnu.org, Allen Li <vianchielfaura@gmail.com>
>
> > Bisected to a46481370, our old friend "Use lexical-binding".
>
> How about this:
>
> >From a8b43e98c592c84957ea304a0dc2d6423af9c5c5 Mon Sep 17 00:00:00 2001
> From: Noam Postavsky <npostavs@gmail.com>
> Date: Thu, 23 Nov 2017 21:57:09 -0500
> Subject: [PATCH] Fix command repetition with lexical-binding (Bug#29334)
>
> `call-interactively' relies on analyzing the source of `interactive'
> forms in order to preserve arguments like (region-end) in the command
> history, rather than just storing the resulting position. However,
> the byte-compiler does not preserve the source of the interactive form
> when lexical-binding is in effect, because `call-interactively' would
> evaluate the form with dynamic binding in that case.
>
> To fix this, change `call-interactively' so that it checks compiled
> functions for lexical-binding as well. Then the byte-compiler can
> preserve the source of interactive forms regardless of the value of
> lexical-binding.

Thanks. If no objections are voiced to this approach, please push it
to the master branch. I think this is too radical for the release
branch.

P.S. Should this change be reflected in the ELisp manual somehow?
N
N
Noam Postavsky wrote on 24 Nov 2017 13:17
(name . Eli Zaretskii)(address . eliz@gnu.org)
877eufzx8p.fsf@users.sourceforge.net
Eli Zaretskii <eliz@gnu.org> writes:

Toggle quote (9 lines)
>> To fix this, change `call-interactively' so that it checks compiled
>> functions for lexical-binding as well. Then the byte-compiler can
>> preserve the source of interactive forms regardless of the value of
>> lexical-binding.
>
> Thanks. If no objections are voiced to this approach, please push it
> to the master branch. I think this is too radical for the release
> branch.

Hah, okay, but that was the conservative approach! The radical one
would be resolving this FIXME:

static void
fix_command (Lisp_Object input, Lisp_Object values)
{
/* FIXME: Instead of this ugly hack, we should provide a way for an
interactive spec to return an expression/function that will re-build the
args without user intervention. */

Toggle quote (2 lines)
> P.S. Should this change be reflected in the ELisp manual somehow?

I don't see where.
N
N
Noam Postavsky wrote on 4 Jan 2018 03:56
(name . Eli Zaretskii)(address . eliz@gnu.org)
87d12qs55z.fsf@users.sourceforge.net
tags 29334 fixed
close 29334 27.1
quit

Noam Postavsky <npostavs@users.sourceforge.net> writes:

Toggle quote (11 lines)
> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> To fix this, change `call-interactively' so that it checks compiled
>>> functions for lexical-binding as well. Then the byte-compiler can
>>> preserve the source of interactive forms regardless of the value of
>>> lexical-binding.
>>
>> Thanks. If no objections are voiced to this approach, please push it
>> to the master branch. I think this is too radical for the release
>> branch.

Pushed to master.

[1: ce48658191]: 2018-01-03 20:51:28 -0500
Fix command repetition with lexical-binding (Bug#29334)
S
S
Stefan Monnier wrote on 4 Jan 2018 06:25
(name . Drew Adams)(address . drew.adams@oracle.com)
jwv4lo2436z.fsf-monnier+bug#29334@gnu.org
Toggle quote (4 lines)
> Yes, thank you! This is something that has bugged me
> for a while. This change is actually a regression (or
> else on purpose?), introduced in Emacs 24.

Indeed. As for the question in parenthesis, yes I consciously decided
to break this "ugly hack" back then, wondering how much time it would
take for people to notice it (and hoping I'd find the time/motivation
to "do it right" in the mean time).

Toggle quote (10 lines)
> Hah, okay, but that was the conservative approach! The radical one
> would be resolving this FIXME:
>
> static void
> fix_command (Lisp_Object input, Lisp_Object values)
> {
> /* FIXME: Instead of this ugly hack, we should provide a way for an
> interactive spec to return an expression/function that will re-build the
> args without user intervention. */

E.g. we could allow the interactive spec to return either a list of
argument *values*, or a list of argument *expressions* (and we could
distinguish the two by having the second be a vector instead of a list,
for example).


Stefan
N
N
Noam Postavsky wrote on 7 Jan 2018 03:55
(name . Eli Zaretskii)(address . eliz@gnu.org)
87bmi6qsxg.fsf@users.sourceforge.net
notfixed 29334 27.1
reopen 29334
tag 29334 - fixed patch
quit

Noam Postavsky <npostavs@users.sourceforge.net> writes:

Toggle quote (6 lines)
> Pushed to master.
>
> [1: ce48658191]: 2018-01-03 20:51:28 -0500
> Fix command repetition with lexical-binding (Bug#29334)
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=ce48658191befb7734a7af484e368af5ed8b9447

Turns out this doesn't actually work, so I've reverted it.

L
L
Lars Ingebrigtsen wrote on 10 Feb 2022 08:56
(name . Allen Li)(address . vianchielfaura@gmail.com)(address . 29334@debbugs.gnu.org)
87a6ezcfye.fsf@gnus.org
Allen Li <vianchielfaura@gmail.com> writes:

Toggle quote (13 lines)
> 1. emacs -Q
> 2. Mark some text in the buffer
> 3. M-x append-to-buffer RET *scratch* RET
> 4. C-x ESC ESC
>
> Expected last command:
>
> (append-to-buffer "*scratch*" (region-beginning) (region-end))
>
> Actual last command:
>
> (append-to-buffer "*scratch*" 1 145)

This bug is still present in Emacs 29.

--
(domestic pets only, the antidote for overdose, milk.)
L
L
Lars Ingebrigtsen wrote on 10 Feb 2022 08:56
control message for bug #29334
(address . control@debbugs.gnu.org)
878rujcfy1.fsf@gnus.org
tags 29334 + confirmed
quit
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 29334@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 29334
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch