Subject: bug#78898: Make read/readevalloop reentrant

From: Lynn Winebarger <[email-masked]>
Date: Tue, 15 Jul 2025 22:22:01 -0400

On Wed, Jul 9, 2025 at 6:57 AM Pip Cet <[email-masked]> wrote:
>
> "Lynn Winebarger" <[email-masked]> writes:
> > I'd be happy to take any hints on those two remaining edebug
> > regressions.  I have no idea what the difference of interest is
> > between those two and all the other similar looking tests is, in terms
> > of reliance on the behavior of
> > eval-region/eval-buffer/load-read-function.
>
> I think the difference is rather subtle, and it's not clear to me the
> old behavior, which edebug relies on, is actually better.
>
> The old readevalloop skips whitespace and comments before calling
> Vload_read_function.  The new code, AFAIT, only calls
> lread_skip_insignificant if use_read0.

That was on purpose.  Actually, there were a number of places I "poked
the bear" by simplifying things I did not see the point of to either
show it was unnecessary or find the undocumented interface promise.
Most of it was deep in the middle of transforming the code/data
structures though (and middle of the night, likely).  In particular,
I'd like to add a "syntax-object" lisp type and augment the reader to
track whitespace and comments in such objects so that I can implement
a "rewriting-pcase" efficiently in read0.  Rewriting the text of the
code means the comments associated with a piece of code should move
with the text, after all.  And since load-read-function *could* be an
arbitrary lisp function, there is no a priori reason to believe it
promises to skip whitespace and comments with no effect.  At least,
there was nothing documented to that effect.
Also, I'm not sure that edebug relies on that behavior so much as the
edebug test framework.

>
> The problem is that edebug assumes the point position when its
> Vload_read_function is called is actually the beginning of the form; it
> then uses
>
>     (end-of-defun)
>     (beginning-of-defun)
>
> to jump to what it thinks is the beginning of the defun.  But point was
> still in the previous defun (before the separating whitespace), so it's
> actually the beginning of the previous defun, so we fail to instrument
> the correct defun in edebug-tests-step-into-function.

Thanks for this.  I think that resolves all the known inconsistent
behaviors. Now I just have to take the final version and rebase it (or
is it "merge" it?) against Mattias's updates.  Oh, and use the patch I
provided in "bug#79001: Retain profile image files generated during
build" to get some concrete, statistically significant measurements of
the impact.

Lynn