Subject: bug#67455: (Record source position, etc., in doc strings, and use this in *Help* and backtraces.)

From: Alan Mackenzie <[email-masked]>
Date: Wed, 27 Mar 2024 21:43:29 +0000

Hello, Stefan.

On Wed, Mar 27, 2024 at 08:22:27 -0400, Stefan Monnier wrote:
> > More precisely, to @dfn{posify} their  containing forms by writing the
> > position information into their doc strings.  We do this in the byte
> > compilation case, too.  The difference is that in the "load from source"
> > case we want to strip the SWP, in byte compilation, we don't.

> [ Nitpick: in byte-compilation, we also do.  We want to keep the SWPs
>   longer, but in the end we also want to strip them away necause we don't
>   want them in the resulting compiled code.  ]

In byte compilation we want to keeo the SWPs for AS LONG AS POSSIBLE.
In other cases, we want to keep them for AS SHORT A TIME AS POSSIBLE.

> > I don't think that is a good name.  The byte compiler has no business
> > setting "internal" variables for the posification processing.  Instead it
> > should announce it's running and expect the posification to respect that.
> > I think byte-compile-in-progress is a good name for this.

> AFAIK we want those SWPs stripped if and only if we're in a "load from
> source" case.  The compilation case is one of those where we don't want
> to strip them, but it's not the only one, so the compiler should not do
> anything w.r.t to that.  Instead it's the code that does the "load from
> source" which should set some indicator that stripping is requested.

I think it is better to regard the byte compilation as the special case.
Only in byte compilation do we want to preserve the SWPs on forms
getting posified.

> Also, I think as a general rule it's better for the caller to set
> a callee variable that controls how the callee behaves, rather than for the
> callee to check a caller variable to decide how to behave, because
> it's normal for the caller to "know about" its callee (after all, it's
> the caller which decides to call the callee), whereas it's not normal
> for the callee to know about specific callers (it creates undesirable
> dependencies).

Byte compilation is NOT calling loading from source.  We don't have a
caller/callee relationship here.  We are doing posification either from
byte compilation or from somewhere else.  It is analogous to testing
lexical binding.  Here, the variable is called lexical-binding; it is
not named after a particular activity to be carried out differently for
l-b and not l-b.

> >> Better yet: to avoid the problem of dynamic scope extending "too far"
> >> (i.e. accidentally applying to nested loads/evals/compile/...), you
> >> could put the relevant info into `macroexpand-all-environment`.
> >> [ That var is also dynamically bound, but we're already careful to
> >>   rebind it when needed so it doesn't apply to nested uses
> >>   of macroexpansion.  ]

> > That variable is only loaded in the 17th loaded Lisp file.  The new
> > facility should be working at the earliest stages of loading Lisp, as it
> > does at the moment.

> The earlier the better, in theory, but not at any cost.

No, the earlier the better, full stop.  The earlier an error happens in
bootstrapping, the more important it is to provide debugging support.
The new facilities were exceptionally helpful whilst debugging the
partially finished code.

> Having to write all that code within the very restrictive sublanguage
> available before subr.el and backquote.el is a cost I don't think
> justifies it.

The cost has already been paid, by me.  The cost of maintaining that
code will be small by comparison; byte-run--posify-defining-form is
_not_ all that difficult to understand and amend.

> If we *really* want that, then we should explore other avenues, such as
> keeping pre-macroexpanded versions of the files (for bootstrapping
> purposes) but generating those files from files where a more normal
> coding style can be used.
> [ Something similar to the ldefs-boot.el.  ]

Possibly - but that will also introduce complications.

> > Besides, macroexpand-all-environment is not
> > documented anywhere, what it is, what it's for, etc.

> Feel free to disregard my advice if you don't like it.
> I'm just pointing out that it's probably the tool which gives you the
> semantics you want.

OK.

