Re: Circular records: how do I best handle them? (The new correct warning position branch now bootstraps in native compilation!)

[Alan Mackenzie <acm@muc.de>]

Hello, Stefan.

On Sun, Dec 26, 2021 at 15:35:35 -0500, Stefan Monnier wrote:
> >> Hmm... circularity is quite normal in data structures, yes.
> >> But presumably this is only applied to source code where circularity is
> >> very rare.  Could it be that you end up recursing in elements which
> >> actually aren't part of the source code (and hence can't have
> >> symbols-with-positions)?
> > I honestly don't know at the moment.

> I think it's worth the effort to try and track this down.  Maybe we can
> completely circumvent the problem.

I don't think there are any such cases.  I'll think it through fully,
some time.

> >> Also, I see in `byte-compile-strip-s-p-1` that you only look inside conses
> >> and vectors.  So I'm not sure what makes you say the recursion was in
> >> records since records are similar to vectors but aren't `vectorp` so
> >> AFAICT your code won't recurse into them.

> > byte-compile-strip-s-p-1 has been enhanced to handle records, too,
> > though I haven't committed that bit yet (along with quite a lot of other
> > amendments, too).

> Hmm... now that I think about it, you only generate
> symbols-with-positions (symposes) when byte-compiling, right?

Correct.

> And you can restrict this to the case where we byte-compile into a file
> (as opposed to the rare case where we just call `byte-compile`).

I suppose this could be done, but there's no need.  compile-defun isn't
that rare a function, and we want the correct warning messages from it.

> So the symposes can end up in 2 places:
> - in the .elc file: no need to strip the pos here, just make sure the
>   symbols get printed without their position.

The positions get stripped before the code is dumped to the .elc.

> - elsewhere: that's the problematic part because this only occurs where
>   the source code gets stealthy passed elsewhere, e.g. when a macro
>   calls (put ARG1 'foo ARG2) during the macro expansion (rather than
>   returning that chunk of code in the expansion).

This isn't a problem.  If it is a compiled macro doing this, the
positions will already be gone from the symbols.  If it is from an
uncompiled macro, XSYMBOL in Feval's subroutines does the Right Thing.

>   Here we don't have much control over where the symposes end up and I
>   don't think `byte-compile-strip-s-p` can help us (unless we call it
>   before passing the result to the macro, but I don't think that's
>   what we want to do).

> So where/why do we need `byte-compile-strip-s-p`?

It's now become macroexp-strip-symbol-position, so that it is always
loaded early, and there is no need for a duplicate function in
cl-macs.el any more.  There didn't seem to be a better place to put it.

It's used all over the place.  In eval-when/and-compile, it is used
before the evaluation.  It is used before dumping the byte compiled code
to the file.elc, and before passing this code to the native compiler.
Several (?most) of the byte-compile-file-form-... functions use it.
It's used in the newish keymap functions near the end of bytecomp.el, in
byte-compile-annotate-call-tree, etc.  Also in cl-define-compiler-macro,
and internal-macro-expand-for-load.  Additionally, also from Fput, to
prevent symbols with positions getting into symbol property lists.

> >> That's what we do elsewhere, yes, except that history taught us that
> >> a hash-table is a better choice to avoid scalability problems.
> >> Tho in your case you'd only need to keep the stack of objects inside of
> >> which you're currently recursing, so maybe a list is good enough.
> > I've tried the list approach (using memq to check for an already
> > processed cons/vector/record.  It fell flat on its face with
> > lisp/leim/ja-dic/ja-dec.el, which has a list with over 60,000 strings
> > in it.

> Oh, right, we have to add to the list all the conses rather than only
> the head conses, so you definitely want to use a hash-table.

Yes, I have to do this.  I am still debating whether just to do it
(which might slow things down quite a bit), or to do it in a
condition-case handler after the recursion has exceeded the 1,600
max-lisp-eval-depth.  I'm inclined towards the latter at the moment.

For other Lisp objects with a read syntax, such as char tables and
decorated strings, I intend to amend the reader just to output plain
symbols for them.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).