bug#46917: 28.0.50; error when unloading feature after loading elp

bug#46917: 28.0.50; error when unloading feature after loading elp

[Mauro Aranda]

Starting from emacs -Q, do:

1. In the *scratch* buffer, type and evaluate (in any order):
(require 'autoarg) ; Whatever feature.
(require 'elp)

2. Type and evaluate
(unload-feature 'autoarg)

3. Emacs signals an error:
"Unsupported qualifiers in function loadhist-unload-element: (:before :extra \"elp\")"

In GNU Emacs 28.0.50 (build 10, x86_64-pc-linux-gnu, GTK+ Version 3.22.30, cairo version 1.15.10)
 of 2021-03-04 built on tbb-desktop
Repository revision: ba33089d50a4f74b88cd7fc9d862d183d096c27d
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Ubuntu 18.04.5 LTS

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SOUND THREADS
TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LC_MONETARY: es_AR.UTF-8
  value of $LC_NUMERIC: es_AR.UTF-8
  value of $LC_TIME: es_AR.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils cl-extra seq help-fns radix-tree cl-print debug
backtrace help-mode find-func cl-loaddefs cl-lib loadhist byte-opt gv
bytecomp byte-compile cconv elp autoarg iso-transl tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win
x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 55143 4629)
 (symbols 48 7124 1)
 (strings 32 20130 1105)
 (string-bytes 1 660685)
 (vectors 16 13335)
 (vector-slots 8 176885 10930)
 (floats 8 30 31)
 (intervals 56 214 2)
 (buffers 992 12))

bug#46917: 28.0.50; error when unloading feature after loading elp

[Mauro Aranda]

[-- Attachment #1: Type: text/plain, Size: 513 bytes --]

tags 46917 patch
quit


Mauro Aranda <maurooaranda@gmail.com> writes:

> Starting from emacs -Q, do:
>
> 1. In the *scratch* buffer, type and evaluate (in any order):
> (require 'autoarg) ; Whatever feature.
> (require 'elp)
>
> 2. Type and evaluate
> (unload-feature 'autoarg)
>
> 3. Emacs signals an error:
> "Unsupported qualifiers in function loadhist-unload-element: (:before :extra \"elp\")"
>

Looks like the problem is the order of the qualifiers; the attached
patch should fix it.

Best regards,
Mauro.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch --]
[-- Type: text/x-patch, Size: 953 bytes --]

From c05dda94c490a93c360bb5b71ddae7eb1f9cdcb6 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Thu, 4 Mar 2021 09:47:39 -0300
Subject: [PATCH] Fix qualifiers order for loadhist-unload-element in elp.el

* lisp/emacs-lisp/elp.el (loadhist-unload-element): The :extra
qualifier is expected to come before the other qualifiers, so do
that.  (Bug#46917)
---
 lisp/emacs-lisp/elp.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/elp.el b/lisp/emacs-lisp/elp.el
index cc2927caf4..411ea2af69 100644
--- a/lisp/emacs-lisp/elp.el
+++ b/lisp/emacs-lisp/elp.el
@@ -583,7 +583,7 @@ elp-unload-function
   ;; continue standard unloading
   nil)
 
-(cl-defmethod loadhist-unload-element :before :extra "elp" ((x (head defun)))
+(cl-defmethod loadhist-unload-element :extra "elp" :before ((x (head defun)))
   "Un-instrument before unloading a function."
   (elp-restore-function (cdr x)))
 
-- 
2.30.0

bug#46917: 28.0.50; error when unloading feature after loading elp

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

>> Starting from emacs -Q, do:
>>
>> 1. In the *scratch* buffer, type and evaluate (in any order):
>> (require 'autoarg) ; Whatever feature.
>> (require 'elp)
>>
>> 2. Type and evaluate
>> (unload-feature 'autoarg)
>>
>> 3. Emacs signals an error:
>> "Unsupported qualifiers in function loadhist-unload-element:
>> (:before :extra \"elp\")"
>>
>
> Looks like the problem is the order of the qualifiers; the attached
> patch should fix it.

[...]

> * lisp/emacs-lisp/elp.el (loadhist-unload-element): The :extra
> qualifier is expected to come before the other qualifiers, so do
> that.  (Bug#46917)

I had a quick look at the documentation -- this limitation doesn't seem
to be stated anywhere?  Perhaps it should be, or perhaps the restriction
should be lifted?

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

bug#46917: 28.0.50; error when unloading feature after loading elp

[Mauro Aranda]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>>> Starting from emacs -Q, do:
>>>
>>> 1. In the *scratch* buffer, type and evaluate (in any order):
>>> (require 'autoarg) ; Whatever feature.
>>> (require 'elp)
>>>
>>> 2. Type and evaluate
>>> (unload-feature 'autoarg)
>>>
>>> 3. Emacs signals an error:
>>> "Unsupported qualifiers in function loadhist-unload-element:
>>> (:before :extra \"elp\")"
>>>
>>
>> Looks like the problem is the order of the qualifiers; the attached
>> patch should fix it.
>
> [...]
>
>> * lisp/emacs-lisp/elp.el (loadhist-unload-element): The :extra
>> qualifier is expected to come before the other qualifiers, so do
>> that.  (Bug#46917)
>
> I had a quick look at the documentation -- this limitation doesn't seem
> to be stated anywhere?

It's hard to take a long look, since the documentation about :extra is
scarce ;-)

> Perhaps it should be, or perhaps the restriction should be lifted?

I don't really have an opinion.  My guess is that expecting :extra to
come first just makes the code simpler.

bug#46917: 28.0.50; error when unloading feature after loading elp

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

>> I had a quick look at the documentation -- this limitation doesn't seem
>> to be stated anywhere?
>
> It's hard to take a long look, since the documentation about :extra is
> scarce ;-)

:-)

Indeed.

>> Perhaps it should be, or perhaps the restriction should be lifted?
>
> I don't really have an opinion.  My guess is that expecting :extra to
> come first just makes the code simpler.

I've now applied your patch, and I've also altered the documentation and
doc string to document the actual syntax.  The manual said, for instance:

----
You can also define @dfn{auxiliary methods}, by using one of the
following values as @var{qualifier}:
----

But then listed both the :before (etc) and :extra in the same section,
when you can use both :extra and :before.

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

bug#46917: 28.0.50; error when unloading feature after loading elp

[Mauro Aranda]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>>> Perhaps it should be, or perhaps the restriction should be lifted?
>>
>> I don't really have an opinion.  My guess is that expecting :extra to
>> come first just makes the code simpler.
>
> I've now applied your patch, and I've also altered the documentation and
> doc string to document the actual syntax.  The manual said, for instance:
>
> ----
> You can also define @dfn{auxiliary methods}, by using one of the
> following values as @var{qualifier}:
> ----
>
> But then listed both the :before (etc) and :extra in the same section,
> when you can use both :extra and :before.

Great, thanks.

bug#46917: 28.0.50; error when unloading feature after loading elp

[Stefan Monnier]

> ----
> You can also define @dfn{auxiliary methods}, by using one of the
> following values as @var{qualifier}:
> ----
>
> But then listed both the :before (etc) and :extra in the same section,
> when you can use both :extra and :before.

It's because it's supposed to be a "list of qualifiers".
Strictly speaking, the set of possible qualifiers can be changed by
adding a new method to `cl-generic-combine-methods`.


        Stefan

bug#46917: 28.0.50; error when unloading feature after loading elp

[Lars Ingebrigtsen]

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

> It's because it's supposed to be a "list of qualifiers".

The three ones that are supported, though, are mutually exclusive, I
think?

> Strictly speaking, the set of possible qualifiers can be changed by
> adding a new method to `cl-generic-combine-methods`.

Perhaps that should be mentioned somewhere.  :-)

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

bug#46917: 28.0.50; error when unloading feature after loading elp

[Stefan Monnier]

>> It's because it's supposed to be a "list of qualifiers".
> The three ones that are supported, though, are mutually exclusive, I think?
>> Strictly speaking, the set of possible qualifiers can be changed by
>> adding a new method to `cl-generic-combine-methods`.
> Perhaps that should be mentioned somewhere.  :-)

Except that it's a bad hook, really, because there's not much on
which you can "dispatch" nor can you satisfactorily use the result of
`cl-call-next-method`, so all you can do is override the default with
a fresh new implementation.

I think it's just barely sufficient so you can use a different method
combination for specific generic functions (i.e. we could implement
something like CLOS's support for specifying in `defgeneric`
a particular method combination to use for that generic function), but
it's not sufficient to be able to extend the set of qualifiers supported
"by default".


        Stefan