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

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

[Alan Mackenzie]

Hello, Emacs.

The reasons for this bug and the ways to solve it have been discussed
extensively in the thread for bug#66750.

We will, in functions' and macros' doc strings record the file name and
position of the source code, and possibly other items, in a machine
parseable format (which has yet to be decided).

This is particularly intended for lambda functions, which currently
appear in backtraces and *Help* buffers with insufficient information to
identify them.  These displays will be enhanced to identify these lambda
functions satisfactorally.

The functions in Emacs which currently use doc strings will be modified
so as not to be negatively affected by the new information written into
them.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Alan Mackenzie]

Hello, Stefan and Eli.

On Sun, Nov 26, 2023 at 14:30:25 +0000, Alan Mackenzie wrote:
> Hello, Emacs.

> The reasons for this bug and the ways to solve it have been discussed
> extensively in the thread for bug#66750.

> We will, in functions' and macros' doc strings record the file name and
> position of the source code, and possibly other items, in a machine
> parseable format (which has yet to be decided).

> This is particularly intended for lambda functions, which currently
> appear in backtraces and *Help* buffers with insufficient information to
> identify them.  These displays will be enhanced to identify these lambda
> functions satisfactorally.

> The functions in Emacs which currently use doc strings will be modified
> so as not to be negatively affected by the new information written into
> them.

I've committed the first stage of this implementation in the new git
branch feature/positioned-lambdas.

It creates for compiled functions (but not yet for interpreted ones) doc
strings starting with a specially formatted string containing the
defining symbol, the source file name, the position of the defining
symbol in the file, and the position of the lambda (should we be in one)
in the file.

