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

From: Stefan Monnier <[email-masked]>
Date: Wed, 27 Mar 2024 08:22:27 -0400

> 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.  ]

> 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.

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).

>> 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.  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.

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.  ]

> 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.

>> 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 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.

>> > 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.

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)))

>> 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.

>> 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.

>> 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.
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`.

        Stefan