GNU bug report logs - #78607
31.0.50; cl-nsubstitute on strings returns an vector of integers

Previous Next

To reply to this bug, email your comments to 78607 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: Madhu <enometh <at> meer.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 31.0.50; cl-nsubstitute on strings returns an vector of integers
Date: Tue, 27 May 2025 21:06:02 +0530 (IST)

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

On recent master:

 (cl-substitute ?\_ ?\/ "foo/bar")

=> [102 111 111 95 98 97 114]

But this should return: "foo_bar"

This seems to be a bug in commit 8c8ff13e "Clean up 'cl-' prefixes for
local variables", which replaced variable names without considering
the semantics. In this case the cl-seq was being used to name a new
variable to distinguish it from `seq' -- see attached patch.

[The commit that introduced the semantically significant variable is
de5a89254 "Don't mutate strings in cl-substitute"

The point of the `n' in nsubstitute is to mark the function as one
that possibly mutates its argument, (when the implementation judges
that is more efficient and the user prefers to reuse string storage
rather than copy it)

https://www.lispworks.com/documentation/HyperSpec/Body/f_sbs_s.htm

I haven't understand this commit message yet but I hope it does not
preculde a comprehension of the semantics of the "n!"-variant.] -- Madhu

[0001-lisp-emacs-lisp-cl-seq.el-fix-previous-commit.patch (text/x-patch, inline)]

From 73d75d55990059db34f0e4bfc420d7bb14e180dd Mon Sep 17 00:00:00 2001
From: Madhu <enometh <at> net.meer>
Date: Tue, 27 May 2025 21:02:09 +0530
Subject: [PATCH] lisp/emacs-lisp/cl-seq.el: fix previous commit

lisp/emacs-lisp/cl-seq.el: (nsubstitute): restore a variable names that
were incorrectly replaced in commit 8c8ff13e, so nsubstitute on a string
returns a string instead of a vector
---
 lisp/emacs-lisp/cl-seq.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/cl-seq.el b/lisp/emacs-lisp/cl-seq.el
index 33f14df0291..c267995a79d 100644
--- a/lisp/emacs-lisp/cl-seq.el
+++ b/lisp/emacs-lisp/cl-seq.el
@@ -469,7 +469,7 @@ cl-nsubstitute
   (declare (important-return-value t))
   (cl--parsing-keywords ( :test :test-not :key :if :if-not :count
                           (:start 0) :end :from-end) ()
-    (let* ((seq (if (stringp seq) (string-to-vector seq) seq))
+    (let* ((cl-seq (if (stringp seq) (string-to-vector seq) seq))
            (len (length seq)))
       (or (eq old new) (<= (or cl-count (setq cl-count len)) 0)
           (if (and (listp seq) (or (not cl-from-end) (> cl-count (/ len 2))))
@@ -495,7 +495,7 @@ cl-nsubstitute
                       (aset seq cl-start new)
                       (setq cl-count (1- cl-count))))
                 (setq cl-start (1+ cl-start))))))
-      (if (stringp seq) (concat seq) seq))))
+      (if (stringp cl-seq) (concat seq) seq))))
 
 ;;;###autoload
 (defun cl-nsubstitute-if (new pred list &rest cl-keys)
-- 
2.49.0.9.gd50a5e8939.dirty

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

From: Madhu <enometh <at> meer.net>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#78607: 31.0.50;
 cl-nsubstitute on strings returns an vector of integers
Date: Tue, 27 May 2025 21:13:33 +0530

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

The patch I supplied is of course wrong: it did the same "blind replace"
refactoring and got it wrong (skitts law in programming), the attached
patch is uglier but hopefully is semantically correct --Madhu

[0001-lisp-emacs-lisp-cl-seq.el-fix-previous-commit.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Madhu <enometh <at> meer.net>, Stefan Kangas <stefankangas <at> gmail.com>
Cc: 78607 <at> debbugs.gnu.org
Subject: Re: bug#78607: 31.0.50;
 cl-nsubstitute on strings returns an vector of integers
Date: Sat, 31 May 2025 14:38:05 +0300

> Date: Tue, 27 May 2025 21:06:02 +0530 (IST)
> From: Madhu <enometh <at> meer.net>
> 
> On recent master:
> 
>  (cl-substitute ?\_ ?\/ "foo/bar")
> 
> => [102 111 111 95 98 97 114]
> 
> But this should return: "foo_bar"
> 
> This seems to be a bug in commit 8c8ff13e "Clean up 'cl-' prefixes for
> local variables", which replaced variable names without considering
> the semantics. In this case the cl-seq was being used to name a new
> variable to distinguish it from `seq' -- see attached patch.
> 
> [The commit that introduced the semantically significant variable is
> de5a89254 "Don't mutate strings in cl-substitute"
> 
> The point of the `n' in nsubstitute is to mark the function as one
> that possibly mutates its argument, (when the implementation judges
> that is more efficient and the user prefers to reuse string storage
> rather than copy it)
> 
> https://www.lispworks.com/documentation/HyperSpec/Body/f_sbs_s.htm
> 
> I haven't understand this commit message yet but I hope it does not
> preculde a comprehension of the semantics of the "n!"-variant.] -- Madhu
> 
> >From 73d75d55990059db34f0e4bfc420d7bb14e180dd Mon Sep 17 00:00:00 2001
> From: Madhu <enometh <at> net.meer>
> Date: Tue, 27 May 2025 21:02:09 +0530
> Subject: [PATCH] lisp/emacs-lisp/cl-seq.el: fix previous commit
> 
> lisp/emacs-lisp/cl-seq.el: (nsubstitute): restore a variable names that
> were incorrectly replaced in commit 8c8ff13e, so nsubstitute on a string
> returns a string instead of a vector
> ---
>  lisp/emacs-lisp/cl-seq.el | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lisp/emacs-lisp/cl-seq.el b/lisp/emacs-lisp/cl-seq.el
> index 33f14df0291..c267995a79d 100644
> --- a/lisp/emacs-lisp/cl-seq.el
> +++ b/lisp/emacs-lisp/cl-seq.el
> @@ -469,7 +469,7 @@ cl-nsubstitute
>    (declare (important-return-value t))
>    (cl--parsing-keywords ( :test :test-not :key :if :if-not :count
>                            (:start 0) :end :from-end) ()
> -    (let* ((seq (if (stringp seq) (string-to-vector seq) seq))
> +    (let* ((cl-seq (if (stringp seq) (string-to-vector seq) seq))
>             (len (length seq)))
>        (or (eq old new) (<= (or cl-count (setq cl-count len)) 0)
>            (if (and (listp seq) (or (not cl-from-end) (> cl-count (/ len 2))))
> @@ -495,7 +495,7 @@ cl-nsubstitute
>                        (aset seq cl-start new)
>                        (setq cl-count (1- cl-count))))
>                  (setq cl-start (1+ cl-start))))))
> -      (if (stringp seq) (concat seq) seq))))
> +      (if (stringp cl-seq) (concat seq) seq))))
>  
>  ;;;###autoload
>  (defun cl-nsubstitute-if (new pred list &rest cl-keys)
> -- 

Stefan, any comments?

Previous Next

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