bug#68113: Wrong error message triggered in cl--generic-standard-method-combination

[Alan Mackenzie <acm@muc.de>]

Hello, Stefan.

On Mon, Jan 01, 2024 at 23:39:57 -0500, Stefan Monnier wrote:
> > , and at the time of calling, qualifiers was nil.  So the call boiled
> > down to

> >     (push method (alist-get nil mets-by-qual))

> > , and there was no element of mets-by-qual with a car of nil.  So what
> > can the push macro do?  There's no list to push onto.  It can generate
> > code either
> > (i) to signal an error; or
> > (ii) to create a new element in the alist mets-by-qual with method being
> > the single element of its cdr.

> > In actual fact, it does (i), but (as reported in the bug report) gives
> > the wrong message.

> But as I pointed out, the normal macroexpansion does (ii), so the
> problem *is* in the wrong macroexpansion.

Yes indeed.

> > That line of code is poor.  It assumes that (alist-get (car qualifiers)
> > mets-by-qual) will always return a list element,

> No, it doesn't.  The `gv-expander` for `alist-get` handles that case
> just fine (tho for some reason not in your case, obviously).

> > and fails to make any checks.  It fails to check that qualifiers is
> > a non-empty list before taking its car.

> It's on purpose.  The "nil" case is the common case, actually.
> This has been working fine since Emacs-25, remember.

Yes.

> > Would you please check the code that signaled that error (you wrote it, I
> > think), and let me know whether you find anything suspicious in it.

> I already did.

> Did you try the patch I sent?

Yes I did, thanks.  It showed up the real problem.

I had modified the macroexpansion system in an attempt to use the
framework in macroexp--expand-all but without the "base case", by
fset'ing macroexp-macroexpand temporarily to an identity function.  This
was an attempt to strip symbols with position of their positions for bug
#67455, getting lambda positions into the doc strings.  In the end this
attempt was hopeless, but I spent quite some time on it.

I hadn't realised that macro expansion was central to gv.el.  Bug #68113
might be a real bug, but I somehow doubt it.  I'll close it as not a
bug.

Sorry to have wasted your time in the process.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).