Subject: bug#78898: Make read/readevalloop reentrant

From: Stefan Monnier <[email-masked]>
Date: Sun, 29 Jun 2025 23:54:59 -0400

>>     temacs ... '(progn (blabla))'
>>
>> I still can't see why you'd need a reentrant `read` for that.
>
> Basically the latter, because I was looking at using it Makefiles and
> didn't want to deal with generating little files to load.  If I was
> willing to limit the strings to single expressions, you'd be right,
> but why should the form a program is input (string, file, buffer,
> whatever) determine what's allowed?

I don't understand the connection.
Hmm... oh, is it because when you use `read` on a string it can read
only the first sexp?  If so, I kinda see your point, but since you know
you're starting from a string, you can use `read-from-string` which
returns the info needed for the loop that reads all the sexps.

> I did try to get by with just save/restore on the string variables,
> but it failed when evaluating '(load "loadup.el") (print "Finished")".

I still can't imagine what that "just save/restore on the string
variables" looked like.

> The failure certainly seems like an issue caused by some subtlety in
> the way recursive use of readevalloop is implemented, but I can't

>From which I gather the above "just save/restore on the string
variables" somehow involved `readevalloop`, but I don't know how.

> bring myself to try and figure out a workaround for the specific
> brokenness when I could just rework it to be sure its reentrant, then
> see if that fixes the issue.

🙂
Seems like a big hammer for that little problem.
But if it can get you to improve our code, who are we to judge, eh?

> Or maybe the problem I'm solving is my ignorance of how the Emacs
> runtime works in practice?  A lack of amusing pasttimes?  A stubborn
> refusal to accept things that do not make sense to me?

You're at the right place.

>> Any hope you can give us an actual concrete scenario?
> I keep giving you one, you just consider it unnecessary.

No, I just can't imagine how you get from the problem you describe to an
attempted solution that uses `readevalloop` (from there I see how you
got to the re-entrancy thingy).

>> >> How/when&why do you expect/want `read` to be called recursively?
>> > It can definitely be entered multiple times if a stream being read
>> > blocks and the command loop executes another function that calls read.
>> > That can clearly happen when reading from user-supplied procedures or
>> > a raw stdio stream (i.e. from "load").
>>
>> A concrete scenario would help, here as well.
>
> Ok, there are calls to maybe quit in readbyte_from_stdio where the
> file returns EOF due to an interrupt.  So, if one function is reading
> a lisp expression when this occurs, then the user type "M-: (read
> <something>)", you will have a recursive re-entry to read.

Hmm... IIRC `maybe_quit` is not supposed to run ELisp code.
The more likely path to the problem would seem to be via a stream
that's a function.

In any case, if you don't know of a recipe to reproduce the problem,
don't bother looking for one, I was asking because I thought you had
bumped into one, so you could just exhibit it, I didn't mean to ask you
to reverse-engineer it from the code.

[ FWIW, I definitely believe you that it's possible.  But I expect we
  have *many* potential problems like these in our code base, so unless
  we have a clear way to solve a whole class of them I'd rather focus on
  the ones that are known to occur in existing code.  ]

> It's not really intended to be a procedure, just a static initializer.

I understand, but functions are just so much better behaved than macros
that it's worth avoiding macros when a function can be used just as well.

> I haven't regularly programmed in C since the decade I learned the
> language from the recently published K&R 2nd edition, so my style
> discipline is pretty low.

That was my assumption.  Since then, compilers have gotten much better
at inlining functions, so you can do

    init_inbuffer (&x);

and they'll generate the same code as it does with your

    struct inbuffer x = INIT_INBUFFER ();

[ Well, admittedly, not with `-O0`.  ]

Note also that

    struct inbuffer x = { .buffer = buf };

should work just as well, so maybe "nothing" is an even better option
than either.

> It took me forever to work out a "missing braces" message was
> referring to a specpdl_ref initializer in the INIT_LREAD_FRAME.

🙂

>> Also, why do you need the "uninit" case?
> That's my unfamiliarity with C99 and zero-initializing a union
> and being certain it zeros the entire thing and not just the
> first alternative.

Ah I see.  AFAIK `memset` is the standard solution.
`grep` shows it's used extensively in our code for that very purpose.
[ Current C compilers routinely replace calls to `memset` and `memcpy`
  with open-coded machine code when the size is known at compile-time.  ]

        Stefan