To instruct a defining macro to set a defining-symbol, it is only
necessary to add a clause like (defining-symbol 1) or (defining-symbol
(if (consp struct) (car struct) struct)) to the macro's declare form.
This is how defun now works.  (Thank you Stefan for the handy "cl.el
feature" in byte-run--parse-declarations.  ;-)

I scanned our Lisp code for debug-spec &define keywords, and added
defining-symbol declare clauses to (most of) the ones I found.

As yet, no use is made of the new position information (beyond stripping
it off for other uses of the doc string).

> -- 
> Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I've committed the first stage of this implementation in the new git
> branch feature/positioned-lambdas.

Haven't had enough time to look closely, so just some quick early comment.

> It creates for compiled functions (but not yet for interpreted ones) doc
> strings starting with a specially formatted string containing the
> defining symbol, the source file name,

Why include the file name?  Presumably we will rely on a `(FILE . POS)`
to find that docstring so including FILE in there is rather redundant
(and in addition to that, this file name can be wrong if it's absolute
since files often get moved between compilation and use).

> the position of the defining symbol in the file, and the position of
> the lambda (should we be in one) in the file.

Why two positions?
Why not use the same position info as used in `byte-compile-warn-x`?

> To instruct a defining macro to set a defining-symbol, it is only
> necessary to add a clause like (defining-symbol 1)

Sounds good.
[ Hopefully we can eventually use that info for `font-lock` so we can
  get rid of ad-hoc rules for `defun` and friends.  It would require an
  additional piece of info (not only the position but also the kind of
  definition, to distinguish `defun` from `defvar`).  ]

> or (defining-symbol (if (consp struct) (car struct) struct))

[ Haven't seen that yet.  ]


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

Thanks for such a quick response.

On Mon, Dec 04, 2023 at 13:33:25 -0500, Stefan Monnier wrote:
> > I've committed the first stage of this implementation in the new git
> > branch feature/positioned-lambdas.

> Haven't had enough time to look closely, so just some quick early comment.

> > It creates for compiled functions (but not yet for interpreted ones) doc
> > strings starting with a specially formatted string containing the
> > defining symbol, the source file name,

> Why include the file name?  Presumably we will rely on a `(FILE . POS)`
> to find that docstring so including FILE in there is rather redundant
> (and in addition to that, this file name can be wrong if it's absolute
> since files often get moved between compilation and use).

No, the (FILE . POS) is the .elc file, the one I'm putting into the new
info is the source file.  But you're right about an absolute name (as it
currently is) not being the Right Thing.  A lot of Emacs users will be
using binaries compiled by somebody else on a different machine, so an
absolute name is unhelpful, as well as divulging information about the
builder's file structure.  Maybe it should be
lisp/emacs-lisp/bytecomp.el rather than
/home/acm/emacs/emacs.git/sub-master-a/lisp/emacs-lisp/bytecomp.el.  At
least it would be shorter.

> > the position of the defining symbol in the file, and the position of
> > the lambda (should we be in one) in the file.

> Why two positions?
> Why not use the same position info as used in `byte-compile-warn-x`?

I'm not sure, yet, but I suspect both positions will be useful.  If not,
it's trivial to change the contents of the new info, at least it is now.

> > To instruct a defining macro to set a defining-symbol, it is only
> > necessary to add a clause like (defining-symbol 1)

> Sounds good.
> [ Hopefully we can eventually use that info for `font-lock` so we can
>   get rid of ad-hoc rules for `defun` and friends.  It would require an
>   additional piece of info (not only the position but also the kind of
>   definition, to distinguish `defun` from `defvar`).  ]

> > or (defining-symbol (if (consp struct) (car struct) struct))

> [ Haven't seen that yet.  ]

Have a look at cl-defgeneric and cl-defmethod in cl-generic.el.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> Why include the file name?  Presumably we will rely on a `(FILE . POS)`
>> to find that docstring so including FILE in there is rather redundant
>> (and in addition to that, this file name can be wrong if it's absolute
>> since files often get moved between compilation and use).
> No, the (FILE . POS) is the .elc file, the one I'm putting into the new
> info is the source file.

Until now, Emacs has always lived with the `.elc` file names (e.g. in
`load-history`) and managed to find the corresponding `.el` file (when
you try to jump to the source via `M-.` or by clicking in `C-h f`).
It comes with its share of problems, but we've learned to live with them.

Is there any reason you think this new functionality should go through
the extra trouble to record the `.el` name (and thus develop its own
set of workarounds for the cases where the `.el` doesn't live where we
think it should)?

>> > the position of the defining symbol in the file, and the position of
>> > the lambda (should we be in one) in the file.
>> Why two positions?
>> Why not use the same position info as used in `byte-compile-warn-x`?
> I'm not sure, yet, but I suspect both positions will be useful.

Can you give an example scenario?

>> > or (defining-symbol (if (consp struct) (car struct) struct))
>> [ Haven't seen that yet.  ]
> Have a look at cl-defgeneric and cl-defmethod in cl-generic.el.

Ah, got it.  I like the way you can refer to args by name.
So I guess all the (defining-symbol 1) could similarly be replaced with
(defining-symbol THE-ARG-NAME).

[ I also notice that this extension is somewhat incompatible with the
  use I proposed for `font-lock`.  :-(  ]

[ As you noted, this info is also related to the `&define` debug spec.
  I wish we could unify that information.  ]


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

On Mon, Dec 04, 2023 at 16:56:41 -0500, Stefan Monnier wrote:
> >> Why include the file name?  Presumably we will rely on a `(FILE . POS)`
> >> to find that docstring so including FILE in there is rather redundant
> >> (and in addition to that, this file name can be wrong if it's absolute
> >> since files often get moved between compilation and use).
> > No, the (FILE . POS) is the .elc file, the one I'm putting into the new
> > info is the source file.

> Until now, Emacs has always lived with the `.elc` file names (e.g. in
> `load-history`) and managed to find the corresponding `.el` file (when
> you try to jump to the source via `M-.` or by clicking in `C-h f`).
> It comes with its share of problems, but we've learned to live with them.

Yes.  I've come to the same conclusion in the last hour or so that, no
doubt, early Emacs hackers reached forty years ago.  We can find the
..elc full filename from load-history (via symbol-file), but there's no
more useful way of finding the corresponding source than removing the
'c' from the end of foo.elc.  And if it's not there, it's nowhere.

> Is there any reason you think this new functionality should go through
> the extra trouble to record the `.el` name (and thus develop its own
> set of workarounds for the cases where the `.el` doesn't live where we
> think it should)?

Not any more, having thought it through.

Maybe, though, it would be useful in some circumstances to record the
buffer the definition was loaded from if it's not a file.  But it's
getting rather late here, so I haven't thought this through at all.

> >> > the position of the defining symbol in the file, and the position of
> >> > the lambda (should we be in one) in the file.
> >> Why two positions?
> >> Why not use the same position info as used in `byte-compile-warn-x`?
> > I'm not sure, yet, but I suspect both positions will be useful.

> Can you give an example scenario?

Something about a lambda form, when we need to find the lambda itself,
but also its containing form.  Sorry I can't be more definite, yet.
When I start hacking out the code to use all this information, it will
become clearer what we actually need.  It's very easy to change at this
stage.

> >> > or (defining-symbol (if (consp struct) (car struct) struct))
> >> [ Haven't seen that yet.  ]
> > Have a look at cl-defgeneric and cl-defmethod in cl-generic.el.

> Ah, got it.  I like the way you can refer to args by name.
> So I guess all the (defining-symbol 1) could similarly be replaced with
> (defining-symbol THE-ARG-NAME).

Yes, I hadn't really thought of that.  But (defining-symbol 1) matches
similar forms like (docstring 3) which are used somewhere.

> [ I also notice that this extension is somewhat incompatible with the
>   use I proposed for `font-lock`.  :-(  ]

> [ As you noted, this info is also related to the `&define` debug spec.
>   I wish we could unify that information.  ]

Ah, the debug spec; yet another difficult domain specific language.  ;-(

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Maybe, though, it would be useful in some circumstances to record the
> buffer the definition was loaded from if it's not a file.  But it's
> getting rather late here, so I haven't thought this through at all.

I think if it comes from a buffer (and one that's not associated with
a file, to boot), it's usually not byte-compiled, so I suspect you'll be
better off confining that handling to the interpreted case.

>> >> > the position of the defining symbol in the file, and the position of
>> >> > the lambda (should we be in one) in the file.
>> >> Why two positions?
>> >> Why not use the same position info as used in `byte-compile-warn-x`?
>> > I'm not sure, yet, but I suspect both positions will be useful.
>> Can you give an example scenario?
> Something about a lambda form, when we need to find the lambda itself,
> but also its containing form.  Sorry I can't be more definite, yet.

I'm having a hard time imagining why you'd need two positions.  Instead,
I want to go to *the* position, which should be "the best we can come up with".
And that's also what `byte-compile-warn-x` wants and it has access to
the same information.

IOW if you think you can do better for lambdas-with-pos, then please try
and retro fit it into `byte-compile-warn-x`.

>> Ah, got it.  I like the way you can refer to args by name.
>> So I guess all the (defining-symbol 1) could similarly be replaced with
>> (defining-symbol THE-ARG-NAME).
> Yes, I hadn't really thought of that.  But (defining-symbol 1) matches
> similar forms like (docstring 3) which are used somewhere.

(defining-symbol N) is nice as well.

> Ah, the debug spec; yet another difficult domain specific language.  ;-(

Yup.  It's even worse, because at this stage I don't think anyone
truly understands its semantics.

[ I just wish we had a "similar" DSL that served /all/ those purposes,
  i.e. would let the macro writer express where the docstring is (if any),
  where the defining names are, how to indent each part, how to instrument
  each part, ...
  And of course, with a well-defined and well-understood semantics,
  a simple implementation (that works efficiently for all those uses),
  and a simple&concise syntax.
  Oh and while at it, it would also be used during macro-expansion to
  provide meaningful error messages when the macro is used
  incorrectly.
  Clearly, [Fortifying macros](https://dl.acm.org/doi/10.1145/1863543.1863577)
  can provide useful inspiration.  ]


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

On Mon, Dec 04, 2023 at 13:33:25 -0500, Stefan Monnier wrote:
> > I've committed the first stage of this implementation in the new git
> > branch feature/positioned-lambdas.

> Haven't had enough time to look closely, so just some quick early comment.

.....

I'm making steady, if not rapid, progress on the use of doc strings to
store position information.  There have been several unexpected
problems (which is only to be expected), all of which, bar one, I've
been able to solve.

The problem that has thrown me is the use of the doc string in oclosures
for other purposes.  For example, in position 2 of a lambda form,
appears something like

    (:documentation 'oclosure-accessor)

..  My current code is expecting, on encountering (:documentation ...),
for the cadr to be a string, onto which it can add a concat form which
will prefix the position information.

A solution to this problem would be to move the above symbol to element
2 of the list, something like

    (:documentation nil 'oclosure-accessor)

, so that my new code could place its information in the now vacant
position 1, giving something like

    (:documentation ";POS\36\0\0\0 [ .... ]\n" 'oclosure-accessor)

..  What do you think of this idea, and have you any better ideas for a
solution to the problem?

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> The problem that has thrown me is the use of the doc string in oclosures
> for other purposes.  For example, in position 2 of a lambda form,
> appears something like
>
>     (:documentation 'oclosure-accessor)

Hmm...

> ..  My current code is expecting, on encountering (:documentation ...),
> for the cadr to be a string,

`:documentation` is supposed to be followed by an expression (whose
evaluation should usually return a string, although indeed we abuse it
in the case of OClosures to return a symbol instead).

Regardless of OClosures, the `cadr` of a `:documentation` will very
rarely be a mere string since you don't need `:documentation` when the
docstring can be written as a literal.

> onto which it can add a concat form which
> will prefix the position information.

For the non-OClosure case you should be able to use the equivalent of:

    (:documentation
     (concat <YOURINFOHERE> <THEORIGINALEXPRESSIONHERE>))

is that what you mean?

> A solution to this problem would be to move the above symbol to element
> 2 of the list, something like
>
>     (:documentation nil 'oclosure-accessor)

Here you're talking about the source code, right?
I think the question is rather what representation to use in the values:
OClosures store their type name (symbol) in the slot that normally holds
the docstring, so I think the best short term answer is to say that
OClosures aren't supported by your new feature.

OClosures are a mix of functions and structs, so while it would be
nice to make them enjoy your new feature, I think it's perfectly OK
to say that in this respect they're more like structs than like
functions.
Also they come with more introspection features than normal
functions, so it's usually easier to figure out where they come from
(you can find their type, and then look for the `oclosure-lambda`s
which built OClosures of this type.  For most OClosure types, there's
only one such lambda anyway).

> , so that my new code could place its information in the now vacant
> position 1, giving something like
>
>     (:documentation ";POS\36\0\0\0 [ .... ]\n" 'oclosure-accessor)
>
> ..  What do you think of this idea, and have you any better ideas for a
> solution to the problem?

If it works it might be a good option.  I don't know how this turns into
when we build the corresponding bytecode object, so it's hard to judge.


        Stefan

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

[Alan Mackenzie]

Hello, Eli and Stefan.

On Sun, Nov 26, 2023 at 14:31:02 +0000, GNU bug Tracking System wrote:
> Thank you for filing a new bug report with debbugs.gnu.org.

[ .... ]

I've just pushed a large commit to feature/positioned-lambdas, a work
in progress commit for bug#67455, putting source position information at
the start of doc strings.  master was merged into it just before the
commit.

The main topic of the commit is putting position information into
interpreted functions, and into lambda forms created at run time.

Still missing is position information for defvars and defconsts, along
with the same for cl-defmethods (which are complicated because the doc
string has no fixed position).  Also missing is the handling of
Oclosures.  There are also some diagnostic functions still in
byte-run.el, as they are still useful.

As yet, I'm not doing anything with this information, though I
anticipate that will be quite easy.  

This commit, although it works, is unfinished, and still contains quite
a few of my private change annotations, and lots of temporary changes to
enable useful backtraces to be produced.  make check runs without any
errors.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I've just pushed a large commit to feature/positioned-lambdas, a work
> in progress commit for bug#67455, putting source position information at
> the start of doc strings.  master was merged into it just before the
> commit.

I barely started to look at the code, but regarding the following hunk:

    diff --git a/lisp/emacs-lisp/backquote.el b/lisp/emacs-lisp/backquote.el
    index 6917128d70a..0d4681bc979 100644
    --- a/lisp/emacs-lisp/backquote.el
    +++ b/lisp/emacs-lisp/backquote.el
    @@ -172,6 +172,30 @@ backquote-process
           (backquote-delay-process s (1- level))))
        ((eq (car s) backquote-backquote-symbol)
           (backquote-delay-process s (1+ level)))
    +   ;; Process a (lambda ...) form specially, since otherwise the
    +   ;; lambda symbol would get separated from its introductory (,
    +   ;; preventing this processing from being done elsewhere in macro
    +   ;; expansion.
    +   ((and (eq (car s) 'lambda)
    +         (symbol-with-pos-p (car s))
    +         (listp (car-safe (cdr s))))
    +    (let ((kdr (backquote-process (cdr s) level))
    +          (lambda-pos (symbol-with-pos-pos (car s)))
    +          )
    +      (if (null byte-compile-in-progress)
    +          (setcar s 'lambda))           ; Strip the position.
    +      (cond
    +       ((= (car kdr) 0)
    +        (cons (car kdr)
    +              (list 'quote
    +                    (byte-run-posify-lambda-form
    +                     (cons (car s) (car (cdr (cdr kdr)))) ; Two cdr's to strip 'quote.
    +                     lambda-pos))))
    +       (t
    +        (cons 1
    +              (list 'byte-run-posify-lambda-form
    +                    (list 'cons (list 'quote (car s)) (cdr kdr))
    +                    lambda-pos))))))
        (t
         (let ((rest s)
              item firstlist list lists expression)

- Testing `byte-compile-in-progress` can't be right.  Do you to test
  whether the result of this backquote will be byte-compiled or do you
  really mean to test whether this backquote happens to be executed
  during compilation (which could be because the compiler ends up
  loading code while executing `eval-when-compile` or `require`)?

- My gut tells me that changing backquote can't be right.
  (lambda (f) ..) *can* appear within a backquote without it being an
  actual lambda expression.
  What alternatives have you considered?


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

Thanks for looking at my patch!

I've got a version almost ready which actually does something, namely
prefixes "anonymous" lines of a backtrace with the name of the defining
symbol, like {foo} .  It'll soon be time to start seriously thinking
about what information ought to go there for the live version.

Unfortunately, I've still got two blocking issues to resolve - one is a
change in functionality to Fbare_symbol, which I've reported as a bug;
the other is some new code merged in from master, a with-eval-after-load
clause in pcase.el which causes havoc in my build.

Still, progress is being made.

On Sat, Mar 09, 2024 at 16:36:57 -0500, Stefan Monnier wrote:
> > I've just pushed a large commit to feature/positioned-lambdas, a work
> > in progress commit for bug#67455, putting source position information at
> > the start of doc strings.  master was merged into it just before the
> > commit.

> I barely started to look at the code, but regarding the following hunk:

>     diff --git a/lisp/emacs-lisp/backquote.el b/lisp/emacs-lisp/backquote.el
>     index 6917128d70a..0d4681bc979 100644
>     --- a/lisp/emacs-lisp/backquote.el
>     +++ b/lisp/emacs-lisp/backquote.el
>     @@ -172,6 +172,30 @@ backquote-process
>            (backquote-delay-process s (1- level))))
>         ((eq (car s) backquote-backquote-symbol)
>            (backquote-delay-process s (1+ level)))
>     +   ;; Process a (lambda ...) form specially, since otherwise the
>     +   ;; lambda symbol would get separated from its introductory (,
>     +   ;; preventing this processing from being done elsewhere in macro
>     +   ;; expansion.
>     +   ((and (eq (car s) 'lambda)
>     +         (symbol-with-pos-p (car s))
>     +         (listp (car-safe (cdr s))))
>     +    (let ((kdr (backquote-process (cdr s) level))
>     +          (lambda-pos (symbol-with-pos-pos (car s)))
>     +          )
>     +      (if (null byte-compile-in-progress)
>     +          (setcar s 'lambda))           ; Strip the position.
>     +      (cond
>     +       ((= (car kdr) 0)
>     +        (cons (car kdr)
>     +              (list 'quote
>     +                    (byte-run-posify-lambda-form
>     +                     (cons (car s) (car (cdr (cdr kdr)))) ; Two cdr's to strip 'quote.
>     +                     lambda-pos))))
>     +       (t
>     +        (cons 1
>     +              (list 'byte-run-posify-lambda-form
>     +                    (list 'cons (list 'quote (car s)) (cdr kdr))
>     +                    lambda-pos))))))
>         (t
>          (let ((rest s)
>               item firstlist list lists expression)

> - Testing `byte-compile-in-progress` can't be right.  Do you to test
>   whether the result of this backquote will be byte-compiled or do you
>   really mean to test whether this backquote happens to be executed
>   during compilation (which could be because the compiler ends up
>   loading code while executing `eval-when-compile` or `require`)?

Quite simply, during compilation, all symbols (except nil) get read with
position, so to strip their positions here would be wrong.  Outside of
compilation (an evaluation of a defun, for example), only defined
symbols get positioned, and these symbols with position would interfere
with Emacs's working.  So they get stripped of their positions as soon
as possible after the info has been transferred to the doc string.  For
example, SWPs cannot be dumped by the portable dumper.

> - My gut tells me that changing backquote can't be right.

I tend to agree.  I put the code into backquote-process when having
problems with things like:

       (mapatoms #'(lambda (,(car spec)) ,@body)

in cl-macs.el, where it's impossible to know where the doc string (if
any) is until after the expansion of the backquotes, or even at run time
(as here).  In the latter case, rather than "posifying" the doc string
at macro expansion time, we have to generate code to do it at run time.

>   (lambda (f) ..) *can* appear within a backquote without it being an
>   actual lambda expression.
>   What alternatives have you considered?

Not a lot of them, as yet.  Maybe testing for (function (lambda ...))
would be safer.  But I think I should try and find some other way of
solving these problems than altering backquote.el, which should be
easier now that the code is basically working.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I've got a version almost ready which actually does something, namely
> prefixes "anonymous" lines of a backtrace with the name of the defining
> symbol, like {foo} .  It'll soon be time to start seriously thinking
> about what information ought to go there for the live version.

Cool!

>> - Testing `byte-compile-in-progress` can't be right.  Do you to test
>>   whether the result of this backquote will be byte-compiled or do you
>>   really mean to test whether this backquote happens to be executed
>>   during compilation (which could be because the compiler ends up
>>   loading code while executing `eval-when-compile` or `require`)?
>
> Quite simply, during compilation, all symbols (except nil) get read with
> position, so to strip their positions here would be wrong.

This isn't quite right: during compilation, some code is read with
positions (the code that we will compile), but some code is read in the
normal way (the code we load for the purpose of running).
The distinction is important.

>> - My gut tells me that changing backquote can't be right.
>
> I tend to agree.  I put the code into backquote-process when having
> problems with things like:
>
>        (mapatoms #'(lambda (,(car spec)) ,@body)
>
> in cl-macs.el, where it's impossible to know where the doc string (if
> any) is until after the expansion of the backquotes, or even at run time
> (as here).  In the latter case, rather than "posifying" the doc string
> at macro expansion time, we have to generate code to do it at run time.

Hmm... here what you call "run time" is really some later
macro-expansion, right?  The `lambda` symbol comes from the first
macro-expansion (ME1), but the docstring comes from the second (ME2).

IIUC the problem you face is that you want to get the function's
position info from the `lambda` symbol, which here would be wrong (even if we
try to preserve it long enough), is that it?

[ Tho, in more complex cases it becomes debatable whether the function's
  position should point to the position corresponding to ME1 or to that
  of ME2.  ]

More generally: what goes wrong in the above example if you just treat
that as a list of symbol (stripping them all of their position info).
AFAICT when *that* macro is expanded (i.e. ME2) you'll presumably get
code like

    (mapatoms #'(lambda (FOO/p) (DO/p SOME/p (THING/p))))

right?  [ where "/p" means that the symbol has a sympos.  ]
Isn't that sufficient info to add a docstring with position?

>>   (lambda (f) ..) *can* appear within a backquote without it being an
>>   actual lambda expression.
>>   What alternatives have you considered?
> Not a lot of them, as yet.  Maybe testing for (function (lambda ...))
> would be safer.

No matter how many extra tests you add to reduce the frequency, you're
fundamentally adding a bug :-(


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

On Sun, Mar 10, 2024 at 13:19:03 -0400, Stefan Monnier wrote:
> > I've got a version almost ready which actually does something, namely
> > prefixes "anonymous" lines of a backtrace with the name of the defining
> > symbol, like {foo} .  It'll soon be time to start seriously thinking
> > about what information ought to go there for the live version.

> Cool!

I've finally got something to show.  I've just committed a merge and a
fix for it to branch feature/positioned-lambdas at savannah.

With this Emacs running, type the following into *scratch*

    (defun foo () "foo doc" (lambda (bar) "lambda doc" (car bar)))

..  Either evaluate this or byte compile it with compile-defun.  Then do

    M-: (funcall (foo) 'baz)

..  This will produce a backtrace like:

Debugger entered--Lisp error: (wrong-type-argument listp baz)
  car(baz)
  {foo} #f(compiled-function (bar) "lambda doc" #<bytecode -0x14ae78a46439bbc>)(baz)
  funcall({foo} #f(compiled-function (bar) "lambda doc" #<bytecode -0x14ae78a46439bbc>) baz)
  (progn (funcall (foo) 'baz))
  eval((progn (funcall (foo) 'baz)) t)
  elisp--eval-last-sexp(nil)
  {eval-last-sexp} #f(compiled-function () #<bytecode -0x1e8241efdb3d2890>)()
  eval-last-sexp(nil)
  funcall-interactively(eval-last-sexp nil)
  command-execute(eval-last-sexp)

..  Note the {eval-last-sexp} and {foo} on the anonymous functions.

:-)

> >> - Testing `byte-compile-in-progress` can't be right.  Do you to test
> >>   whether the result of this backquote will be byte-compiled or do you
> >>   really mean to test whether this backquote happens to be executed
> >>   during compilation (which could be because the compiler ends up
> >>   loading code while executing `eval-when-compile` or `require`)?

> > Quite simply, during compilation, all symbols (except nil) get read with
> > position, so to strip their positions here would be wrong.

> This isn't quite right: during compilation, some code is read with
> positions (the code that we will compile), but some code is read in the
> normal way (the code we load for the purpose of running).
> The distinction is important.

OK, I wasn't really counting code that we load as "during compilation",
but I take the point.

> >> - My gut tells me that changing backquote can't be right.

> > I tend to agree.  I put the code into backquote-process when having
> > problems with things like:

> >        (mapatoms #'(lambda (,(car spec)) ,@body)

> > in cl-macs.el, where it's impossible to know where the doc string (if
> > any) is until after the expansion of the backquotes, or even at run time
> > (as here).  In the latter case, rather than "posifying" the doc string
> > at macro expansion time, we have to generate code to do it at run time.

> Hmm... here what you call "run time" is really some later
> macro-expansion, right?  The `lambda` symbol comes from the first
> macro-expansion (ME1), but the docstring comes from the second (ME2).

Yes.  I often get confused between lots of different macro expansion
times, compile time and run time.  It's a lot easier in C.  ;-)

> IIUC the problem you face is that you want to get the function's
> position info from the `lambda` symbol, which here would be wrong (even if we
> try to preserve it long enough), is that it?

Something like that.  The lambda's position currently gets preserved in
the generated code so that ME2 can use it.

> [ Tho, in more complex cases it becomes debatable whether the function's
>   position should point to the position corresponding to ME1 or to that
>   of ME2.  ]

The code currently preserves both positions.  :-)  But only one buffer
name.

My latest thoughts on that are perhaps two file names (relative to the
Emacs top directory) would be better than one buffer name.  Then I could
put buttons on the backtrace display which on being clicked would open
either of the source files at the right position.

> More generally: what goes wrong in the above example if you just treat
> that as a list of symbol (stripping them all of their position info).
> AFAICT when *that* macro is expanded (i.e. ME2) you'll presumably get
> code like

>     (mapatoms #'(lambda (FOO/p) (DO/p SOME/p (THING/p))))

> right?  [ where "/p" means that the symbol has a sympos.  ]
> Isn't that sufficient info to add a docstring with position?

It's the lambda which has a position rather than the expanded bits from
ME2.

> >>   (lambda (f) ..) *can* appear within a backquote without it being an
> >>   actual lambda expression.
> >>   What alternatives have you considered?
> > Not a lot of them, as yet.  Maybe testing for (function (lambda ...))
> > would be safer.

> No matter how many extra tests you add to reduce the frequency, you're
> fundamentally adding a bug :-(

Yes.  I'll see what I can do to remove that extra code from
backquote-process.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> >> - Testing `byte-compile-in-progress` can't be right.  Do you to test
>> >>   whether the result of this backquote will be byte-compiled or do you
>> >>   really mean to test whether this backquote happens to be executed
>> >>   during compilation (which could be because the compiler ends up
>> >>   loading code while executing `eval-when-compile` or `require`)?
>
>> > Quite simply, during compilation, all symbols (except nil) get read with
>> > position, so to strip their positions here would be wrong.
>
>> This isn't quite right: during compilation, some code is read with
>> positions (the code that we will compile), but some code is read in the
>> normal way (the code we load for the purpose of running).
>> The distinction is important.
>
> OK, I wasn't really counting code that we load as "during compilation",
> but I take the point.

The point is that `byte-compile-in-progress` will be non-nil during
those loads, so you can't use this variable to get the information you need.

>> More generally: what goes wrong in the above example if you just treat
>> that as a list of symbol (stripping them all of their position info).
>> AFAICT when *that* macro is expanded (i.e. ME2) you'll presumably get
>> code like
>
>>     (mapatoms #'(lambda (FOO/p) (DO/p SOME/p (THING/p))))
>
>> right?  [ where "/p" means that the symbol has a sympos.  ]
>> Isn't that sufficient info to add a docstring with position?
>
> It's the lambda which has a position rather than the expanded bits from
> ME2.

Hmm... then I misunderstand something.  How can the `lambda` have
a position if you don't include any special treatment of backquote?
AFAICT the `lambda` in the result of ME1 should not include position
information because at that time we don't know that it will be used to
build code rather than be some element of a normal list.

And how come the rest doesn't have position information?


        Stefan

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

[Alan Mackenzie]

Hello again, Stefan.

On Sun, Mar 10, 2024 at 13:19:03 -0400, Stefan Monnier wrote:

[ .... ]

> >> - My gut tells me that changing backquote can't be right.

> > I tend to agree.  I put the code into backquote-process when having
> > problems with things like:

> >        (mapatoms #'(lambda (,(car spec)) ,@body)

[ .... ]

> >>   (lambda (f) ..) *can* appear within a backquote without it being an
> >>   actual lambda expression.
> >>   What alternatives have you considered?
> > Not a lot of them, as yet.  Maybe testing for (function (lambda ...))
> > would be safer.

> No matter how many extra tests you add to reduce the frequency, you're
> fundamentally adding a bug :-(

Well, macroexp--expand-all has treated (function (lambda ...)) as a
function long before I started on this project in November.  Would
trusting the same thing in backquote-process be any more dangerous?

Anyway, I've started looking at getting that extra code out of
backquote.el.  Simply commenting it out produces build time errors, which
shows that it's not totally redundant.

How about adding (an) extra arm(s) to the large pcase in
macroexp--expand-all which would recognise backquote's output for
"evaluated" lambdas.

What we get back from backquote looks like:

(cons 'lambda (cons plain-args body)) or
(list 'lambda args)) or maybe one or two other forms.

So I could extend such code fragments to add posification code quite
easily.  I'm not sure it'd be more elegant than what's currently in
backquote.el, though.  I think I'll give it a try.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> No matter how many extra tests you add to reduce the frequency, you're
>> fundamentally adding a bug :-(
>
> Well, macroexp--expand-all has treated (function (lambda ...)) as a
> function long before I started on this project in November.

Of course it does, and that's correct, because by definition the
argument to `macroexp--expand-all` must be source code expression.

> Would trusting the same thing in backquote-process be any
> more dangerous?

Yup, because backquote has no guarantee that the code it must produce is
one that will build a source code expression.

> How about adding (an) extra arm(s) to the large pcase in
> macroexp--expand-all which would recognise backquote's output for
> "evaluated" lambdas.
>
> What we get back from backquote looks like:
>
> (cons 'lambda (cons plain-args body)) or
> (list 'lambda args)) or maybe one or two other forms.

Same problem:

    (cons 'lambda (cons plain-args body))

constructs something that may look like a function but that may not be
intended to be used as a function.  All we know is that it should build
a list with a `lambda` symbol in it.

It's only when the result of the execution of this code is passed to
`macroexp--expand-all` that we discover that it was meant to build
a list that represents the source code of a function; and only at *that*
point are we allowed to modify that list by macro-expanding expressions
in its body, modifying its docstring, byte-compiling, etc...


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

On Sun, Mar 10, 2024 at 20:50:05 -0400, Stefan Monnier wrote:
> >> No matter how many extra tests you add to reduce the frequency, you're
> >> fundamentally adding a bug :-(

> > Well, macroexp--expand-all has treated (function (lambda ...)) as a
> > function long before I started on this project in November.

> Of course it does, and that's correct, because by definition the
> argument to `macroexp--expand-all` must be source code expression.

> > Would trusting the same thing in backquote-process be any
> > more dangerous?

> Yup, because backquote has no guarantee that the code it must produce is
> one that will build a source code expression.

> > How about adding (an) extra arm(s) to the large pcase in
> > macroexp--expand-all which would recognise backquote's output for
> > "evaluated" lambdas.

> > What we get back from backquote looks like:

> > (cons 'lambda (cons plain-args body)) or
> > (list 'lambda args)) or maybe one or two other forms.

> Same problem:

>     (cons 'lambda (cons plain-args body))

> constructs something that may look like a function but that may not be
> intended to be used as a function.  All we know is that it should build
> a list with a `lambda` symbol in it.

> It's only when the result of the execution of this code is passed to
> `macroexp--expand-all` that we discover that it was meant to build
> a list that represents the source code of a function; and only at *that*
> point are we allowed to modify that list by macro-expanding expressions
> in its body, modifying its docstring, byte-compiling, etc...

OK, so it seems like I'll need a new pcase arm in macroexp--expand-all,
and this new code will need to handle (function (cons 'lambda ...)), and
the like.

I'll not actually be able to do too much in the next few days, though.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> OK, so it seems like I'll need a new pcase arm in macroexp--expand-all,
> and this new code will need to handle (function (cons 'lambda ...)), and
> the like.

If macroexp--expand-all receives code of the form

    (function (cons 'lambda ...))

it means it received broken code.
IOW, such an arm will never do anything useful (the best it can do is
emit a warning).

I suspect what you're looking for is yet different.


        Stefan

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

[Alan Mackenzie]

Hello Stefan.

On Wed, Mar 13, 2024 at 07:52:50 -0400, Stefan Monnier wrote:
> > OK, so it seems like I'll need a new pcase arm in macroexp--expand-all,
> > and this new code will need to handle (function (cons 'lambda ...)), and
> > the like.

> If macroexp--expand-all receives code of the form

>     (function (cons 'lambda ...))

> it means it received broken code.
> IOW, such an arm will never do anything useful (the best it can do is
> emit a warning).

> I suspect what you're looking for is yet different.

How about the following (as yet vague) idea?
(i) Amend backquote slightly so that the integer part of the result of
backquote-process gets transmitted to the caller, somehow.  This would
let macroexp--expand-all know it's dealing with smething awkward like
`#'(lambda ,@args . ,body).
(ii) On encountering such, m--e-all would insert a call to (new macro)
macroexp--maybe-posify-lamda-form at ME1 time.
(iii) At ME2 time, m--e-all should be able to tell whether or not the
lambda form is "real".  In this case the macro would be replaced by a
call to (existing) byte-run--posify-lambda-position.  Whether or not this
is done, the SWP lambda would be stripped of its position.

This might work.  What do you think?

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany)

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> This might work.  What do you think?

I don't know what is the problem you're trying to fix, so it's hard for
me to have an opinion.

Could you clarify what is the problem when backquote is not changed
at all (after all, this is the problem that will also occur if the
programmer happened to use `cons/list` instead of backquote)?


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

On Tue, Mar 19, 2024 at 16:47:46 -0400, Stefan Monnier wrote:
> > This might work.  What do you think?

> I don't know what is the problem you're trying to fix, so it's hard for
> me to have an opinion.

> Could you clarify what is the problem when backquote is not changed
> at all (after all, this is the problem that will also occur if the
> programmer happened to use `cons/list` instead of backquote)?

When Lisp gets read for interpretation, defined symbols (e.g. folliowing
defun or cl-defgneric) get positioned, as do lambdas.  When there are ,s
or ,@s on the arg list or the doc string of the lambda, the lambda
currently gets posified by the new code in backquote-process.  Without
the new code, the "complicated" lambdas retain their positions, which
cause errors in pdump, which doesn't (and shouldn't) handle SWPs.

You're right about my sketched approach not working if the programmer
uses cons/list instead of `, ,, and ,@.  (Thanks!)  Maybe I can somehow
wait until (cons 'lambda (cons args body)) has been evaluated in ME2,
before posifying the lambda.  And also take the change out of
backquote-process.

With this idea, most of the new code would go into the (`(function ,(and
f `(lambda ,_ . ,_))) ...) pcase arm of macrexp--expand-all, with
possibly a new arm to catch and "neutralise" the remaining lambdas, which
aren't functions.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> When Lisp gets read for interpretation, defined symbols (e.g. folliowing
> defun or cl-defgneric) get positioned, as do lambdas.

I don't know what that means.

> When there are ,s or ,@s on the arg list or the doc string of the
> lambda, the lambda currently gets posified by the new code in
> backquote-process.

Are you talking about "lambda" as in "the symbol" or as in "a (lambda
...) expression".  If it's "the symbol", then I can't see where a , or ,@
can appear.  If the other, then if there's a , or ,@ in there it
presumably means we don't yet know whether it *will* be
a lambda-expression or just a list with a lambda symbol: at that point,
it's just data and we don't know if it will be used to build code.

> Without the new code, the "complicated" lambdas retain their
> positions, which cause errors in pdump, which doesn't (and shouldn't)
> handle SWPs.

So, IIUC, your "get positioned" above means you preserve/add (rather than
strip) the position info on some symbols, most notably those `lambda`s
which "you" predict will be used for code, and if your prediction is
wrong then those sympos end up escaping into the wild.

> You're right about my sketched approach not working if the programmer
> uses cons/list instead of `, ,, and ,@.  (Thanks!)  Maybe I can somehow
> wait until (cons 'lambda (cons args body)) has been evaluated in ME2,
> before posifying the lambda.  And also take the change out of
> backquote-process.

Sounds about right.  You'll lose information about the place where the
`lambda` symbol was found in the code, but it's hard to do much better
with what we have,

We could introduce a new `backquote-lisp-form` which works just like
backquote but which additionally asserts that what it builds will be
used as a Lisp form rather than as data.

BTW, we already have such a thing under the name "edebug-\`".

> With this idea, most of the new code would go into the (`(function ,(and
> f `(lambda ,_ . ,_))) ...) pcase arm of macrexp--expand-all, with
> possibly a new arm to catch and "neutralise" the remaining lambdas, which
> aren't functions.

Not sure what other lambdas you're thinking of.


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

On Sun, Mar 10, 2024 at 17:03:28 -0400, Stefan Monnier wrote:
> >> >> - Testing `byte-compile-in-progress` can't be right.  Do you to test
> >> >>   whether the result of this backquote will be byte-compiled or do you
> >> >>   really mean to test whether this backquote happens to be executed
> >> >>   during compilation (which could be because the compiler ends up
> >> >>   loading code while executing `eval-when-compile` or `require`)?

> >> > Quite simply, during compilation, all symbols (except nil) get read with
> >> > position, so to strip their positions here would be wrong.

> >> This isn't quite right: during compilation, some code is read with
> >> positions (the code that we will compile), but some code is read in the
> >> normal way (the code we load for the purpose of running).
> >> The distinction is important.

> > OK, I wasn't really counting code that we load as "during compilation",
> > but I take the point.

> The point is that `byte-compile-in-progress` will be non-nil during
> those loads, so you can't use this variable to get the information you need.

Yes.  How about binding it to nil around `load' and recursive edits, and
possibly one or two other things?

> >> More generally: what goes wrong in the above example if you just treat
> >> that as a list of symbol (stripping them all of their position info).
> >> AFAICT when *that* macro is expanded (i.e. ME2) you'll presumably get
> >> code like

> >>     (mapatoms #'(lambda (FOO/p) (DO/p SOME/p (THING/p))))

> >> right?  [ where "/p" means that the symbol has a sympos.  ]
> >> Isn't that sufficient info to add a docstring with position?

> > It's the lambda which has a position rather than the expanded bits from
> > ME2.

> Hmm... then I misunderstand something.  How can the `lambda` have
> a position if you don't include any special treatment of backquote?

In read-positioning-defined-symbols, lambdas get positioned, along with
symbols being defined by defun, defmacro, cl-defmethod, ....  This is not
to do with backquote handling.

> AFAICT the `lambda` in the result of ME1 should not include position
> information because at that time we don't know that it will be used to
> build code rather than be some element of a normal list.

read-positioning-defined-symbols cannot know how (lambda ...) is going to
be used.  It is up to other code (here, macroexp--expand-all) to strip
the position from the lambda when it would be obtrusive.

> And how come the rest doesn't have position information?

read-positioning-defined-symbols is specifically coded (with a state
machine) to position only the lambdas and the defined symbols.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Alan Mackenzie]

Hello, Stefan.

On Tue, Mar 19, 2024 at 21:40:12 +0000, Alan Mackenzie wrote:
> On Tue, Mar 19, 2024 at 16:47:46 -0400, Stefan Monnier wrote:
> > > This might work.  What do you think?

> > I don't know what is the problem you're trying to fix, so it's hard for
> > me to have an opinion.

> > Could you clarify what is the problem when backquote is not changed
> > at all (after all, this is the problem that will also occur if the
> > programmer happened to use `cons/list` instead of backquote)?

> When Lisp gets read for interpretation, defined symbols (e.g. folliowing
> defun or cl-defgneric) get positioned, as do lambdas.  When there are ,s
> or ,@s on the arg list or the doc string of the lambda, the lambda
> currently gets posified by the new code in backquote-process.  Without
> the new code, the "complicated" lambdas retain their positions, which
> cause errors in pdump, which doesn't (and shouldn't) handle SWPs.

I was possibly too dogmatic about this.  I've actually amended pdumper.c
to handle SWPs, and this appears to be working well.  The SWPs from
"complicated" lambdas get recorded in the dumped image, and later used in
ME2.  I've reverted my changes to backquote.el.  :-)

> You're right about my sketched approach not working if the programmer
> uses cons/list instead of `, ,, and ,@.  (Thanks!)  Maybe I can somehow
> wait until (cons 'lambda (cons args body)) has been evaluated in ME2,
> before posifying the lambda.  And also take the change out of
> backquote-process.

Now coded up.

> With this idea, most of the new code would go into the (`(function ,(and
> f `(lambda ,_ . ,_))) ...) pcase arm of macrexp--expand-all, with
> possibly a new arm to catch and "neutralise" the remaining lambdas, which
> aren't functions.

It turns out there was little (?nothing) to change in
macroexp--expand-all.  The code was there already.

> >         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> The point is that `byte-compile-in-progress` will be non-nil during
>> those loads, so you can't use this variable to get the information you need.
> Yes.  How about binding it to nil around `load' and recursive edits, and
> possibly one or two other things?

You mean trying to enumerate all the places we can think of where we
know that compilation is not taking place?  That sounds rather ugly.
I'd rather first try and define precisely what is we mean by
"compilation in progress".

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.

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.

Is that branch trying to provide function-position for compiled
functions only, for interpreted functions only, or both?

If both, could you split it into two, then?
AFAICT doing it only for compiled functions should be significantly
simpler than for interpreted functions, so it would be a good
stepping stone.

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


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

Thanks for taking the time and trouble to review my branch.

On Mon, Mar 25, 2024 at 14:23:50 -0400, Stefan Monnier wrote:
> >> The point is that `byte-compile-in-progress` will be non-nil during
> >> those loads, so you can't use this variable to get the information you need.
> > Yes.  How about binding it to nil around `load' and recursive edits, and
> > possibly one or two other things?

> You mean trying to enumerate all the places we can think of where we
> know that compilation is not taking place?

Yes.

> 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.  Contrast this with, say, loading a .el file, where only some
symbols get positioned.

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

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

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

> Is that branch trying to provide function-position for compiled
> functions only, for interpreted functions only, or both?

It not only tries, but succeeds (modulo remaining bugs) in providing
posification for both interpreted and compiled functions.

> If both, could you split it into two, then?

I'm not sure that would be possible or sensible - both use a common
approach.

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

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

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.

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.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

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

[Alan Mackenzie]

Hello, Stefan.

On Mon, Mar 25, 2024 at 18:10:11 -0400, Stefan Monnier wrote:
> >> 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"?

The symbols read from the source code being compiled by the byte
compiler.  By "active" I mean that a byte compilation has been started,
is not yet complete, and hasn't been temporarily suspended (e.g. for
loading another file with (require 'foo)).

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

We now have two distinct uses of SWPs: providing warning source locations
to the compiler (where we want to keep the position as long as possible)
and providing position information for the doc string (where we want to
strip the position from the symbol ASAP, to avoid trying to use the SWP
when we need a plain symbol).  If both of these occur together, we want
to keep the SWP.

This is the purpose of byte-compile-in-progress.

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

To macroexp--expand-all from wherever.  Here (L423 of macroexp.el) is
where the position gets stripped from the SWP in the non compilation
case.

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

Indeed, yes.

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

Yes, or inside a macro (defun, defmacro, ...) which expands to a
defalias.

Ideally, I would like to have bound defining-symbol inside defun.  But
this would have lost the binding at the end of defun, before evaluating
the defalias.  It was a tricky problem which I think has been solved.

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

> ?

Pretty much, yes.  (declare (defining-symbol name docstring)) also
informs the reader that NAME is to be positioned when in (defun NAME
....).

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

Fload uses read-positioning-DEFINED-symbols, as contrasted with the
compiler, which uses read-positioning-symbols.  r-p-d-s positions only
lambdas and NAMEs.  r-p-s positions all symbols except nil.

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

Ah, right.  I hadn't considered this before.  The changes are by their
very nature essentially complicated and difficult to understand.

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

Neither do I, but it's a possibility.

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

This will not be simpler.  It would involve re-evaluating defun, then
compensating for all the functions up to now whose NAMEs had been read
without positions.  There is unavoidable conplexity, here.  We need defun
to build backquote, byte-run--posify-defining-symbol to build defun, and
so we need to write b-r--p-d-s without backquote.  All that could be done
is to shift the complexity to a different arm of that dependency
triangle.

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

Well all I can say here is that the lack of special cases here has been
most helpful in debugging the current code.  It's possible (?likely) that
somebody will need to look at it again, sometime.

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

I see things somewhat differently.  We shouldn't increase the debugging
burden even on "expert users".  My view is that debugging Lisp in Emacs
is too difficult and tedious, and can be improved.  debug-early.el and
getting backtraces from redisplay errors are two already implemented such
improvements.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Since I still don't understand the general picture, let me tell you how
I would plan to do it, so you can tell me where it matches your
approach and where it doesn't:

- Change `load-source-file-function` so it uses
  `read-positioning-symbols` instead of plain `read`.
  [ This means that macro-expansion will now almost always have sympos,
    rather than only during compilation,  ]
- This in turn requires a strip-sympos pass after the
  eager-macroexpansion phase of `load-source-file-function`.
- Change macros like `lambda` so as to use the extract position info
  from themselves/theirargs/thecontext (when available, since there will
  still be corner cases where it's not available, such as during
  non-eager macro expansion) and stash it in their docstring.
  This might be as simple as adding a line
  (setq docstring (add-pos-to-docstring docstring ARG)).


- Stefan

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

[Alan Mackenzie]

Hello, Stefan.

On Tue, Mar 26, 2024 at 09:40:19 -0400, Stefan Monnier wrote:
> >> > 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.

> Since I still don't understand the general picture, let me tell you how
> I would plan to do it, so you can tell me where it matches your
> approach and where it doesn't:

> - Change `load-source-file-function` so it uses
>   `read-positioning-symbols` instead of plain `read`.
>   [ This means that macro-expansion will now almost always have sympos,
>     rather than only during compilation,  ]

load-source-file-function is set to read-positioning-defined-symbols.
(In a change to be committed, it gets bound to this function in Fload).
In reading

    (defun foo () "foo doc" (lambda (bar) "lambda doc" (car bar)))

, foo gets positioned (because it follows defun), and so does lambda
(because it is a lambda following "(").

> - This in turn requires a strip-sympos pass after the
>   eager-macroexpansion phase of `load-source-file-function`.

No such pass is needed, due to the state machine in
read-positioning-defined-symbols.

> - Change macros like `lambda` so as to use the extract position info
>   from themselves/theirargs/thecontext (when available, since there will
>   still be corner cases where it's not available, such as during
>   non-eager macro expansion) ....

The macro lambda has become obsolete; it had no access to the SWP
lambda for which it was invoked.  It has been replaced by code in
macroexpand-1 and Fmacroexpand which wraps the lambda form in (function
....) and preserves the SWP lambda.

I'm not entirely sure, but I think in non-eager macro expansion the
position information in SWPs is typically available.

>   .... and stash it in their docstring.
>   This might be as simple as adding a line
>   (setq docstring (add-pos-to-docstring docstring ARG)).

This posification is done (mainly) in a call to
byte-run-posify-lambda-form from the (function (lambda ...)) pcase arm
in macroexp--expand-all.

> - Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> >> > 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.
>
>> Since I still don't understand the general picture, let me tell you how
>> I would plan to do it, so you can tell me where it matches your
>> approach and where it doesn't:
>
>> - Change `load-source-file-function` so it uses
>>   `read-positioning-symbols` instead of plain `read`.
>>   [ This means that macro-expansion will now almost always have sympos,
>>     rather than only during compilation,  ]
>
> load-source-file-function is set to read-positioning-defined-symbols.

[ I see it's `load-read-function`.  ]

How does this differ from `read-positioning-symbols` and why do we need
it to be different?

> (In a change to be committed, it gets bound to this function in Fload).
> In reading
>
>     (defun foo () "foo doc" (lambda (bar) "lambda doc" (car bar)))
>
> , foo gets positioned (because it follows defun), and so does lambda
> (because it is a lambda following "(").

IIUC "gets positioned" means that it is a symbol-with-pos rather than
a bare symbol?

> I'm not entirely sure, but I think in non-eager macro expansion the
> position information in SWPs is typically available.

In lazy macro-expansion, SWPs are not available, no.
But that's OK, it's rare and there's very little we can do about it (the
reason it's lazy is indeed because we only discover very late that those
sexps were meant to represent code.  It's typically when a sexp is
passed to `eval`).


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

On Tue, Mar 26, 2024 at 15:40:05 -0400, Stefan Monnier wrote:
> >> >> > 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.

> >> Since I still don't understand the general picture, let me tell you how
> >> I would plan to do it, so you can tell me where it matches your
> >> approach and where it doesn't:

> >> - Change `load-source-file-function` so it uses
> >>   `read-positioning-symbols` instead of plain `read`.
> >>   [ This means that macro-expansion will now almost always have sympos,
> >>     rather than only during compilation,  ]

> > load-source-file-function is set to read-positioning-defined-symbols.

> [ I see it's `load-read-function`.  ]

Yes!

> How does this differ from `read-positioning-symbols` and why do we need
> it to be different?

r-p-defined-s positions only lambdas and NAMEs defined by defun,
defmacro, defvar, .... (around 50 defining symbols).  r-p-s positions
every symbol apart from nil.  They have different purposes.  r-p-d-s
gets info for the doc strings, which requires SWPs only for some
symbols.  r-p-s is needed to get warning message locations.  Were r-p-s
used for the doc string position information, most of the symbols would
need to be stripped of their positions before the form could be used.
It is simpler and faster not to position them at all.

> > (In a change to be committed, it gets bound to this function in Fload).
> > In reading

> >     (defun foo () "foo doc" (lambda (bar) "lambda doc" (car bar)))

> > , foo gets positioned (because it follows defun), and so does lambda
> > (because it is a lambda following "(").

> IIUC "gets positioned" means that it is a symbol-with-pos rather than
> a bare symbol?

Yes.

> > I'm not entirely sure, but I think in non-eager macro expansion the
> > position information in SWPs is typically available.

> In lazy macro-expansion, SWPs are not available, no.
> But that's OK, it's rare and there's very little we can do about it (the
> reason it's lazy is indeed because we only discover very late that those
> sexps were meant to represent code.  It's typically when a sexp is
> passed to `eval`).

OK.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> We now have two distinct uses of SWPs: providing warning source locations
> to the compiler (where we want to keep the position as long as possible)
> and providing position information for the doc string (where we want to
> strip the position from the symbol ASAP, to avoid trying to use the SWP
> when we need a plain symbol).  If both of these occur together, we want
> to keep the SWP.

I think I'm beginning to understand.  So in the "load from source case",
some of your symbols are SWPs and you want to turn them into bare
symbols "on the fly" during macro-expansion rather than via a separate
"strip" phase, so you want the macro expansion to know whether it's done
for "load from source" or for some other purpose and you use
`byte-compile-in-progress` as a proxy for that information.
Is that it?

If so, (and if the stripping happens within macros), then indeed passing
it as a separate argument through all the recursive calls to
`macroexp--expand-all` would be cumbersome.  But I suggest you use
another name for that(e.g. `macroexp-strip-position`) so the
intention is made more clear.

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

>> > 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`?
> Yes, or inside a macro (defun, defmacro, ...) which expands to a
> defalias.

These are *very* different times: `Fdefalias` and `Fdefvar_1` are
executed long after macroexpansion.  And they're small C functions which
run almost no external code at all, so they "occupy stack frames" only
for a very short time.

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

If my crystal ball is right, then the better place to put that info is
probably `macroexpand-all-environment`.

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

> Fload uses read-positioning-DEFINED-symbols, as contrasted with the
> compiler, which uses read-positioning-symbols.  r-p-d-s positions only
> lambdas and NAMEs.  r-p-s positions all symbols except nil.

I think I'm beginning to understand (I guess I was struggling with your
use of "position" as a verb for some reason which made me think that
symbols were being moved rather than gaining position information).

So, IIUC you use `read-positioning-DEFINED-symbols` instead of
`read-positioning-symbols` because it's cheaper?  Do you have rough
numbers comparing the cost of `read`, `read-positioning-symbols`, and
`read-positioning-DEFINED-symbols`?

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

> Ah, right.  I hadn't considered this before.  The changes are by their
> very nature essentially complicated and difficult to understand.

[ Hmm... maybe not the best salespitch.  ]

>> 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`.
>
> This will not be simpler.  It would involve re-evaluating defun, then
> compensating for all the functions up to now whose NAMEs had been read
> without positions.

Not at all.  Those will remain without position, but only in
`src/bootstrap-emacs`.  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.

> There is unavoidable conplexity, here.

I'm definitely not convinced.  I suspect you've been asking yourself
"can it be made simpler" and you may indeed then convince yourself that
the answer is no, because of assumptions you don't reconsider.

Try instead to think about "what would it take to remove that complexity?".

> I see things somewhat differently.  We shouldn't increase the debugging
> burden even on "expert users".

Yet it's imposing more complexity (and hence more debugging burden) on
those same expert users.  🙁


        Stefan

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> r-p-defined-s positions only lambdas and NAMEs defined by defun,
> defmacro, defvar, .... (around 50 defining symbols).  r-p-s positions
> every symbol apart from nil.  They have different purposes.  r-p-d-s
> gets info for the doc strings, which requires SWPs only for some
> symbols.  r-p-s is needed to get warning message locations.  Were r-p-s
> used for the doc string position information, most of the symbols would
> need to be stripped of their positions before the form could be used.
> It is simpler and faster not to position them at all.

In terms of code, I can't see why it'd be simpler: we already have the
r-p-s function, and we already have a function to strip that info when we
don't need it any more, so it would be less new code to write if we just
used r-p-s, I think.


        Stefan

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

[Drew Adams]

> I think I'm beginning to understand (I guess I was struggling with your
> use of "position" as a verb for some reason which made me think that
> symbols were being moved rather than gaining position information).

Not following this thread (at all).

I suggest that for the new meaning ("gaining position info") you use the (just minted)
verb "positionize" instead of "position".

Or some other change in vocabulary.

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

[Alan Mackenzie]

Hello, Stefan.

On Tue, Mar 26, 2024 at 16:42:28 -0400, Stefan Monnier wrote:
> > r-p-defined-s positions only lambdas and NAMEs defined by defun,
> > defmacro, defvar, .... (around 50 defining symbols).  r-p-s positions
> > every symbol apart from nil.  They have different purposes.  r-p-d-s
> > gets info for the doc strings, which requires SWPs only for some
> > symbols.  r-p-s is needed to get warning message locations.  Were r-p-s
> > used for the doc string position information, most of the symbols would
> > need to be stripped of their positions before the form could be used.
> > It is simpler and faster not to position them at all.

> In terms of code, I can't see why it'd be simpler: we already have the
> r-p-s function, ....

We also already have r-p-d-s.  Both functions (together with plain read)
have read0 as their core engine.  The enhancement to read0 to support
r-p-d-s was only moderate in size and not complicated to anybody who
understands finite state machines.

> .... and we already have a function to strip that info when we don't
> need it any more, so it would be less new code to write if we just
> used r-p-s, I think.

I think you're envisaging an extensive redesign where SWPs would not be
tightly and individually controlled as they are at the moment, but
instead would be created en masse and stripped en masse a bit later.

There seems to me to be little justification for this.  It would need
more code in byte-run.el, and would be slower, too.  It would take a lot
of work to implement and debug.  At the moment, some SWPs (to do with
"complicated" backquoted lambda forms) survive long term, meaning we
would need complicated Lisp code to protect these individually.  

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Alan Mackenzie]

Hello, Stefan.

On Tue, Mar 26, 2024 at 16:30:06 -0400, Stefan Monnier wrote:
> > We now have two distinct uses of SWPs: providing warning source locations
> > to the compiler (where we want to keep the position as long as possible)
> > and providing position information for the doc string (where we want to
> > strip the position from the symbol ASAP, to avoid trying to use the SWP
> > when we need a plain symbol).  If both of these occur together, we want
> > to keep the SWP.

> I think I'm beginning to understand.  So in the "load from source case",
> some of your symbols are SWPs and you want to turn them into bare
> symbols "on the fly" during macro-expansion rather than via a separate
> "strip" phase, ....

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.

> .... so you want the macro expansion to know whether it's done
> for "load from source" or for some other purpose and you use
> `byte-compile-in-progress` as a proxy for that information.
> Is that it?

More or less.  But byte-compile-in-progress isn't a proxy, it's the prime
criterion for deciding.

> If so, (and if the stripping happens within macros), then indeed passing
> it as a separate argument through all the recursive calls to
> `macroexp--expand-all` would be cumbersome.  But I suggest you use
> another name for that(e.g. `macroexp-strip-position`) so the
> intention is made more clear.

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.

> 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.  Besides, macroexpand-all-environment is not
documented anywhere, what it is, what it's for, etc.

> >> > 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`?
> > Yes, or inside a macro (defun, defmacro, ...) which expands to a
> > defalias.

> These are *very* different times: `Fdefalias` and `Fdefvar_1` are
> executed long after macroexpansion.  And they're small C functions which
> run almost no external code at all, so they "occupy stack frames" only
> for a very short time.

I think we were talking about the handling of defining-symbol.  It has a
valid binding outside of macros such as defun, and this binding is used
to posify the containing form.

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

> If my crystal ball is right, then the better place to put that info is
> probably `macroexpand-all-environment`.

See above.

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

> > Fload uses read-positioning-DEFINED-symbols, as contrasted with the
> > compiler, which uses read-positioning-symbols.  r-p-d-s positions only
> > lambdas and NAMEs.  r-p-s positions all symbols except nil.

> I think I'm beginning to understand (I guess I was struggling with your
> use of "position" as a verb for some reason which made me think that
> symbols were being moved rather than gaining position information).

Sorry about that.

> So, IIUC you use `read-positioning-DEFINED-symbols` instead of
> `read-positioning-symbols` because it's cheaper?

No, because it does the Right Thing.

> 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) since they
use the same engine, read0 (in lread.c).  Each of them will be one or two
orders of magnitude faster than emulating them in Lisp.

> 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, right.  I hadn't considered this before.  The changes are by their
> > very nature essentially complicated and difficult to understand.

> [ Hmm... maybe not the best salespitch.  ]

;-)  It's the truth, though.

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

> > This will not be simpler.  It would involve re-evaluating defun, then
> > compensating for all the functions up to now whose NAMEs had been read
> > without positions.

> Not at all.  Those will remain without position, but only in
> `src/bootstrap-emacs`.

This would be a Bad Thing.  The current code is active right after
loading byte-run.

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

> > There is unavoidable conplexity, here.

> I'm definitely not convinced.  I suspect you've been asking yourself
> "can it be made simpler" and you may indeed then convince yourself that
> the answer is no, because of assumptions you don't reconsider.

> Try instead to think about "what would it take to remove that complexity?".

> > I see things somewhat differently.  We shouldn't increase the debugging
> > burden even on "expert users".

> Yet it's imposing more complexity (and hence more debugging burden) on
> those same expert users.  🙁

That's a fairly difficult philosophical question.  Do we provide full
functionality at the cost of more (difficult) source code, or do we
restrict the functioality to keep the source simpler?  I think with Emacs
we usually go with the first alternative.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> 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

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> > r-p-defined-s positions only lambdas and NAMEs defined by defun,
>> > defmacro, defvar, .... (around 50 defining symbols).  r-p-s positions
>> > every symbol apart from nil.  They have different purposes.  r-p-d-s
>> > gets info for the doc strings, which requires SWPs only for some
>> > symbols.  r-p-s is needed to get warning message locations.  Were r-p-s
>> > used for the doc string position information, most of the symbols would
>> > need to be stripped of their positions before the form could be used.
>> > It is simpler and faster not to position them at all.
>> In terms of code, I can't see why it'd be simpler: we already have the
>> r-p-s function, ....
> We also already have r-p-d-s.

You're playing on words here: we don't "already have" `r-p-d-s` on master.

> Both functions (together with plain read) have read0 as their core
> engine.  The enhancement to read0 to support r-p-d-s was only moderate
> in size and not complicated to anybody who understands finite
> state machines.

Just because it's not a complex change doesn't mean it's "simpler" than
no change at all.

>> .... and we already have a function to strip that info when we don't
>> need it any more, so it would be less new code to write if we just
>> used r-p-s, I think.
> I think you're envisaging an extensive redesign where SWPs would not be
> tightly and individually controlled as they are at the moment, but
> instead would be created en masse and stripped en masse a bit later.

Yes, that's the starting design I had in mind (and that I described
a few emails back).  it's also what we do in the byte-compilation case,
so it's code we already have and use.
The "en masse" doesn't make it complex.


        Stefan

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

[Alan Mackenzie]

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

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

[Alan Mackenzie]

Hello, Stefan.

On Wed, Mar 27, 2024 at 08:23:52 -0400, Stefan Monnier wrote:
> >> > r-p-defined-s positions only lambdas and NAMEs defined by defun,
> >> > defmacro, defvar, .... (around 50 defining symbols).  r-p-s positions
> >> > every symbol apart from nil.  They have different purposes.  r-p-d-s
> >> > gets info for the doc strings, which requires SWPs only for some
> >> > symbols.  r-p-s is needed to get warning message locations.  Were r-p-s
> >> > used for the doc string position information, most of the symbols would
> >> > need to be stripped of their positions before the form could be used.
> >> > It is simpler and faster not to position them at all.
> >> In terms of code, I can't see why it'd be simpler: we already have the
> >> r-p-s function, ....
> > We also already have r-p-d-s.

> You're playing on words here: we don't "already have" `r-p-d-s` on master.

I'm not playing on words.  My point is that
read-positioning-defined-symbols exists and works.  It is not a
speculative "would be nice to have".  The work has already been done.

> > Both functions (together with plain read) have read0 as their core
> > engine.  The enhancement to read0 to support r-p-d-s was only moderate
> > in size and not complicated to anybody who understands finite
> > state machines.

> Just because it's not a complex change doesn't mean it's "simpler" than
> no change at all.

No change isn't an option, here.  Something has to determine which
symbols are to be positioned / stripped of their positions.  Currently,
this is done economically in r-p-d-s.

> >> .... and we already have a function to strip that info when we don't
> >> need it any more, so it would be less new code to write if we just
> >> used r-p-s, I think.
> > I think you're envisaging an extensive redesign where SWPs would not be
> > tightly and individually controlled as they are at the moment, but
> > instead would be created en masse and stripped en masse a bit later.

> Yes, that's the starting design I had in mind (and that I described
> a few emails back).  it's also what we do in the byte-compilation case, ...

It's not.  In each case, the optimum number of symbols gets positioned.
Positioning "everything" is sub-optimal for everything bar byte
compilation.

> so it's code we already have and use.

See above.

> The "en masse" doesn't make it complex.

It changes the design extensively, thus introducing new complications.

Why do you think this design change will be better than the existing
design?

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> 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.
[...]
> Byte compilation is NOT calling loading from source.  We don't have a
> caller/callee relationship here.  We are doing posification either from

The "callee" I'm talking about is `load-source-file-function` (which is
instead another "caller", beside the byte compiler), it is the
code that tests `byte-compile-in-progress`.

Based on the above two elements, I suggest the name
`macroexp--inhibit-strip-sympos` (and set/pass/bind it as needed in the
compiler).

[ FWIW, the reason I associate this variable with
  `load-source-file-function` rather than with the compiler is because
  the macroexp code which tests this variable only strips the SWPs at
  a very few different spots, more specifically those very spots that
  are expected to get SWPs in the `load-source-file-function`.
  Different minds work differently, I guess. 🙂  ]

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

Indeed testing `lexical-binding` in macros (like we do at a few places)
sucks; it's an ugly hack.  We use it because that was the least bad
option we could come up with given the need to preserve
backward compatibility.
Here there's no such problem.

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

Please "full stop" being absolutist.  We're talking about opinions and
preferences here.

When hitting an error, I spend more time reading the code (and modifying
it) than looking at debug output, so to me the clarity of the code is
more important than whether a few lambdas get some addition positional
info, especially since I usually know full well where those lambdas
come from.

I understand it affects us differently, but the tradeoff is real.

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

Code is not "fire and forget".

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

We're still miscommunicating.  You're talking about how your code is
implemented, apparently, whereas I'm asking about what is the
intended behavior.

It's like I'm asking what the C spec says and you're answering me by
telling me how GCC works.

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

Without knowing what it is intended to do, the only thing we can say is
that it does what it does, so no indeed it won't fail to do what it
does, since that's what it does. 🙂

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

They'd all need to be stripped, AFAICT, so we'd do:

    (strip-all-symbol-positions
     (macroexp--expand-all
      (read-positioning-symbols)))

What would be hard about it?

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

Actually, now that I look at the code I only see:

            ((guard (and (not byte-compile-in-progress)
                         (symbol-with-pos-p form)))
             (bare-symbol form))

is that the "arm" you're talking about?  AFAICT this will handle only
those symbols which appear as Lisp expressions (IOW symbols which are
variable references), so it will strip the `bar` in the second example
but not the `bar` in my first exmple, nor the two `lambda`s, nor
those in

    '(lambda (defun bar))

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

A lot of it is hard to read because it is constrained to a restrictive
subset of ELisp.

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

"Most needed" according to which criteria?

> I'm not playing on words.  My point is that
> read-positioning-defined-symbols exists and works.  It is not a
> speculative "would be nice to have".  The work has already been done.

Code costs by merely existing.

> Why do you think this design change will be better than the existing
> design?

I don't actually know whether it will be better.  It just seems it could
lead to simpler code, with no change at all to the reader, for example.
I'm here asking what lead you to the current design, under the
assumption that the complexity you introduced was the result of
other experiments.

Am I to understand that the current code is basically your first attempt
at adding such functionality?


        Stefan

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> The "callee" I'm talking about is `load-source-file-function` (which is
                                  ^^^
                                  not

> instead another "caller", beside the byte compiler), it is the
> code that tests `byte-compile-in-progress`.


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

On Thu, Mar 28, 2024 at 12:25:11 -0400, Stefan Monnier wrote:

[ .... ]

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

> Please "full stop" being absolutist.  We're talking about opinions and
> preferences here.

What you're proposing is only handling some fuctions because you think
we're collectively not clever enough to maintain
byte-run--posify-defining-form.  This would leave Emacs inconsistent,
some functions failing to be handled not for any functional reason, but
because of an alleged lack of our capability.

> When hitting an error, I spend more time reading the code (and modifying
> it) than looking at debug output, so to me the clarity of the code is
> more important than whether a few lambdas get some addition positional
> info, especially since I usually know full well where those lambdas
> come from.

My prime method of debugging is reading code, too.  But you're conflating
the clarity of b-r--p-defining-f with the clarity of the code you're
debugging.  They're different things.  The former is less important than
the latter.  The whole point in these changes is to give info precisely
in those anonymous lambda entries in backtraces which currently contain no
information.

> I understand it affects us differently, but the tradeoff is real.

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

This is done in other functions, too.

> > The cost has already been paid, by me.

> Code is not "fire and forget".

[ .... ]

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

> A lot of it is hard to read because it is constrained to a restrictive
> subset of ELisp.

byte-run--posify-defining-form uses the same techniques as other declare
clause handlers, such as byte-run--set-interactive-only and many others.
Why is b-r--p-defining-f objectionable, but not b-r--s-i-only?

It was tedious rather than difficult to write, and it is tedious rather
than difficult to read.

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

> "Most needed" according to which criteria?

The difficulty of debugging in early bootstrap compared with when further
debugging tools have already been loaded.

[ .... ]

> Code costs by merely existing.

Inconsistencies and sloppy implementation cost too.

[ .... ]

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Alan Mackenzie]

Hello, Stefan.

On Thu, Mar 28, 2024 at 12:25:11 -0400, Stefan Monnier wrote:

[ .... ]

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

> We're still miscommunicating.  You're talking about how your code is
> implemented, apparently, whereas I'm asking about what is the
> intended behavior.

I am still mystified by your failure to understand "currently being
defined", a phrase that to me could hardly be clearer.

> It's like I'm asking what the C spec says and you're answering me by
> telling me how GCC works.

OK, let's try again.  defining-symbol records the symbol currently being
defined.  It's used to set the defining symbol and buffer offset fields
in the position structure in that symbol's doc string, and also in the
doc strings of contained lambda forms.

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

> Without knowing what it is intended to do, the only thing we can say is
> that it does what it does, so no indeed it won't fail to do what it
> does, since that's what it does. 🙂

Is my previous paragraph sufficiently clear?  If so, can you envisage a
scenario where a symbol being defined would fail to get the two fields
correctly set in its doc string or a lambda form's doc string?

[ .... ]

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Alan Mackenzie]

Hello, Stefan.

On Thu, Mar 28, 2024 at 12:25:11 -0400, Stefan Monnier wrote:

[ .... ]

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

> They'd all need to be stripped, AFAICT, so we'd do:

>     (strip-all-symbol-positions
>      (macroexp--expand-all
>       (read-positioning-symbols)))

> What would be hard about it?

Some symbols must not be stripped.  For example, in cl-generic.el L403
we have:

    (fun `(cl-function (lambda ,plain-args ,@body)))              

..  There the position on the lambda must be preserved until ME2 time
when it becomes clear what the shape of the lambda is.  In particular,
whether there is already a doc string in ,@body to amend, or we need to
insert a new one.

> >> >> 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 (lambda bar)
  `(cons ,lambda ,bar))

exoands to

(macro closure (t) (lambda bar) ";POS^^^A^A^A [foo *scratch* 158 nil]
" (list 'cons lambda bar))

..

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

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

Similarly, we get

(defun baz ()
  (let ((lambda 'foo)
        (defun 'bar))
    (cons lambda defun)))

(symbol-function 'baz)
(closure (t) nil ";POS^^^A^A^A [baz *scratch* 323 nil]
" (let ((lambda 'foo) (defun 'bar)) (cons lambda defun)))

, so it is clear this case is getting handled OK.  I'm afraid I can't
point out the exact place in the code at the moment where this is
getting done.

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

> Actually, now that I look at the code I only see:

>             ((guard (and (not byte-compile-in-progress)
>                          (symbol-with-pos-p form)))
>              (bare-symbol form))

> is that the "arm" you're talking about?

Yes.

> AFAICT this will handle only those symbols which appear as Lisp
> expressions (IOW symbols which are variable references), so it will
> strip the `bar` in the second example but not the `bar` in my first
> exmple, nor the two `lambda`s, nor those in

>     '(lambda (defun bar))

See above.

[ .... ]

> > Why do you think this design change will be better than the existing
> > design?

> I don't actually know whether it will be better.  It just seems it could
> lead to simpler code, with no change at all to the reader, for example.

The exercise is intrinsically complicated.  read0 is largely self
contained, meaning any compexity introduced there won't spill over into
other functions.  What you're suggesting is that the code to decide
which SWPs to strip is going to be simpler than the enhancements to the
reader.

> I'm here asking what lead you to the current design, under the
> assumption that the complexity you introduced was the result of
> other experiments.

The complexity is essential to the task being done.  I'm convinced it
cannot be avoided, though it could be moved around the code base, to
some extent.  I don't have a record of the precise reasons for the
design.  To a large extent it was a sequence of solutions to the often
awkward problems the code presented.

> Am I to understand that the current code is basically your first attempt
> at adding such functionality?

By no means.  This is the second implementation, the first having been
rejected by Eli and you last autumn.  I'm very much looking forward to
not needing a third implementation.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> We're still miscommunicating.  You're talking about how your code is
>> implemented, apparently, whereas I'm asking about what is the
>> intended behavior.
> I am still mystified by your failure to understand "currently being
> defined", a phrase that to me could hardly be clearer.

AFAIK, the definition itself happens inside `Fdefalias`.  It's a very
short amount of time during which none of your code is executed, so that
can't be it.  The rest is the actual construction of the value/function
object which will make up the definition.  This construction is done
piecemeal in various phases at potentially various different times, not
all of them necessarily on the same machine.  Some of the code executed
during the course of the construction of this object have nothing at all
to do with that object, they just happen to be used by some code which
participates in the construction of that object.

So "currently" (i.e. referring to *time*) is very problematic because
it's only loosely correlated to what you're interested in.

>> It's like I'm asking what the C spec says and you're answering me by
>> telling me how GCC works.
>
> OK, let's try again.  defining-symbol records the symbol currently being
> defined.  It's used to set the defining symbol and buffer offset fields
> in the position structure in that symbol's doc string, and also in the
> doc strings of contained lambda forms.

I can try it again also if you want: to do it right, I think you want to
store that information in `macroexpand-all-environment`.

> Is my previous paragraph sufficiently clear?  If so, can you envisage a
> scenario where a symbol being defined would fail to get the two fields
> correctly set in its doc string or a lambda form's doc string?

Yes, for example I can imagine a lazy macroexpansion done to run some
code during the eager macroexpansion could mistakenly think that it is
part of the eagerly macroexpanded function (whereas it's only part of
the code used during the construction of that function).

I'm sure there are other cases, by thinking of it makes my head hurt.
Which is why I recommend you put that info into
`macroexpand-all-environment` which is designed specifically for such
purpose of "currently within the scope of something".


        Stefan

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Some symbols must not be stripped.  For example, in cl-generic.el L403
> we have:
>
>     (fun `(cl-function (lambda ,plain-args ,@body)))              
>
> ..  There the position on the lambda must be preserved until ME2 time
> when it becomes clear what the shape of the lambda is.  In particular,
> whether there is already a doc string in ,@body to amend, or we need to
> insert a new one.

I could see some reasons you *may* want to keep some info here, but it's
definitely not a "must" because the source position of those functions
should generally not point to `cl-generic.el:403` but to where
`cl-defmethod` was used.

Also, if you do want to preserve some info there (presumably with the
intent to combine it with the more important info that will be available
at ME2) it will need cooperation from `cl-generic.el` because, as far as
the semantic of Emacs Lisp is concerned, the above constructs a list
with a `lambda` symbol inside of it, with no guarantee that it will be
used as a function, and even if ever used as a function there's no
guarantee that this list will pass through the few places where we strip
SWPs, so keeping SWPs in there without some explicit request from
`cl-generic.el` would be a bug.

IOW, I don't think it's a good reason to rule out

    (strip-all-symbol-positions
     (macroexp--expand-all
      (read-positioning-symbols)))

BTW, AFAIK the above is conceptually what the byte-compiler does (except
it performs a few more transformations between `macroexp--expand-all`
and `strip-all-symbol-positions`).  Is it the case that `cl-defmethod`
generates a function whose source position (partly) points to
`generic.el:403` if `cl-generic.el` was interpreted but not if
it compiled?

>> >> >> 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 (lambda bar)
>   `(cons ,lambda ,bar))
>
> exoands to
>
> (macro closure (t) (lambda bar) ";POS^^^A^A^A [foo *scratch* 158 nil]
> " (list 'cons lambda bar))

IIUC your reader will make the `lambda` formal argument into an SWP.
Where is that SWP stripped?

> so it is clear this case is getting handled OK.  I'm afraid I can't
> point out the exact place in the code at the moment where this is
> getting done.

I think it would be good to know, so as to be able to decide whether
it'll indeed always work right, or we just got lucky this time.

>> I don't actually know whether it will be better.  It just seems it could
>> lead to simpler code, with no change at all to the reader, for example.
> The exercise is intrinsically complicated.

Could you explain what you think makes it intrinsically complex?

> What you're suggesting is that the code to decide which SWPs to strip
> is going to be simpler than the enhancements to the reader.

As seen above, I suggest to leave the reader unchanged and to strip all
SWPs.  I'm pretty sure it would give comparable info to what you have
and it would be simpler (also, it would make it much less likely to
have discrepancies between the compiled case and the interpreted case).
My main worry with it would be performance.


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

Sorry about the delay - I lost my email server after an obsolete SSL
library got deleted from my system, and one or two other things, too.

On Sat, Mar 30, 2024 at 22:54:18 -0400, Stefan Monnier wrote:
> > Some symbols must not be stripped.  For example, in cl-generic.el L403
> > we have:

> >     (fun `(cl-function (lambda ,plain-args ,@body)))              

> > ..  There the position on the lambda must be preserved until ME2 time
> > when it becomes clear what the shape of the lambda is.  In particular,
> > whether there is already a doc string in ,@body to amend, or we need to
> > insert a new one.

> I could see some reasons you *may* want to keep some info here, but it's
> definitely not a "must" because the source position of those functions
> should generally not point to `cl-generic.el:403` but to where
> `cl-defmethod` was used.

Pretending the problem doesn't exist won't solve it.  In the ;POS...
structures for a lambda, there are two pointers - one to the definition
of the lambda, the other to the point of use.

> Also, if you do want to preserve some info there (presumably with the
> intent to combine it with the more important info that will be available
> at ME2) it will need cooperation from `cl-generic.el` because, as far as
> the semantic of Emacs Lisp is concerned, the above constructs a list
> with a `lambda` symbol inside of it, with no guarantee that it will be
> used as a function, ....

Mostly there is the symbol `function' in that position.  Here we've got
`cl-function' which expands to function.  Surely with (function (lambda
....)) we know we're dealing with a function.

> .... and even if ever used as a function there's no guarantee that this
> list will pass through the few places where we strip SWPs, so keeping
> SWPs in there without some explicit request from `cl-generic.el` would
> be a bug.

I don't think this is right.  The code will pass through
macroexp--expand-all, which is where the SWP wii be stripped.

> IOW, I don't think it's a good reason to rule out

>     (strip-all-symbol-positions
>      (macroexp--expand-all
>       (read-positioning-symbols)))

As I've said, we'd need code to preserve the SWPs on "complicated"
lambdas.  I haven't even begun to think about how this could work.

> BTW, AFAIK the above is conceptually what the byte-compiler does (except
> it performs a few more transformations between `macroexp--expand-all`
> and `strip-all-symbol-positions`).

It is a bad idea to conflate these two radically different uses of SWPs.
That can only lead to confusion and bugs.

> Is it the case that `cl-defmethod` generates a function whose source
> position (partly) points to `generic.el:403` if `cl-generic.el` was
> interpreted but not if it compiled?

No, the intention is that the source positions are independent of whether
the code is compiled.

> >> >> >> 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 (lambda bar)
> >   `(cons ,lambda ,bar))

> > expands to

> > (macro closure (t) (lambda bar) ";POS^^^A^A^A [foo *scratch* 158 nil]
> > " (list 'cons lambda bar))

> IIUC your reader will make the `lambda` formal argument into an SWP.
> Where is that SWP stripped?

In macroexp--expand-all in the "guard arm" near the end.

> > so it is clear this case is getting handled OK.  I'm afraid I can't
> > point out the exact place in the code at the moment where this is
> > getting done.

> I think it would be good to know, so as to be able to decide whether
> it'll indeed always work right, or we just got lucky this time.

See above.

> >> I don't actually know whether it will be better.  It just seems it could
> >> lead to simpler code, with no change at all to the reader, for example.
> > The exercise is intrinsically complicated.

> Could you explain what you think makes it intrinsically complex?

The mass of detail that needs dealing with that Emacs has collected over
the decades.  As a counter question, why do you think the exercise ought
to be simple (assuming you do think this)?

> > What you're suggesting is that the code to decide which SWPs to strip
> > is going to be simpler than the enhancements to the reader.

> As seen above, I suggest to leave the reader unchanged and to strip all
> SWPs.  I'm pretty sure it would give comparable info to what you have
> and it would be simpler (also, it would make it much less likely to
> have discrepancies between the compiled case and the interpreted case).

"Comparable" isn't good enough - we need the position info on
"complicated" lambdas to endure, somehow.  There are no discrepancies
between compiled and interpreted forms because they both use the same
mechanism in macro expansion.

> My main worry with it would be performance.

Yhat, too.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Alan Mackenzie]

Hello, Stefan.

On Sat, Mar 30, 2024 at 22:22:52 -0400, Stefan Monnier wrote:
> >> We're still miscommunicating.  You're talking about how your code is
> >> implemented, apparently, whereas I'm asking about what is the
> >> intended behavior.
> > I am still mystified by your failure to understand "currently being
> > defined", a phrase that to me could hardly be clearer.

> AFAIK, the definition itself happens inside `Fdefalias`.  It's a very
> short amount of time during which none of your code is executed, so that
> can't be it.

The definition starts when the reader reads (defun foo ...).  It ends
when that Fdefalias has been evaluated.  Between those two events,
defining-symbol is bound to foo.

> The rest is the actual construction of the value/function object which
> will make up the definition.  This construction is done piecemeal in
> various phases at potentially various different times, not all of them
> necessarily on the same machine.

Really?  :-)

> Some of the code executed during the course of the construction of this
> object have nothing at all to do with that object, they just happen to
> be used by some code which participates in the construction of that
> object.

That's a bit too abstract.  I can't see the potential problems that you
see, possibly because I've already solved them.

> So "currently" (i.e. referring to *time*) is very problematic because
> it's only loosely correlated to what you're interested in.

Again, I don't see these problems.

> >> It's like I'm asking what the C spec says and you're answering me by
> >> telling me how GCC works.

> > OK, let's try again.  defining-symbol records the symbol currently being
> > defined.  It's used to set the defining symbol and buffer offset fields
> > in the position structure in that symbol's doc string, and also in the
> > doc strings of contained lambda forms.

> I can try it again also if you want: to do it right, I think you want to
> store that information in `macroexpand-all-environment`.

That variable doesn't exist, yet.

> > Is my previous paragraph sufficiently clear?  If so, can you envisage a
> > scenario where a symbol being defined would fail to get the two fields
> > correctly set in its doc string or a lambda form's doc string?

> Yes, for example I can imagine a lazy macroexpansion done to run some
> code during the eager macroexpansion could mistakenly think that it is
> part of the eagerly macroexpanded function (whereas it's only part of
> the code used during the construction of that function).

The critical thing here is the variable defining-symbol.  I think you're
suggesting that its value could accidentally find its way into other
symbols' position structures.  When are those other symbols getting
defined?  Surely if a secondary defun, defmacro, defvar, ... happens
during the main defun, its defining symbol is that of the main defun?

> I'm sure there are other cases, by thinking of it makes my head hurt.
> Which is why I recommend you put that info into
> `macroexpand-all-environment` which is designed specifically for such
> purpose of "currently within the scope of something".

Well, m-a-e is defined too late.  Even if it weren't, how would it solve
any of these problems you see?  It's just swapping one dynamically bound
variable for another.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> The definition starts when the reader reads (defun foo ...).  It ends
> when that Fdefalias has been evaluated.  Between those two events,
> defining-symbol is bound to foo.

When the code is compiled, years can pass between those two events.

> The critical thing here is the variable defining-symbol.  I think you're
> suggesting that its value could accidentally find its way into other
> symbols' position structures.  When are those other symbols getting
> defined?  Surely if a secondary defun, defmacro, defvar, ... happens
> during the main defun, its defining symbol is that of the main defun?

That secondary symbol might be defined by the macro for its own use,
rather than for the use of the returned code.

> Well, m-a-e is defined too late.

According to your self-imposed rule 🙂

> Even if it weren't, how would it solve any of these problems you see?
> It's just swapping one dynamically bound variable for another.

Indeed, m-a-e has to fight the same issues, in theory, but in practice,
those issues have already been addressed over the years.  It's rebound
to nil "all the time" to try and make sure its effect doesn't leak.

BTW, it would be nice to separate your patches into some that add
position info to lambda's docstring, and then others that add
"defining symbol" to the available position info.


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

On Sun, Apr 07, 2024 at 22:19:28 -0400, Stefan Monnier wrote:
> > The definition starts when the reader reads (defun foo ...).  It ends
> > when that Fdefalias has been evaluated.  Between those two events,
> > defining-symbol is bound to foo.

> When the code is compiled, years can pass between those two events.

Really?  Then defining-symbol would stay bound over these years, too.
Where's the problem?

> > The critical thing here is the variable defining-symbol.  I think you're
> > suggesting that its value could accidentally find its way into other
> > symbols' position structures.  When are those other symbols getting
> > defined?  Surely if a secondary defun, defmacro, defvar, ... happens
> > during the main defun, its defining symbol is that of the main defun?

> That secondary symbol might be defined by the macro for its own use,
> rather than for the use of the returned code.

> > Well, m-a-e is defined too late.

> According to your self-imposed rule 🙂

> > Even if it weren't, how would it solve any of these problems you see?
> > It's just swapping one dynamically bound variable for another.

> Indeed, m-a-e has to fight the same issues, in theory, but in practice,
> those issues have already been addressed over the years.  It's rebound
> to nil "all the time" to try and make sure its effect doesn't leak.

defining-symbol isn't bound like this, in order for its value to be
available when needed.

> BTW, it would be nice to separate your patches into some that add
> position info to lambda's docstring, and then others that add
> "defining symbol" to the available position info.

All this could be moot.  Byte compiling is appreciably slower on this
branch than on master.  make bootstrap is taking me nearly 7 minutes
compared with master's 4min 40sec.  Running make -j17 check is taking
1min 50sec rather than 50sec.  Once compiled, the code runs at the same
speed, though.

I think this issue needs sorting out first.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Pretending the problem doesn't exist won't solve it.  In the ;POS...
> structures for a lambda, there are two pointers - one to the definition
> of the lambda, the other to the point of use.

Fancy.  Could you give me an example where I see this in play?
[ To help me understand also what you mean by "definition of the
  lambda" and "point of use"?  ]

I looked around but all I could see where position info like

    [foo foo.el 41 nil]

which point to "the definition" of the function.

>> BTW, AFAIK the above is conceptually what the byte-compiler does (except
>> it performs a few more transformations between `macroexp--expand-all`
>> and `strip-all-symbol-positions`).
> It is a bad idea to conflate these two radically different uses of SWPs.

In what way are they radically different uses of SWPs?

>> Is it the case that `cl-defmethod` generates a function whose source
>> position (partly) points to `generic.el:403` if `cl-generic.el` was
>> interpreted but not if it compiled?
> No, the intention is that the source positions are independent of whether
> the code is compiled.

Good.  So why do the interpreted and compiled cases need to be
"radically different uses of SWPs"?

>> > (defmacro foo (lambda bar)
>> >   `(cons ,lambda ,bar))
>
>> > expands to
>
>> > (macro closure (t) (lambda bar) ";POS^^^A^A^A [foo *scratch* 158 nil]
>> > " (list 'cons lambda bar))
>
>> IIUC your reader will make the `lambda` formal argument into an SWP.
>> Where is that SWP stripped?
>
> In macroexp--expand-all in the "guard arm" near the end.

How?  `macroexp--expand-all` will not be passed this `lambda` because
it's not an *expression*.

`eval-buffer` of a buffer containing the above defmacro does:

1 -> (macroexp--expand-all (defalias 'foo (cons 'macro #'{foo} (lambda (#<symbol lambda at 46> bar) ";POS\x01\x01 [foo foo.el 41 nil]\n" `(cons ,lambda ,bar)))))
| 2 -> (macroexp--expand-all 'foo)
| 2 <- macroexp--expand-all: 'foo
| 2 -> (macroexp--expand-all (cons 'macro #'{foo} (lambda (#<symbol lambda at 46> bar) ";POS\x01\x01 [foo foo.el 41 nil]\n" `(cons ,lambda ,bar))))
| | 3 -> (macroexp--expand-all 'macro)
| | 3 <- macroexp--expand-all: 'macro
| | 3 -> (macroexp--expand-all #'{foo} (lambda (#<symbol lambda at 46> bar) ";POS\x01\x01 [foo foo.el 41 nil]\n" `(cons ,lambda ,bar)))
| | | 4 -> (macroexp--expand-all ";POS\x01\x01 [foo foo.el 41 nil]\n")
| | | 4 <- macroexp--expand-all: ";POS\x01\x01 [foo foo.el 41 nil]\n"
| | | 4 -> (macroexp--expand-all `(cons ,lambda ,bar))
| | | | 5 -> (macroexp--expand-all 'cons)
| | | | 5 <- macroexp--expand-all: 'cons
| | | | 5 -> (macroexp--expand-all lambda)
| | | | 5 <- macroexp--expand-all: lambda
| | | | 5 -> (macroexp--expand-all bar)
| | | | 5 <- macroexp--expand-all: bar
| | | 4 <- macroexp--expand-all: (list 'cons lambda bar)
| | 3 <- macroexp--expand-all: #'{foo} (lambda (#<symbol lambda at 46> bar) ";POS\x01\x01 [foo foo.el 41 nil]\n" (list 'cons lambda bar))
| 2 <- macroexp--expand-all: (cons 'macro #'{foo} (lambda (#<symbol lambda at 46> bar) ";POS\x01\x01 [foo foo.el 41 nil]\n" (list 'cons lambda bar)))
1 <- macroexp--expand-all: (defalias 'foo (cons 'macro #'{foo} (lambda (#<symbol lambda at 46> bar) ";POS\x01\x01 [foo foo.el 41 nil]\n" (list 'cons lambda bar))))

So we see that indeed it returns code where the formal argument `lambda`
is (incorrectly) a SYMPOS.  Yet somehow the sympos is stripped after
macroexpansion somewhere since `(symbol-function 'foo)` shows the
resulting function doesn't have any symposes in it.

>> > so it is clear this case is getting handled OK.  I'm afraid I can't
>> > point out the exact place in the code at the moment where this is
>> > getting done.
>> I think it would be good to know, so as to be able to decide whether
>> it'll indeed always work right, or we just got lucky this time.
> See above.

Yes, please, see above 🙂

>> Could you explain what you think makes it intrinsically complex?
> The mass of detail that needs dealing with that Emacs has collected over
> the decades.  As a counter question, why do you think the exercise ought
> to be simple (assuming you do think this)?

Because you solved the hard part when you added the symposes for the compiler.


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

On Sun, Apr 07, 2024 at 23:16:13 -0400, Stefan Monnier wrote:
> > Pretending the problem doesn't exist won't solve it.  In the ;POS...
> > structures for a lambda, there are two pointers - one to the definition
> > of the lambda, the other to the point of use.

> Fancy.  Could you give me an example where I see this in play?
> [ To help me understand also what you mean by "definition of the
>   lambda" and "point of use"?  ]

> I looked around but all I could see where position info like

>     [foo foo.el 41 nil]

> which point to "the definition" of the function.

Apologies.  I was thinking of my latest not yet committed version, where
I've added a fifth element into the position information, the buffer
containing the lambda.  This should enable buttons to be set on the
interactive backtrace, pointing back at the two source code positions.

> >> BTW, AFAIK the above is conceptually what the byte-compiler does (except
> >> it performs a few more transformations between `macroexp--expand-all`
> >> and `strip-all-symbol-positions`).
> > It is a bad idea to conflate these two radically different uses of SWPs.

> In what way are they radically different uses of SWPs?

You're confused in precisely the way I feared.  "conceptually what the
byte-compiler does" is what it does to strip the SWPs used as WARNING
POSITIONS.  When the SWPs are used for function position structures
(whether in interpreted or compiled code) the handling is radically
different - the P in the SWP is stripped as soon as possible after its
use.  In the warning pos use, the positions are preserved for as long as
possible.

> >> Is it the case that `cl-defmethod` generates a function whose source
> >> position (partly) points to `generic.el:403` if `cl-generic.el` was
> >> interpreted but not if it compiled?
> > No, the intention is that the source positions are independent of whether
> > the code is compiled.

> Good.  So why do the interpreted and compiled cases need to be
> "radically different uses of SWPs"?

They're not.  It is the use for warning positions that differs from the
use for putting pos info into the doc string.

> >> > (defmacro foo (lambda bar)
> >> >   `(cons ,lambda ,bar))

> >> > expands to

> >> > (macro closure (t) (lambda bar) ";POS^^^A^A^A [foo *scratch* 158 nil]
> >> > " (list 'cons lambda bar))

> >> IIUC your reader will make the `lambda` formal argument into an SWP.
> >> Where is that SWP stripped?

> > In macroexp--expand-all in the "guard arm" near the end.

> How?  `macroexp--expand-all` will not be passed this `lambda` because
> it's not an *expression*.

Well, when I commented out that pcase arm, the lambda no longer got
stripped.  I'm not sure what you mean by expression.

> `eval-buffer` of a buffer containing the above defmacro does:

> 1 -> (macroexp--expand-all (defalias 'foo (cons 'macro #'{foo} (lambda (#<symbol lambda at 46> bar) ";POS\x01\x01 [foo foo.el 41 nil]\n" `(cons ,lambda ,bar)))))
> | 2 -> (macroexp--expand-all 'foo)
> | 2 <- macroexp--expand-all: 'foo
> | 2 -> (macroexp--expand-all (cons 'macro #'{foo} (lambda (#<symbol lambda at 46> bar) ";POS\x01\x01 [foo foo.el 41 nil]\n" `(cons ,lambda ,bar))))
> | | 3 -> (macroexp--expand-all 'macro)
> | | 3 <- macroexp--expand-all: 'macro
> | | 3 -> (macroexp--expand-all #'{foo} (lambda (#<symbol lambda at 46> bar) ";POS\x01\x01 [foo foo.el 41 nil]\n" `(cons ,lambda ,bar)))
> | | | 4 -> (macroexp--expand-all ";POS\x01\x01 [foo foo.el 41 nil]\n")
> | | | 4 <- macroexp--expand-all: ";POS\x01\x01 [foo foo.el 41 nil]\n"
> | | | 4 -> (macroexp--expand-all `(cons ,lambda ,bar))
> | | | | 5 -> (macroexp--expand-all 'cons)
> | | | | 5 <- macroexp--expand-all: 'cons
> | | | | 5 -> (macroexp--expand-all lambda)
> | | | | 5 <- macroexp--expand-all: lambda
> | | | | 5 -> (macroexp--expand-all bar)
> | | | | 5 <- macroexp--expand-all: bar
> | | | 4 <- macroexp--expand-all: (list 'cons lambda bar)
> | | 3 <- macroexp--expand-all: #'{foo} (lambda (#<symbol lambda at 46> bar) ";POS\x01\x01 [foo foo.el 41 nil]\n" (list 'cons lambda bar))
> | 2 <- macroexp--expand-all: (cons 'macro #'{foo} (lambda (#<symbol lambda at 46> bar) ";POS\x01\x01 [foo foo.el 41 nil]\n" (list 'cons lambda bar)))
> 1 <- macroexp--expand-all: (defalias 'foo (cons 'macro #'{foo} (lambda (#<symbol lambda at 46> bar) ";POS\x01\x01 [foo foo.el 41 nil]\n" (list 'cons lambda bar))))

> So we see that indeed it returns code where the formal argument `lambda`
> is (incorrectly) a SYMPOS.  Yet somehow the sympos is stripped after
> macroexpansion somewhere since `(symbol-function 'foo)` shows the
> resulting function doesn't have any symposes in it.

OK, this needs clearing up.

> >> > so it is clear this case is getting handled OK.  I'm afraid I can't
> >> > point out the exact place in the code at the moment where this is
> >> > getting done.
> >> I think it would be good to know, so as to be able to decide whether
> >> it'll indeed always work right, or we just got lucky this time.
> > See above.

> Yes, please, see above 🙂

> >> Could you explain what you think makes it intrinsically complex?
> > The mass of detail that needs dealing with that Emacs has collected over
> > the decades.  As a counter question, why do you think the exercise ought
> > to be simple (assuming you do think this)?

> Because you solved the hard part when you added the symposes for the compiler.

OK, but that's not the way things turned out.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> How?  `macroexp--expand-all` will not be passed this `lambda` because
>> it's not an *expression*.
> Well, when I commented out that pcase arm, the lambda no longer got
> stripped.  I'm not sure what you mean by expression.

In Edebug specs we call it `form`.

The ((x 3) (y 4)) in a `let` is not an expression, for example.
(4 . 5) could appear in a piece of code in the position where we expect
an expression but it is not a valid expression (it will result in a syntax
error at compile- or run-time).

The grammar of ELisp is made of various elements, one of which would
traditionally be called "expressions".  If we could specify it in EBNF
it could look something like the following:

    <formalargs> ::= "(" <id>* [ "&optional" <id>* [ "&rest" <id> ]] ")"
    <decls> ::= "(" <decl>* ")"
    <decl> ::= <id> | "(" <id> <exp> ")"
    <func> ::= <id> | "(" "lambda" <formalargs> <exp>* ")"
    ...
    <exp> ::= <id>
            | "(" <func> <exp>* ")"
            | "(" "quote" <sexp> ")"
            | "(" "let" <decls> <exp>* ")"
            | "(" "function" <func> ")"
            | ...

Only the <exp> parts go through `macroexp--expand-all`.


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

On Mon, Apr 08, 2024 at 02:56:20 +0000, Alan Mackenzie wrote:

[ .... ]

> Byte compiling is appreciably slower on this branch than on master.
> make bootstrap is taking me nearly 7 minutes compared with master's
> 4min 40sec.  Running make -j17 check is taking 1min 50sec rather than
> 50sec.  Once compiled, the code runs at the same speed, though.

> I think this issue needs sorting out first.

I think I know now why this is.  I put a call to
byte-run-strip-symbol-positions into readevalloop_eager_expand_eval to
get the thing to build.  This is probably the cause of the slowdown.

It is also surely what is stripping positions from non-function lambdas.

This call needs to be taken out, and other things which need it (e.g.
Fdefalias) fixed.

> >         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).