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

[Stefan Monnier]

>> That sounds rather ugly.  I'd rather first try and define precisely
>> what is we mean by "compilation in progress".
> That the byte compiler is active, and all symbols (bar nil) get
> positioned.

That is much too vague to be usable.  Which symbols are we
talking about?  What do we mean by "active"?

I think we need to describe it in terms that link the symbol with code
in bytecomp.el (not based on time, but based on function bytecomp-FOO
calling BAR calling BAZ ... touching that symbol).

Note that I also have no idea why we need to care about whether we're in
the compiled case or the non-compiled case, so maybe my questions are
"XY" style problems.

> An alternative might be to pass an &optional boolean argument meaning
> "preserve positions on symbols".

>From where to where should we pass that argument?
[ And yes, explicit arguments might be preferable unless the
  call-trace is too long for it to be practical or unless some of the
  functions along that call trace can't be changed easily to take such an
  extra argument.  ]

BTW, do you really mean "preserve" (meaning that the symbols were sympos
to start with)?

>> I see the same problem with:
>
>>       DEFVAR_LISP ("defining-symbol", Vdefining_symbol,
>>             doc: /* The symbol currently being defined by a defining form.
>>     This variable is bound in the read-eval-print loop and certain
>>     high-level functions in the byte compiler.  It is set to a value by
>>     functions and macros such as `defun', `defmacro', and `defvar'.  */);
>
>> Lots and lots of things can happen "during the definition" of a form,
>> including definition of lots of other forms.  So I think we'd need to
>> define much more precisely what you meant by "currently".
>> In addition, a definition is "intemporal" (it's declarative), so
>> "currently being defined" is almost like an oxymoron.
>
> By "currently", I mean that a defining form such as defun or defvar has
> commenced, but not yet terminated; its functions currently occupy stack
> frames.

So you mean we're inside `Fdefalias` or `Fdefvar_1`?

>> I'm trying to understand your code, but I clearly lack a high-level
>> overview of the approach you decided to takes, so I don't understand
>> what's going on there.
>
> Sorry about that.  A quick summary: defined symbols (and lambda) get
> positioned by the new reader function read-positioning-defined symbols.
> The new declare clause defining-symbol marks a macro such as defun or
> cl-defgeneric as a macro which defines such symbols.
>
> The conversion of these SWPs into position structures in doc strings
> happens at macro expansion time, through byte-run-posify-lambda-form.

So, IIUC

    (defmacro defun (name args docstring &rest body)
      (declare (defining-symbol 1))
      ...)

is akin to:

    (defmacro defun (name args docstring &rest body)
      (setq docstring (add-pos-to-docstring
                       (symbol-with-pos-pos name) docstring))
      ...)

?

>> If both, could you split it into two, then?
> I'm not sure that would be possible or sensible - both use a common
> approach.

So, IIUC a first part of the change was to make `load` use
`read-positioning-symbols` just like the compiler?

>> AFAICT doing it only for compiled functions should be significantly
>> simpler than for interpreted functions, so it would be a good
>> stepping stone.
> The work has already been done, and there is working code.  Just as a
> matter of interest, the branch runs the test suite without errors (not
> counting "expensive" tests ).

I'm not talking about a separate branch.  I'm talking about splitting
your changes into understandable commits.

>> On the cosmetic side, you have way too much code in `byte-run.el`.
>> I think most of this code can be moved elsewhere, e.g. somewhere where
>> backquote can be used
>
> Yes, I noticed this, too.  A lot of the bulk is for diagnostic functions
> for SWPs, and these can eventually be deleted.  Or possibly moved into a
> new file with-pos.el to be loaded before byte-run.el.

I don't like the idea of adding another file before byte-run.el.

> byte-run--posify-defining-symbol, the function with the extreme hand
> expansion of backquotes is used as a declare clause handler, and is
> needed by defun.  Hence it couldn't really be moved to after the loading
> of backquote.el.

I think you can simply wait to add the entry to
`macro-declarations-alist` until a later time, so the `defining-symbol`
thingies will be ignored during the early bootstrap and once we have
more infrastructure in place we can then register the handler on
`macro-declarations-alist`.

> There are some additional functions which batch-posify functions and
> variables defined before the posification mechanism is in place.  This
> must be done ASAP, for the benefit of backtraces in early bootstrap.

That complexifies the early bootstrap code.  I'd rather keep that code
simpler.  During early bootstrap, all those functions are interpreted
and I can't remember ever having difficulty tracking the origin of those
interpreted lambdas during early bootstrap, so I'm rather opposed to
such complexity just for the sake of maybe occasionally hypothetically
giving a bit of help to the 3 of us who have to deal with
those problems.

I see functions-with-pos as something mostly targeted at "end users" who
aren't expert enough to figure out the origin of anonymous functions by
looking at the disassembly of the bytecode and taking an educated guess.


        Stefan