> I want to use readevalloop to process command-line arguments, and my
> initial attempts failed to handle '"(load "loadup.el") (princ
> "Done")'. So I took a stab at making all the entry points synchronize
> with an explicit stack of continuation frames shared between read0 and
> readevalloop.
I'm afraid with the `Subject:` and the above paragraph, I'm still not
able to figure out what you're trying to achieve (and the patch is
quite large).
Also `read` and `readevalloop` are quite different. Do you want both to
be re-entrant? Is it in both cases for the same reason?
AFAIK `readevalloop` is also re-entrant in that you can call it
recursively from its "eval" part. What do you mean by "process
command-line arguments"?
It's true that read and readevalloop are distinct, but, ignoring the readfun parameter of readevalloop, any static variables read0 is dependent on, readevalloop is also dependent on. It's also true that not every entry to read0 is an entry to readevalloop, but practically every entry to readevalloop will enter read0. And they both depend on the state accessed by readchar and unreadchar. So, readevalloop can never really be reentrant as long as read0/readchar/unreadchar are not. My initial (current) approach has been to identify all the private state in lread.c accessed (directly or indirectly) and create a single data structure representing that state as a collective stack frame for essentially all the procedures of lread.c in the transitive closure of readevalloop 's call graph. It's true that this data structure is bigger than necessary, but readevalloop only has to allocate one frame that will be used by each invocation of read0 it makes in that activation frame of readevalloop.
Initially, I had tried implementing a simple read-eval loop for processing sequences of expressions in a string, similar to "load" for files or "eval-buffer". I had implemented a new temacs mode, eval-args, which skips loadup and all other command line options processing, to evaluate programs given as command line arguments. I made read_internal_start reentrant with respect to the obvious string state variables using unwind-protect and specbind techniques. However, it wasn't enough, because while it worked for simple cases like "(princ comp-abi-hash) (terpri)" or even a simple repl on stdin, the program '(load "loadup.el") (print "Finished")' would complete loading load up, but immediately exit without printing "Finished". Or, if I typed '(load "loadup.el")' into that simple repl, it would immediately exit after completing the load.
I considered that even if I determined the proximate cause of the issue, it would be difficult to tell whether any particular solution would be TRT or just a band-aid. So, I suppose the most immediate advantage for me is improving my ability to debug recursive invocations of read0/readevalloop by looking at the explicit stack of frames. In lread.c's current state, you can't tell just from looking at the static variables how or whether they are related to one another, and in what order, at least not without travelling along the backtrace and digging through the specpdl entries.
So, for example, I've implemented an input stream "reader-input-stream" that can be used arbitrarily by code being evaluated for characters from the "current" input stream, which is pretty much impossible in the current implementation. It's main use is for "readfun" in readevalloop. Implementing "eval-stream", which evaluates all input streams as programs uniformly, becomes trivial.
Another consideration is just the incremental removal of some barriers to concurrency . Really, the "current_frame" variable should be thread local, the way specpdl variables are. It just seemed like too much for my first cut at this. Obviously, it's just a drop in the bucket, but with the progress of the igc branch, effective concurrency is looking more feasible.
That said, the patch was derived by dropping the lread.c from that other project into a recent master, and hacking on it a bit to make it function on a stand-alone basis. So I can cut out some of the elements of the patch that aren't really needed for re-entrancy.
Some of the changes (like moving a lot of definitions/declarations to the top of the file) are just because I had to identify the functions that were being changed to take the single "frame" parameter and keep track of them somehow. There was a lot of trial and error in figuring out what information belonged to the "input" based on the type of stream it represented, versus the fields that were properly part of the general logic in read0/readevalloop/etc. I'm sure there's room for improvement, but my brain is a bit fried from combing over the code.
I also made a minor effort to differentiate between the case where readevalloop initiates and eval and is responsible for handling errors, versus the case where an intervening evaluation began due to an interrupt (i.e. while processing input events or a timer in a maybe_quit() somewhere). It's half-baked because I only implemented it for calls to maybe_quit() that appear in lread.c, just to see if I understood how I might make it work. In particular, at least for signals processed during those particular maybe_quit calls, An unhandled end-of-file error generated during one of those intervening evaluations will not be intercepted by readevalloop and mistaken for ending the current reader stream. Instead, all such spurious signals are passed down to the entry point of that activation of readevalloop (or read) and then.rethrown appropriately. That piece isn't really necessary, as any real solution will require overhauling the way interrupts are handled.
At any rate, I'm working on cutting down the patch. Pip asked that it be split into smaller chunks. I can do that, but if I don't think I'd want to guarantee that the project would necessarily even compile after applying a subset of the patches, even when applied in order. Is that acceptable?
Lynn