GNU bug report logs - #50219
28.0.50; Provide better errors when trying to specialize on optional args in generic methods

Previous Next

To reply to this bug, email your comments to 50219 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; Provide better errors when trying to specialize on
 optional args in generic methods
Date: Thu, 26 Aug 2021 16:11:51 -0700

If you write a generic method and try to specialize on an &optional
argument, like so:

--8<---------------cut here---------------start------------->8---
(cl-defgeneric testy (arg1 &optional arg2))

(cl-defmethod testy ((arg1 string) &optional (arg2 integer))
  (message "Don't do this"))
--8<---------------cut here---------------end--------------->8---

It will behave weirdly when you try to call the function with a second
argument, but won't bark at you when you evaluate this form, and during
compilation will give you the error:

Warning: reference to free variable 'integer'

I was doing this with a specialization on an EIEIO class in EBDB, and
the warning was even weirder, but never mind that.

A few things could be done here:

- the manual section on "Generic Functions" could say explicitly that
  you can't specialize on optional arguments
- `eval'ling the `defmethod' form above could raise an error directly
- the compiler could say more explicitly what the problem is

I'd favor all three of these changes! Happy to implement what I can
(maybe not the compiler part).

Eric

Message #8 received at 50219 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: 50219 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#50219: 28.0.50; Provide better errors when trying to
 specialize on optional args in generic methods
Date: Fri, 27 Aug 2021 02:34:49 +0200

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> - the manual section on "Generic Functions" could say explicitly that
>   you can't specialize on optional arguments
> - `eval'ling the `defmethod' form above could raise an error directly
> - the compiler could say more explicitly what the problem is
>
> I'd favor all three of these changes!

Me, too.

> Happy to implement what I can (maybe not the compiler part).

I've added Stefan to the CCs; perhaps he has some comments.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #11 received at 50219 <at> debbugs.gnu.org (full text, mbox):

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 50219 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#50219: 28.0.50; Provide better errors when trying to
 specialize on optional args in generic methods
Date: Thu, 26 Aug 2021 18:31:44 -0700

[Message part 1 (text/plain, inline)]

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> - the manual section on "Generic Functions" could say explicitly that
>>   you can't specialize on optional arguments
>> - `eval'ling the `defmethod' form above could raise an error directly
>> - the compiler could say more explicitly what the problem is
>>
>> I'd favor all three of these changes!
>
> Me, too.

Here's a very simply manual patch.

I squinted long and hard at cl-generic.el, and I'm not confident about where
we'd put the error. In `cl--generic-lambda'?

>> Happy to implement what I can (maybe not the compiler part).
>
> I've added Stefan to the CCs; perhaps he has some comments.

He has already (kindly) made fun of me for my dumb code, so he ought to
be aware. :)

[dont-specialize-optional-args.diff (text/x-patch, attachment)]

Message #14 received at 50219 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 50219 <at> debbugs.gnu.org
Subject: Re: bug#50219: 28.0.50; Provide better errors when trying to
 specialize on optional args in generic methods
Date: Sat, 28 Aug 2021 10:15:17 -0400

Eric Abrahamsen [2021-08-26 18:31:44] wrote:
> (cl-defmethod testy ((arg1 string) &optional (arg2 integer))
>   (message "Don't do this"))
[...]
> Warning: reference to free variable 'integer'

Hmm....

The same thing happens in Common-Lisp, tho.  What's going on here is
that the arglist of `cl-defmethod` allows Common-Lisp style arguments
(e.g. `&key`) so your `(arg2 integer)` means to use `integer` as the
default value for `arg2`, just as in

    (cl-defun testy (arg1 &optional (arg2 integer)) ...)


-- Stefan

Message #17 received at 50219 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eric Abrahamsen <eric <at> ericabrahamsen.net>, 50219 <at> debbugs.gnu.org
Subject: Re: bug#50219: 28.0.50; Provide better errors when trying to
 specialize on optional args in generic methods
Date: Sat, 28 Aug 2021 18:02:40 +0200

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

