GNU bug report logs - #78704
[PATCH] Use `seq-do' instead of `seq-map' for side-effects

Previous Next

To reply to this bug, email your comments to 78704 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: Zach Shaftel <zach <at> shaf.tel>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Use `seq-do' instead of `seq-map' for side-effects
Date: Thu, 05 Jun 2025 23:18:26 -0400

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

Tags: patch

self explanatory i think.



In GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.49, cairo version 1.18.4) of 2025-06-04 built on bigbox
Repository revision: 680fa61b5989b84c0e19ac568be012afd8345f0c
Repository branch: master
System Description: Arch Linux

Configured using:
 'configure --with-modules --without-xwidgets --with-native-compilation
 --with-tree-sitter --without-gsettings --without-gconf --without-gpm
 --with-pgtk --without-compress-install 'CFLAGS=-mtune=native
 -march=native -O2 -g -fuse-ld=mold''

[0001-Use-seq-do-instead-of-seq-map-for-side-effects.patch (text/patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Zach Shaftel <zach <at> shaf.tel>, Nicolas Petton <nicolas <at> petton.fr>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 78704 <at> debbugs.gnu.org
Subject: Re: bug#78704: [PATCH] Use `seq-do' instead of `seq-map' for
 side-effects
Date: Sat, 07 Jun 2025 13:20:24 +0300

> Date: Thu, 05 Jun 2025 23:18:26 -0400
> From:  Zach Shaftel via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> self explanatory i think.

I'm not sure it is, but I added a few people who might have an
opinion.

> >From ba328dd06ebc503d6bc7c1ab1f6c2c45bdf6f375 Mon Sep 17 00:00:00 2001
> From: Zach Shaftel <zach <at> shaf.tel>
> Date: Thu, 5 Jun 2025 00:05:20 -0400
> Subject: [PATCH] Use `seq-do' instead of `seq-map' for side-effects
> 
> * lisp/emacs-lisp/seq.el (seq-reverse): Use `seq-do' instead of
> `seq-map' since it's being used for effect.
> * lisp/emacs-lisp/seq.el (seq-map): Declare `important-return-value' so
> the compiler will complain about it next time.
> ---
>  lisp/emacs-lisp/seq.el | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
> index a7954e7614c..af425456fe1 100644
> --- a/lisp/emacs-lisp/seq.el
> +++ b/lisp/emacs-lisp/seq.el
> @@ -195,6 +195,7 @@ seq-subseq
>  
>  (cl-defgeneric seq-map (function sequence)
>    "Return the result of applying FUNCTION to each element of SEQUENCE."
> +  (declare (important-return-value t))
>    (let (result)
>      (seq-do (lambda (elt)
>                (push (funcall function elt) result))
> @@ -295,9 +296,9 @@ seq-sort-by
>  (cl-defgeneric seq-reverse (sequence)
>    "Return a sequence with elements of SEQUENCE in reverse order."
>    (let ((result '()))
> -    (seq-map (lambda (elt)
> -               (push elt result))
> -             sequence)
> +    (seq-do (lambda (elt)
> +              (push elt result))
> +            sequence)
>      (seq-into result (type-of sequence))))
>  
>  ;; faster implementation for sequences (sequencep)
> -- 
> 2.49.0
> 

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Nicolas Petton <nicolas <at> petton.fr>, 78704 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, Zach Shaftel <zach <at> shaf.tel>
Subject: Re: bug#78704: [PATCH] Use `seq-do' instead of `seq-map' for
 side-effects
Date: Sat, 07 Jun 2025 14:29:41 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Thu, 05 Jun 2025 23:18:26 -0400
>> From:  Zach Shaftel via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>
>> self explanatory i think.
>
> I'm not sure it is, but I added a few people who might have an
> opinion.

First of all, this is a good catch!  The situation is similar for map-do
/ map-apply, including a usage in radix-tree.el which may not be
reported by the byte compiler.

But I'm not sure adding important-return-value to functions which aren't
usually side-effect-free is a good idea given the warnings it currently
produces.

The main reason is that it's not a helpful warning unless we tell the
user which function to use instead: mapc is a special case which is
handled by an explicit message, but without a "use `seq-do' instead"
in the message, fixing the warning requires looking up docstrings to
find the right alternative, which might not exist.

Some other languages have chosen a different approach and provide a way
for functions to know, at compile time or run time, whether their return
value is used in a particular call.  Maybe we should do that instead?
As an analogy, the byte compiler won't complain about (equal x 3), but
will silently replace it by (eq x 3), so it's not like we always warn
the user about code which can be optimized in a similar fashion.

However, I did notice that we warn about fewer important-return-value
functions than I thought, and opened bug#78716 to discuss whether we
should add more warnings.

Pip

Previous Next

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