> >> My crystal ball suggests that "currently" may be the wrong way to think
> >> about it: maybe instead of thinking of "when" (as in "during the
> >> definition of function FOO") what you're looking for might be "where"
> >> (as in "within the body of FOO").
> >> [ That's the same difference as the difference between dynamic and
> >>   static scoping.  ]
> > I'm having trouble understanding what you're saying, here.

> Is it because you don't understand the difference between dynamic
> scoping and static scoping, or because you don't see the relationship
> with that and your notion of "currently being defined"?

The latter, I think.  defining-symbol is entirely dynamically scoped.

> The above citation is in the context of my question about what you mean
> by "currently" in:

>        doc: /* The symbol currently being defined by a defining form.

> I personally don't really understand it, and AFAICT, you don't really
> understand it either because you haven't been able to describe it.

I understand it fully.  I'm puzzled by your failure to understand what
I've written.

The flow goes as follows:
(i) defining-symbol gets bound to nil in readevalloop_eager_eval_loop.
(ii) d-s gets setq'd to NAME in defun, defvar, cl-defgeneric, .....
  Usually, the setq has been generated by
  byte-run--posify-defining-form.
(iii) d-s gets used in byte-run-posify-doc-string, which writes its
  details to a new or existing doc string.

By "currently", I mean that d-s holds NAME between (ii) and (iii) and
until the process of defining the new form is complete.

> >> > Ideally, I would like to have bound defining-symbol inside defun.

> >>     (defmacro my-defun (name args &rest body)
> >>       `(cl-macrolet ((defining-symbol () '',name))
> >>         (defun ,name ,args ,@body)))

> >>     (my-defun my-foo (x) (list x (defining-symbol)))

> >>     (symbol-function 'my-foo)
> >>     ==> #f(lambda (x) [t] (list x 'my-foo))

> >> `cl-macrolet` uses `macroexpand-all-environment` for that.

> > cl-macs gets loaded far too late for such an approach to be useful.

> That's not really relevant since we're just trying to understand what
> you mean by "currently".  What is relevant is whether it gives
> the intended semantics.

I'm convinced it does.  Can you suggest a scenario where the
defining-symbol mechanism (outlined above) might fail?

> But if you insist, here's the equivalent version without `cl-macs`
> (using the same underlying technique as used by `cl-macrolet`):

>     (defmacro my-defun (name args &rest body)
>       (macroexpand-all
>        `(defun ,name ,args ,@body)
>        (cons
>         (cons 'defining-symbol (lambda () `',name))
>         macroexpand-all-environment)))

Thanks.

> >> Do you have rough numbers comparing the cost of `read`,
> >> `read-positioning-symbols`, and `read-positioning-DEFINED-symbols`?
> > No, but they will be very close to eachother (and very cheap)

> Then I think we should use `read-positioning-symbols`, which
> requires fewer code changes.

It won't.  Required would be Lisp code to determine whether a particular
SWP needs to be stripped or not.  This is not going to be simple.  It is
likely to be about as complicated as the existing enhancements to read0.

> >> Also, IIUC you don't have a separate phase to strip the SWPs when
> >> loading from source, but instead you strip them as you "consume" their
> >> info during macroexpansion.  If so, how/when/where do you strip the
> >> false positives that may occur inside quoted data or in code like:

> >>     (defmacro foo (lambda bar) ...)
> >>     (defmacro foo (defun bar) ...)

> >>     (let* ((lambda foo)
> >>            (defun bar))
> >>       ...)

> > There's a pcase arm right at the end of macroexp--expand-all which strips
> > SWPs of their positions.  Recursing through macroexp--all-forms will
> > eventually hit this pcase arm for these lambdas.

> Ah, so it's like a "strip phase" but "fused" into the macroexpansion phase.

I suppose so.

> >> Not at all.  Those will remain without position, but only in
> >> `src/bootstrap-emacs`.
> > This would be a Bad Thing.

> But your current code in byte-run.el is a Bad Thing as well.

What, precisely, do you find bad about it?  It may be possible to improve
it without wholesale redesign.

> It's all a question of trade-offs :-(

> >> In the real `src/emacs` they will get the position because they'll come
> >> from the `.el[cn]` file and by the time we get compile those files
> >> `macro-declarations-alist` will be fully populated.
> > The understanding we reached in November was that loading from source
> > files would be handled, too.

> I'm not suggesting to drop support for lambdas loaded from source.
> I'm saying we don't need to support it for the first N files loaded into
> `src/emacs-bootstrap`.

You're suggesting dropping support for many source files, where that
support is most needed.

You're suggesting introducing awkward special cases where the code won't
work.  As currently implemented, the code DOES work.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).