> Eric Abrahamsen [2021-08-26 18:31:44] wrote:
>> (cl-defmethod testy ((arg1 string) &optional (arg2 integer))
>>   (message "Don't do this"))
> [...]
>> Warning: reference to free variable 'integer'
>
> Hmm....
>
> The same thing happens in Common-Lisp, tho.  What's going on here is
> that the arglist of `cl-defmethod` allows Common-Lisp style arguments
> (e.g. `&key`) so your `(arg2 integer)` means to use `integer` as the
> default value for `arg2`, just as in
>
>     (cl-defun testy (arg1 &optional (arg2 integer)) ...)

Hm...  So the warning here is correct, I guess.  But perhaps it could
also have said something about it being ambiguous syntax (since is also
knows the declared parameter list from `cl-defgeneric')...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #20 received at 50219 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eric Abrahamsen <eric <at> ericabrahamsen.net>, 50219 <at> debbugs.gnu.org
Subject: Re: bug#50219: 28.0.50; Provide better errors when trying to
 specialize on optional args in generic methods
Date: Sat, 28 Aug 2021 12:42:33 -0400

> Hm...  So the warning here is correct, I guess.  But perhaps it could
> also have said something about it being ambiguous syntax (since is also
> knows the declared parameter list from `cl-defgeneric')...

[ I don't see how the arglist of `cl-defgeneric` would have helped here
  discover the confusion.  ]

Or we could deprecate the use of Common-Lisp style arglists in
cl-defmethod.  I don't know how often it's used, tho.


        Stefan

Message #23 received at 50219 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eric Abrahamsen <eric <at> ericabrahamsen.net>, 50219 <at> debbugs.gnu.org
Subject: Re: bug#50219: 28.0.50; Provide better errors when trying to
 specialize on optional args in generic methods
Date: Sat, 28 Aug 2021 19:12:13 +0200

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> Hm...  So the warning here is correct, I guess.  But perhaps it could
>> also have said something about it being ambiguous syntax (since is also
>> knows the declared parameter list from `cl-defgeneric')...
>
> [ I don't see how the arglist of `cl-defgeneric` would have helped here
>   discover the confusion.  ]

I may be misremembering the semantics of defgeneric, but I thought it
was fine to say:

(cl-defgeneric zot (a &optional b)
  )

But this means that you can only specialise on a -- b is an optional
argument.  So

(cl-defmethod zot ((a integer) &optional (b "foo"))
  (list a b))

is fine and valid -- it's a default value for b.

(cl-defmethod foo ((a integer) &optional (b string))
  ...)

on the other hand, looks like the person who wrote it wanted to
specialise on b, so if the default is something that is a type
specifier, then we could output an additional warning about that.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #26 received at 50219 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eric Abrahamsen <eric <at> ericabrahamsen.net>, 50219 <at> debbugs.gnu.org
Subject: Re: bug#50219: 28.0.50; Provide better errors when trying to
 specialize on optional args in generic methods
Date: Sat, 28 Aug 2021 13:41:48 -0400

> (cl-defmethod foo ((a integer) &optional (b string))
>   ...)
>
> on the other hand, looks like the person who wrote it wanted to
> specialise on b, so if the default is something that is a type
> specifier, then we could output an additional warning about that.

The default value part of args in Common lisp arglists is an
*expression*, so (b string) just means that `b` should take the value of
the `string` variable.

The best I can see (if we keep the CL arglist feature) is to try and see
if that expression looks like a valid CLOS specializer and if so emit
a warning about possible confusion.

I wonder what Common Lisp compilers do.


        Stefan

Message #29 received at 50219 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eric Abrahamsen <eric <at> ericabrahamsen.net>, 50219 <at> debbugs.gnu.org
Subject: Re: bug#50219: 28.0.50; Provide better errors when trying to
 specialize on optional args in generic methods
Date: Sat, 28 Aug 2021 19:49:11 +0200

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

> The default value part of args in Common lisp arglists is an
> *expression*, so (b string) just means that `b` should take the value of
> the `string` variable.
>
> The best I can see (if we keep the CL arglist feature) is to try and see
> if that expression looks like a valid CLOS specializer and if so emit
> a warning about possible confusion.

Yes, that's what I tried to say.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #32 received at 50219 <at> debbugs.gnu.org (full text, mbox):

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 50219 <at> debbugs.gnu.org
Subject: Re: bug#50219: 28.0.50; Provide better errors when trying to
 specialize on optional args in generic methods
Date: Sat, 28 Aug 2021 17:47:12 -0700

On 08/28/21 13:41 PM, Stefan Monnier wrote:
>> (cl-defmethod foo ((a integer) &optional (b string))
>>   ...)
>>
>> on the other hand, looks like the person who wrote it wanted to
>> specialise on b, so if the default is something that is a type
>> specifier, then we could output an additional warning about that.
>
> The default value part of args in Common lisp arglists is an
> *expression*, so (b string) just means that `b` should take the value of
> the `string` variable.
>
> The best I can see (if we keep the CL arglist feature) is to try and see
> if that expression looks like a valid CLOS specializer and if so emit
> a warning about possible confusion.

That's a pretty byzantine set of rules! Anyway, I think "reference to
free variable string" is probably close enough to clue the developer in
to what is happening. In the case of an EIEIO class name, though, the
error was:

"`ebdb-record' is an obsolete variable, use `ebdb-record' instead"

which is just...

So maybe something special for class and struct types would be sufficient?

Message #35 received at 50219 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 50219 <at> debbugs.gnu.org
Subject: Re: bug#50219: 28.0.50; Provide better errors when trying to
 specialize on optional args in generic methods
Date: Sun, 29 Aug 2021 13:10:53 +0200

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> "`ebdb-record' is an obsolete variable, use `ebdb-record' instead"

This sounds like Bug#39169.  Did I forget to commit the fix I had
posted there?

Michael.

Message #38 received at 50219 <at> debbugs.gnu.org (full text, mbox):

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 50219 <at> debbugs.gnu.org
Subject: Re: bug#50219: 28.0.50; Provide better errors when trying to
 specialize on optional args in generic methods
Date: Sun, 29 Aug 2021 08:41:51 -0700

On 08/29/21 13:10 PM, Michael Heerdegen wrote:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> "`ebdb-record' is an obsolete variable, use `ebdb-record' instead"
>
> This sounds like Bug#39169.  Did I forget to commit the fix I had
> posted there?

I thought I'd seen discussion about this before! The code you described
in that bug report did get committed, so I can only assume this is a
separate but look-alike situation.

Message #41 received at 50219 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 50219 <at> debbugs.gnu.org
Subject: Re: bug#50219: 28.0.50; Provide better errors when trying to
 specialize on optional args in generic methods
Date: Sun, 29 Aug 2021 21:16:44 +0200

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> > This sounds like Bug#39169.  Did I forget to commit the fix I had
> > posted there?
>
> I thought I'd seen discussion about this before! The code you described
> in that bug report did get committed, so I can only assume this is a
> separate but look-alike situation.

Please search for the string "use \\='%s" in eieio-core.el.  We made one
of three occurrences less confusing, seems you hit some other (2/3?).  I
guess we want to change this one as well.

AFAIR you can also just disable `eieio-backward-compatibility' (file or
directory locally).

Michael.

Message #44 received at 50219 <at> debbugs.gnu.org (full text, mbox):

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 50219 <at> debbugs.gnu.org
Subject: Re: bug#50219: 28.0.50; Provide better errors when trying to
 specialize on optional args in generic methods
Date: Sun, 29 Aug 2021 20:33:04 -0700

On 08/29/21 21:16 PM, Michael Heerdegen wrote:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> > This sounds like Bug#39169.  Did I forget to commit the fix I had
>> > posted there?
>>
>> I thought I'd seen discussion about this before! The code you described
>> in that bug report did get committed, so I can only assume this is a
>> separate but look-alike situation.
>
> Please search for the string "use \\='%s" in eieio-core.el.  We made one
> of three occurrences less confusing, seems you hit some other (2/3?).  I
> guess we want to change this one as well.

Both the line 343 and line 423 sites do the same thing: define the class
symbol as an obsolete variable, and recommend using the quoted symbol
instead.

This is applicable to most situations where people are specifically
trying to do something with the class, but in this case (the &optional
args) the method is supposed to be doing something with the symbol-value
of a (here unquoted) symbol (I was using it wrong). I can't think of a
situation where it would make sense to be using the class symbol in this
way, so I think the proper place to raise an error is in cl-generic.el,
not in the class definitions here.

> AFAIR you can also just disable `eieio-backward-compatibility' (file or
> directory locally).

But I can't do that for everyone who's compiling EBDB locally (on
package update, etc).

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.