Subject: bug#78737: sit-for behavior changes when byte-compiled

From: Daniel Colascione <[email-masked]>
Date: Wed, 11 Jun 2025 07:08:45 -0700

Eli Zaretskii <[email-masked]> writes:

>> From: Daniel Colascione <[email-masked]>
>> Cc: Stefan Monnier <[email-masked]>,  Eli Zaretskii
>>  <[email-masked]>,  [email-masked]
>> Date: Wed, 11 Jun 2025 05:40:29 -0700
>> 
>> Pip Cet <[email-masked]> writes:
>> 
>> > (while t (read-event))
>> >
>> > on your branch appears to hang the Emacs session unquittably (even
>> > SIGUSR2 doesn't seem to work).  It should permit quits, because that
>> > code says nothing about wanting quits to be reported.
>> 
>> By calling read-event, you're asking Emacs read an event.  C-g is an
>> event.  Lisp isn't saying
>> read-event-unless-it's-C-g-in-which-case-exit-non-locally.  Lisp says
>> read-event.  Emacs is doing exactly what Lisp is asking it to do.
>
> How is calling read-event different from calling any other primitive
> in this context?  Typing C-g while any Lisp primitive is called is
> expected to throw to top-level, instead of doing whatever the
> primitive's contract says.  Why is read-event different in this
> regard?

Why is it good that read-key-sequence swallows quits but not read-char?
C-g is an event.  Why should read-event (but not read-key-sequence?)
translate one kind of input event (C-g) to an action (signal quit) but
return other kinds of events as they're given?

>> Besides, (let ((inhibit-quit t)) (while t (read-event))) will be
>> similarly unquittable on master today.
>
> Because the caller explicitly didn't want it to be quittable.

The caller asked to read an event.  That's exactly what we did.  The way
to reduce the likelihood of bugs is not to freeze the code, but to
understand it, simplify its contract, and probably write some tests
and documentation.

What specifically do you think might break here?

>> On current master, (read-event) then C-g signals quit but
>> (read-key-sequence "") C-g returns a quit_char key.  Why?  Why should we
>> expect (while t (read-event)) to be quittable but not (while t
>> (read-key-sequence "")), which can't be quit today on master?  Why would
>> the _high level_ event reading function report quit as a character but
>> the low-level one exit nonlocally?
>
> Because a higher-level function knows the context better?  I don't see
> anything wrong with that.

The higher level function wants to read an event.  That's exactly what
this function does now.

>> So what if the behavior of read-event changes?  Does having read-event
>> report a quit as an event actually break anything considering that
>> _sometimes_ it already can report a C-g as an event and not a
>> quit signal.
>
> Sorry, we cannot assume that attitude, not in this case.  Making
> incompatible behavior changes in this area means we will definitely
> break enough use cases to cause a flood of bug reports.  We cannot do
> that so freely and nonchalantly.  We could perhaps make it an optional
> behavior, but that's all.
>
> I appreciate the urge to fix what you perceive as inconsistencies, but
> that alone doesn't IMO justify risky incompatible changes in default
> behavior, especially if they leave no "fire escape".  Please
> understand where I stand on this.  This position isn't arbitrary, it
> is based on our experience from making changes in this area in the
> recent years.

We can't just avoid changing things because we don't understand the
code.  I put a fair bit of time into understanding this code, and that
means

>> The current ambiguity is confusing and leads to real-world actual bugs.
>> A lisp-visible behavior change isn't dispositive by itself: any
>> resolution of the current ambiguity will result in new behavior, so we
>> might as well make it good new behavior.
>
> We could also leave the old behavior alone.  It isn't bad behavior.

So you're saying it's a good thing that the behavior of the code changes
depending on whether it's compiled?  You're right: that's not bad
behavior.  It's abysmal behavior.

This whole area is full of bugs.  Are we seriously in "fixing bugs is
bad because something might change" territory?  In a development branch?
With a knob (for now) to revert to the old, broken behavior?

One way to make the code more understandable is to
make it consistent and clearly define its contract.  When code has a
regular structure, you don't need to keep a thousand special cases in
your mind when working on it.  Regularity allows you to substitute
reason for memory.  The code is pretty irregular right now.