GNU bug report logs - #59710
Wrong type argument when editing a multisession variable

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59710 in the body.
You can then email your comments to 59710 AT debbugs.gnu.org in the normal way.

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

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

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Bug-Gnu-Emacs <bug-gnu-emacs <at> gnu.org>
Subject: Wrong type argument when editing a multisession variable
Date: Wed, 30 Nov 2022 02:49:15 +0100

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

Package: emacs
Version: 29.0.60
Severity: normal
X-Debbugs-Cc: "Lars Ingebrigtsen" <larsi <at> gnus.org>


;;; init.el
(require 'multisession)
(define-multisession-variable test-var nil "Test var" :package "test")
(setf (multisession-value test-var) t)
;;; end of init.el

emacs
M-x list-multisession-values <RET>
e

Debugger entered--Lisp error: (wrong-type-argument symbolp (intern (cdr
id)))
  multisession-edit-value(("test" . "test-var"))
  funcall-interactively(multisession-edit-value ("test" . "test-var"))
  command-execute(multisession-edit-value)


The problem comes from this change

  commit bd586121ac21e046f60f75eeb0200866c38d6f9f
  Author: Lars Ingebrigtsen <larsi <at> gnus.org>
  Date:   2022-01-22 11:56:13 +0100

      Make the test for existing multisession variables more sensible

      * lisp/emacs-lisp/multisession.el (multisession-edit-value):
      Unconfuse the code.

  diff --git a/lisp/emacs-lisp/multisession.el
b/lisp/emacs-lisp/multisession.el
  index 4a293796a8..25307594c6 100644
  --- a/lisp/emacs-lisp/multisession.el
  +++ b/lisp/emacs-lisp/multisession.el
  @@ -437,8 +437,8 @@ multisession-edit-value
     (let* ((object (or
                     ;; If the multisession variable already exists, use
                     ;; it (so that we update it).
  -                  (and (boundp (intern-soft (cdr id)))
  -                       (symbol-value (intern (cdr id))))
  +                  (and (intern-soft (cdr id))
  +                       (bound-and-true-p (intern (cdr id))))
                     ;; Create a new object.
                     (make-multisession
                      :package (car id)


because `bound-and-true-p' is a macro that requires as argument a symbol,
which (intern ...) is not.

ELISP> (bound-and-true-p (intern "whatever"))
*** Eval error ***  Wrong type argument: symbolp, (intern "whatever")

so I'm afraid this change was never tested.

The fix is reverting the change, doing perhaps this

diff --git a/lisp/emacs-lisp/multisession.el
b/lisp/emacs-lisp/multisession.el
index 9d6e8c0d88..78d4137317 100644
--- a/lisp/emacs-lisp/multisession.el
+++ b/lisp/emacs-lisp/multisession.el
@@ -447,8 +447,9 @@ multisession-edit-value
   (let* ((object (or
                   ;; If the multisession variable already exists, use
                   ;; it (so that we update it).
-                  (and (intern-soft (cdr id))
-                       (bound-and-true-p (intern (cdr id))))
+                  (if-let (sym (intern-soft (cdr id)))
+                      (and (boundp sym) (symbol-value sym))
+                    nil)
                   ;; Create a new object.
                   (make-multisession
                    :package (car id)

[Message part 2 (text/html, inline)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juanma Barranquero <lekktu <at> gmail.com>
Cc: larsi <at> gnus.org, 59710 <at> debbugs.gnu.org
Subject: Re: bug#59710: Wrong type argument when editing a multisession
 variable
Date: Fri, 02 Dec 2022 15:07:02 +0200

> Cc: "lars ingebrigtsen" <larsi <at> gnus.org>
> From: Juanma Barranquero <lekktu <at> gmail.com>
> Date: Wed, 30 Nov 2022 02:49:15 +0100
> 
> Debugger entered--Lisp error: (wrong-type-argument symbolp (intern (cdr id)))
>   multisession-edit-value(("test" . "test-var"))
>   funcall-interactively(multisession-edit-value ("test" . "test-var"))
>   command-execute(multisession-edit-value)
> 
> The problem comes from this change
> 
>   commit bd586121ac21e046f60f75eeb0200866c38d6f9f
>   Author: Lars Ingebrigtsen <larsi <at> gnus.org>
>   Date:   2022-01-22 11:56:13 +0100
> 
>       Make the test for existing multisession variables more sensible
> 
>       * lisp/emacs-lisp/multisession.el (multisession-edit-value):
>       Unconfuse the code.
> 
>   diff --git a/lisp/emacs-lisp/multisession.el b/lisp/emacs-lisp/multisession.el
>   index 4a293796a8..25307594c6 100644
>   --- a/lisp/emacs-lisp/multisession.el
>   +++ b/lisp/emacs-lisp/multisession.el
>   @@ -437,8 +437,8 @@ multisession-edit-value
>      (let* ((object (or
>                      ;; If the multisession variable already exists, use
>                      ;; it (so that we update it).
>   -                  (and (boundp (intern-soft (cdr id)))
>   -                       (symbol-value (intern (cdr id))))
>   +                  (and (intern-soft (cdr id))
>   +                       (bound-and-true-p (intern (cdr id))))
>                      ;; Create a new object.
>                      (make-multisession
>                       :package (car id)
> 
> because `bound-and-true-p' is a macro that requires as argument a symbol, which (intern ...) is not.
> 
> ELISP> (bound-and-true-p (intern "whatever"))
> *** Eval error ***  Wrong type argument: symbolp, (intern "whatever")
> 
> so I'm afraid this change was never tested.
> 
> The fix is reverting the change, doing perhaps this
> 
> diff --git a/lisp/emacs-lisp/multisession.el b/lisp/emacs-lisp/multisession.el
> index 9d6e8c0d88..78d4137317 100644
> --- a/lisp/emacs-lisp/multisession.el
> +++ b/lisp/emacs-lisp/multisession.el
> @@ -447,8 +447,9 @@ multisession-edit-value
>    (let* ((object (or
>                    ;; If the multisession variable already exists, use
>                    ;; it (so that we update it).
> -                  (and (intern-soft (cdr id))
> -                       (bound-and-true-p (intern (cdr id))))
> +                  (if-let (sym (intern-soft (cdr id)))
> +                      (and (boundp sym) (symbol-value sym))
> +                    nil)
>                    ;; Create a new object.
>                    (make-multisession
>                     :package (car id)

This makes sense to me, so please go ahead and install (assuming that the
multisession tests still all pass after this change).

Thanks.

Message #13 received at 59710-done <at> debbugs.gnu.org (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: larsi <at> gnus.org, 59710-done <at> debbugs.gnu.org
Subject: Re: bug#59710: Wrong type argument when editing a multisession
 variable
Date: Fri, 2 Dec 2022 14:30:52 +0100

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

On Fri, Dec 2, 2022 at 2:07 PM Eli Zaretskii <eliz <at> gnu.org> wrote:

> This makes sense to me, so please go ahead and install (assuming that the
> multisession tests still all pass after this change).

Yes, they pass. Installed in commit e5b0141b0d of 2022-12-02.

BTW, I wonder if it would make sense to make bound-and-true-p to check that
it gets a symbol:

diff --git i/lisp/bindings.el w/lisp/bindings.el
index c1ad5f7520..6ee730af58 100644
--- i/lisp/bindings.el
+++ w/lisp/bindings.el
@@ -671,4 +671,6 @@ bound-and-true-p
 Note that if `lexical-binding' is in effect, this function isn't
 meaningful if it refers to a lexically bound variable."
+  (unless (symbolp var)
+    (error "Wrong type argument: symbolp, %S" var))
   `(and (boundp (quote ,var)) ,var))

[Message part 2 (text/html, inline)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juanma Barranquero <lekktu <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Lars Ingebrigtsen <larsi <at> gnus.org> 
Cc: 59710 <at> debbugs.gnu.org
Subject: Re: bug#59710: Wrong type argument when editing a multisession
 variable
Date: Fri, 02 Dec 2022 17:06:17 +0200

> From: Juanma Barranquero <lekktu <at> gmail.com>
> Date: Fri, 2 Dec 2022 14:30:52 +0100
> Cc: larsi <at> gnus.org, 59710-done <at> debbugs.gnu.org
> 
> BTW, I wonder if it would make sense to make bound-and-true-p to check that it gets a symbol:
> 
> diff --git i/lisp/bindings.el w/lisp/bindings.el
> index c1ad5f7520..6ee730af58 100644
> --- i/lisp/bindings.el
> +++ w/lisp/bindings.el
> @@ -671,4 +671,6 @@ bound-and-true-p
>  Note that if `lexical-binding' is in effect, this function isn't
>  meaningful if it refers to a lexically bound variable."
> +  (unless (symbolp var)
> +    (error "Wrong type argument: symbolp, %S" var))
>    `(and (boundp (quote ,var)) ,var))

I have no opinion on that.  Lars, Stefan: WDYT?

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Juanma Barranquero <lekktu <at> gmail.com>, Lars Ingebrigtsen <larsi <at> gnus.org>,
 59710 <at> debbugs.gnu.org
Subject: Re: bug#59710: Wrong type argument when editing a multisession
 variable
Date: Fri, 02 Dec 2022 10:40:04 -0500

>> @@ -671,4 +671,6 @@ bound-and-true-p
>>  Note that if `lexical-binding' is in effect, this function isn't
>>  meaningful if it refers to a lexically bound variable."
>> +  (unless (symbolp var)
>> +    (error "Wrong type argument: symbolp, %S" var))
>>    `(and (boundp (quote ,var)) ,var))
>
> I have no opinion on that.  Lars, Stefan: WDYT?

I wish Someone™ implemented something like
[Fortifying macros](https://dl.acm.org/doi/10.1145/1863543.1863577)
to solve this in a more general way :-)

I have no objections against such a change.


        Stefan

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

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Lars Ingebrigtsen <larsi <at> gnus.org>,
 59710 <at> debbugs.gnu.org
Subject: Re: bug#59710: Wrong type argument when editing a multisession
 variable
Date: Sat, 3 Dec 2022 01:25:13 +0100

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

In fact, I think it would be marginally better

(unless (symbolp var)
  (signal 'wrong-type-argument (list 'symbolp var)))

Assuming ok to install... where? emacs-29 or master?

[Message part 2 (text/html, inline)]

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juanma Barranquero <lekktu <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Lars Ingebrigtsen <larsi <at> gnus.org>,
 59710 <at> debbugs.gnu.org
Subject: Re: bug#59710: Wrong type argument when editing a multisession
 variable
Date: Fri, 02 Dec 2022 19:28:21 -0500

Juanma Barranquero [2022-12-03 01:25:13] wrote:

> In fact, I think it would be marginally better
>
> (unless (symbolp var)
>   (signal 'wrong-type-argument (list 'symbolp var)))
>
> Assuming ok to install... where? emacs-29 or master?

master


        Tefan

Previous Next

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