bug#67483: Wrong warning position given by the byte compiler for a malformed function

[Alan Mackenzie <acm@muc.de>]

Hello, Mattias.

On Fri, Dec 22, 2023 at 14:12:46 +0100, Mattias Engdegård wrote:
> 22 dec. 2023 kl. 12.24 skrev Alan Mackenzie <acm@muc.de>:

> > What's the process of converting to a closure got to do with
> > maintaining the stack of forms for error processing?

> Not much, but cconv has evolved to one major part of the compiler
> front-end (the other being macroexp-all) and isn't restricted to just
> closure conversion.

No, it hasn't "evolved", somebody has changed it.  Why?  A file, just
like a function, should have a particular purpose, and degrading a
specific purpose file to being a general place to put random things is a
BAD THING.  What's going on, here?

> In fact, it's now used for dynbound code as well.

Does this file, in fact, still have a purpose?

> In particular it's a natural place for various front-end checks and
> transforms, so don't let the place and name distract you. There are
> plans to refactor it later on for other reasons.

> > Whoops!  There was no patch.

> Attached it now, sorry.

> > You've put the new macro into macroexp.el.  This file is purely about
> > macro handling.

> Actually macroexp.el does more than that, and in any case the file
> isn't very important; the macro ended up there to be next to
> byte-compile-form-stack. Nor is the name; it can be changed at any
> time.

> However, it probably needs to be in that file for bootstrap reasons.

byte-compile-form-stack is in there for bootstrap reasons.  But it's
logically part of bytecomp.el, hence the name.

> > And is the "--" in the name appropriate, given that the macro is used
> > by several files?  I'm not sure about that rule.

> The double-dash just means that users shouldn't get any funny ideas.
> (The converse isn't true: a name without double-dash isn't
> automatically fair game.)

> > Also, byte-compile-form-stack gets bound in cconv-closure-convert.
> > Why?

> It's just a backstop. Not strictly needed. It's probably fine to remove
> it if you are worried, but then again there shouldn't be any (non-bug)
> error signalling here. I'll have a look.

How can you be so casual about this?  It's critically important to
byte-compile-form-stack's correct working that it does NOT get bound.
Surely you understand this?

Also, you ignored and snipped the most important point in my last post,
namely this:

> > But cconv-closure-convert doesn't get called recursively.  So it
> > would seem the wrong place to be maintaining byte-compile-form-stack.
> > What's needed is a place where that stack grows steadily as the
> > source code is recursed into, to ensure there will be a correct
> > position on it in the event of an warning/error.

Please attend to this point now.  Does byte-compile-form-stack get pushed
onto at each recursive descent into the source code?  This is important.

-- 
Alan Mackenzie (Nuremberg, Germany).