From unknown Fri Jun 20 07:14:07 2025 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Mailer: MIME-tools 5.509 (Entity 5.509) Content-Type: text/plain; charset=utf-8 From: bug#78704 <78704@debbugs.gnu.org> To: bug#78704 <78704@debbugs.gnu.org> Subject: Status: [PATCH] Use `seq-do' instead of `seq-map' for side-effects Reply-To: bug#78704 <78704@debbugs.gnu.org> Date: Fri, 20 Jun 2025 14:14:07 +0000 retitle 78704 [PATCH] Use `seq-do' instead of `seq-map' for side-effects reassign 78704 emacs submitter 78704 Zach Shaftel severity 78704 normal tag 78704 patch thanks From debbugs-submit-bounces@debbugs.gnu.org Thu Jun 05 23:18:49 2025 Received: (at submit) by debbugs.gnu.org; 6 Jun 2025 03:18:49 +0000 Received: from localhost ([127.0.0.1]:40325 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uNNbV-00088Q-25 for submit@debbugs.gnu.org; Thu, 05 Jun 2025 23:18:49 -0400 Received: from lists.gnu.org ([2001:470:142::17]:56584) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uNNbS-000886-HZ for submit@debbugs.gnu.org; Thu, 05 Jun 2025 23:18:47 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1uNNbN-0006Bw-2x for bug-gnu-emacs@gnu.org; Thu, 05 Jun 2025 23:18:41 -0400 Received: from smtp.forwardemail.net ([149.28.215.223]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1uNNbH-0005Tj-D5 for bug-gnu-emacs@gnu.org; Thu, 05 Jun 2025 23:18:40 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=shaf.tel; h=Content-Type: MIME-Version: Message-ID: Date: Subject: To: From; q=dns/txt; s=fe-acc5b42812; t=1749179911; bh=WNTZ7cFDwRT/3UGsZ9+J4zcJuMHguOFRlVTI+OwFzcQ=; b=kCMfF371thwkJkOJYlPKBhUv62f5Zry047KrmEn8bTNc5pZ8jfpacW2sqQYCv0kwPulNaqHUg Mrkv9JG8SD5UCXUiLpcu6F8u4OY29cMLVyYUnGNVlg/NL2fTMK3Mk9rzKITAmx/cFIJdo0FAUWx nf4xuoPyVXFVEbnpqjOEJPU= X-Forward-Email-ID: 68425e05d910800012efbc4b X-Forward-Email-Sender: rfc822; zach@shaf.tel, smtp.forwardemail.net, 149.28.215.223 X-Forward-Email-Version: 1.0.3 X-Forward-Email-Website: https://forwardemail.net X-Complaints-To: abuse@forwardemail.net X-Report-Abuse: abuse@forwardemail.net X-Report-Abuse-To: abuse@forwardemail.net From: Zach Shaftel To: bug-gnu-emacs@gnu.org Subject: [PATCH] Use `seq-do' instead of `seq-map' for side-effects User-Agent: mu4e 1.12.10; emacs 31.0.50 X-Debbugs-Cc: Date: Thu, 05 Jun 2025 23:18:26 -0400 Message-ID: <87wm9p7e2l.fsf@shaf.tel> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Received-SPF: pass client-ip=149.28.215.223; envelope-from=SRS0=ea70=YV=shaf.tel=zach@fe-bounces.shaf.tel; helo=smtp.forwardemail.net X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, SPF_HELO_PASS=-0.001, T_SPF_TEMPERROR=0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: 0.9 (/) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.1 (/) --=-=-= Content-Type: text/plain 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'' --=-=-= Content-Type: text/patch Content-Disposition: attachment; filename=0001-Use-seq-do-instead-of-seq-map-for-side-effects.patch >From ba328dd06ebc503d6bc7c1ab1f6c2c45bdf6f375 Mon Sep 17 00:00:00 2001 From: Zach Shaftel 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 --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Sat Jun 07 06:20:38 2025 Received: (at 78704) by debbugs.gnu.org; 7 Jun 2025 10:20:38 +0000 Received: from localhost ([127.0.0.1]:47038 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uNqfG-0005MS-4I for submit@debbugs.gnu.org; Sat, 07 Jun 2025 06:20:38 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:41624) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uNqfD-0005MB-Cc for 78704@debbugs.gnu.org; Sat, 07 Jun 2025 06:20:36 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1uNqf6-0006ot-H7; Sat, 07 Jun 2025 06:20:28 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=FteoAU69fu+lvSeEm5gkSZYDUa0+k1Rv+Vp9KtIljMg=; b=hd3N2M6Wiooc eiQA/WFIBjo4Af5IzUbJu7BVFnanuV3xzFkgb1Ab8ZItcAR4elOFobk/XPLIqAurMUirYip1lhtai wf+UoBVTvC20Mk7yZWjYiS+vIlehY4y6lRyudL6AvZ71Wg17CcxeYG9rB6vW2yAtQxOM0nG6kXg5U r27koMhYthT8WwVHky2OZXWjrk4SMOEpHDf30eb3nTcrrD25Ai5VeuctjIuVxF0AP3Urx/9oX8si6 Dz7DZ4r5FcnvjkTmCDglUCzYbiR+yC/6Xm5zRWwtcwW3lHdwdR5K5HDYP0O5nD95t+hLwNdbL/Tmi 1IqFAJ8ZGaZy7D9pMEYsvg==; Date: Sat, 07 Jun 2025 13:20:24 +0300 Message-Id: <861prvj1jr.fsf@gnu.org> From: Eli Zaretskii To: Zach Shaftel , Nicolas Petton , Stefan Monnier In-Reply-To: <87wm9p7e2l.fsf@shaf.tel> (bug-gnu-emacs@gnu.org) Subject: Re: bug#78704: [PATCH] Use `seq-do' instead of `seq-map' for side-effects References: <87wm9p7e2l.fsf@shaf.tel> X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 78704 Cc: 78704@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > 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" > > 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 > 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 > From debbugs-submit-bounces@debbugs.gnu.org Sat Jun 07 10:29:56 2025 Received: (at 78704) by debbugs.gnu.org; 7 Jun 2025 14:29:56 +0000 Received: from localhost ([127.0.0.1]:49209 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uNuYW-0007L5-1E for submit@debbugs.gnu.org; Sat, 07 Jun 2025 10:29:56 -0400 Received: from mail-4322.protonmail.ch ([185.70.43.22]:45215) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uNuYT-0007Ka-CS for 78704@debbugs.gnu.org; Sat, 07 Jun 2025 10:29:54 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1749306586; x=1749565786; bh=bKVicLTsa+VYgqcsOsNBO6OY0r2SYLTm1Eukrwg5wEA=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=M1UHKFlpWd0v6tqk5xIAWD01fCVNVNVaeXfhtBYPvt9jF3Vq4LIPg9MyNeU+t4gxK pX8nInBTLTrp2Cohqi3ihvKjkYnTu6xPAfX4SYjjcCFfNXleebEozeQAhRmiCq5S+o tjwAo1+mh0pDb+PUyJNb4ETi3lzTzcsW7nDn4l3vsZ+eZXPHiSB69ZJgRh/qZn0Jg8 1bn5gYivrdFr8xWnGdtGW+pdUAM+L5xosI4q4t0u9f/c/fCBGJzPt7V2g+lyN5Rbso DxQMmPFH1wWHDCWpcG1sOnip+p9jH66/pXBAdG5pc2Ae7a3+uvKFZfgzxbS7x7ciCe HkNgdOBcoyzWg== Date: Sat, 07 Jun 2025 14:29:41 +0000 To: Eli Zaretskii From: Pip Cet Subject: Re: bug#78704: [PATCH] Use `seq-do' instead of `seq-map' for side-effects Message-ID: <87jz5n62wg.fsf@protonmail.com> In-Reply-To: <861prvj1jr.fsf@gnu.org> References: <87wm9p7e2l.fsf@shaf.tel> <861prvj1jr.fsf@gnu.org> Feedback-ID: 112775352:user:proton X-Pm-Message-ID: 18846cade5c15c8d8e4329317c7cf28453190820 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 78704 Cc: Nicolas Petton , 78704@debbugs.gnu.org, Stefan Monnier , Zach Shaftel X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) "Eli Zaretskii" 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" >> >> 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 From debbugs-submit-bounces@debbugs.gnu.org Sat Jun 07 15:06:28 2025 Received: (at 78704) by debbugs.gnu.org; 7 Jun 2025 19:06:28 +0000 Received: from localhost ([127.0.0.1]:49615 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uNys8-0004Dk-0h for submit@debbugs.gnu.org; Sat, 07 Jun 2025 15:06:28 -0400 Received: from smtp.forwardemail.net ([121.127.44.73]:16657) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1uNys5-0004DP-EU for 78704@debbugs.gnu.org; Sat, 07 Jun 2025 15:06:26 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=shaf.tel; h=Content-Type: MIME-Version: Message-ID: Date: References: In-Reply-To: Subject: Cc: To: From; q=dns/txt; s=fe-acc5b42812; t=1749323179; bh=JZQSFJ/4U3FTVw9MNRtRgGoucaQPxqktvY1HM6vz+rI=; b=cj4ogn9gqiGKMSHuSbJH2gAWIBr+nPkskkI3ExwLpksO+pteuETN9CASjIKKYL+FLsikKSSxO ngC62K69i5m65ZeLC3WlCVj6IH0cvxPPIQuqgtRwovUFJFFeiJ5jz2hlhPyCDfHw/K6k4pLKsZU QFPUUG8IztXsL1FHSsZR0xA= X-Forward-Email-ID: 68448daad3af295f63c0c41b X-Forward-Email-Sender: rfc822; zach@shaf.tel, smtp.forwardemail.net, 121.127.44.73 X-Forward-Email-Version: 1.0.3 X-Forward-Email-Website: https://forwardemail.net X-Complaints-To: abuse@forwardemail.net X-Report-Abuse: abuse@forwardemail.net X-Report-Abuse-To: abuse@forwardemail.net From: Zach Shaftel To: Pip Cet Subject: Re: bug#78704: [PATCH] Use `seq-do' instead of `seq-map' for side-effects In-Reply-To: <87jz5n62wg.fsf@protonmail.com> References: <87wm9p7e2l.fsf@shaf.tel> <861prvj1jr.fsf@gnu.org> <87jz5n62wg.fsf@protonmail.com> User-Agent: mu4e 1.12.10; emacs 31.0.50 Date: Sat, 07 Jun 2025 15:06:15 -0400 Message-ID: <87ecvvjrrs.fsf@shaf.tel> MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 78704 Cc: Eli Zaretskii , 78704@debbugs.gnu.org, Nicolas Petton , Stefan Monnier X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Pip Cet writes: > 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. my understanding was that important-return-value is meant to instruct the compiler to warn about discarded values, without also allowing it to optimize away those calls the way the side-effect-free property does (for example, `mapcan' is declared important-return-value). so side-effect-free implies important-return-value. is that not the intended meaning? > 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. very true. maybe important-return-value could accept a string argument to specify a helpful message? this could also be useful for functions like `plist-put', which are side-effect-full but whose return value should still be captured; we could notify the user that (plist-put plist :prop val) should be wrapped with `setq'. > 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. interesting, i wasn't aware of that approach. where could i see some examples? > 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. that seems appropriate to me. (equal x 3) is still a valid usage of the `equal' function, and will do what the user intended. on the other hand, we do complain about (eq x '(t)) in bytecomp--warn-dodgy-eq-arg, because that almost certainly will not do what the user intended. > 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 From debbugs-submit-bounces@debbugs.gnu.org Mon Jun 09 06:55:15 2025 Received: (at 78704) by debbugs.gnu.org; 9 Jun 2025 10:55:15 +0000 Received: from localhost ([127.0.0.1]:54059 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uOa9r-0001Be-6k for submit@debbugs.gnu.org; Mon, 09 Jun 2025 06:55:15 -0400 Received: from mail-10630.protonmail.ch ([79.135.106.30]:60095) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uOa9o-00019F-Gg for 78704@debbugs.gnu.org; Mon, 09 Jun 2025 06:55:13 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1749466505; x=1749725705; bh=hkUhFeXNnBZKo59tvxbwZyfaEcr0hxp6YC+D0mezwBc=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=QZP/e9ONH0nNpNqPyeJo+xSCFNBDQ9gtM1oviaABBsy1Os87McaAHWVwTN1ER343t oAhCg6fR7xqirB4Z29S4YIy8dIAv6Pp2dXvDDF25maddXQVmlaWKSNwbg68BK6DdA/ 5KLKgAxgTIBrm94PMT5qopZpmATr+kELTDGkvJrZ63hEnHgvC34n8J/SAF28RVTN8s ky5QC4X+xE6NWKUAqnuVPKZ/NuV8zmq7Bi+2JW6c9YoMH0aaxtqk29maaAzW8A1BCl LShfbBoYDUgHoa9cQ5Npus9GAg5sBsloFy02bRcbSGu8yurMyX7IZk/8rCUiZzIZu6 /6jYvldOWt6cg== Date: Mon, 09 Jun 2025 10:55:00 +0000 To: Zach Shaftel From: Pip Cet Subject: Re: bug#78704: [PATCH] Use `seq-do' instead of `seq-map' for side-effects Message-ID: <87cybd422p.fsf@protonmail.com> In-Reply-To: <87ecvvjrrs.fsf@shaf.tel> References: <87wm9p7e2l.fsf@shaf.tel> <861prvj1jr.fsf@gnu.org> <87jz5n62wg.fsf@protonmail.com> <87ecvvjrrs.fsf@shaf.tel> Feedback-ID: 112775352:user:proton X-Pm-Message-ID: b80a567130bb0ba713b63036d95b283fd75e64f4 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -1.0 (-) X-Debbugs-Envelope-To: 78704 Cc: Eli Zaretskii , 78704@debbugs.gnu.org, Nicolas Petton , Stefan Monnier X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.0 (--) "Zach Shaftel" writes: > Pip Cet writes: > >> 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. > > my understanding was that important-return-value is meant to instruct > the compiler to warn about discarded values, without also allowing it to > optimize away those calls the way the side-effect-free property does Usually, the byte compiler only optimizes away calls to functions whose side-effect-free property is 'error-free, not those for which it is t. (Bug#63288 was about unexpected failure to do so after evaluating eieio-core.el, and the same code is causing trouble in bug#78685, so I'm a bit skeptical that such conditional optimizations are worth it.) > (for example, `mapcan' is declared important-return-value). so > side-effect-free implies important-return-value. is that not the > intended meaning? I believe it was, but it seems we never complained about discarding, for example, (not (beep)), even though the "not" is superfluous. See bug#78716 for what happens when we enable such warnings (some false positives, 5 places in the code base that look like potential bugs; and there are places like x-dnd-handle-unsupported-drop which I cannot make sense of at all). >> 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. > > very true. maybe important-return-value could accept a string argument > to specify a helpful message? this could also be useful for functions I think that'd be a good idea. > like `plist-put', which are side-effect-full but whose return value > should still be captured; we could notify the user that > (plist-put plist :prop val) should be wrapped with `setq'. Note that while it's traditional (and documented as a requirement) to wrap plist-put in setq, Emacs always was very forgiving and modified any non-empty plist passed to plist-put in a way that ensured the original cons cell would be returned. At least on the C side of things, we rely on this behavior. plist-put (along with nconc, delq, and delete) is commented out in the list of important-return-value functions in bytecomp.el, FWIW. Uncommenting them produces lots of warnings. >> 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. > > interesting, i wasn't aware of that approach. where could i see some > examples? Perl 5's wantarray, and the way GCC (incorrectly) rewrites printf ("%s\n", x) to puts (x) in void context. I'm surprised I haven't been able to find more, to be honest. >> 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. > > that seems appropriate to me. (equal x 3) is still a valid usage of the > `equal' function, and will do what the user intended. The same is true of (mapcar #'message strings) and discarding the result. It's perfectly valid and does what the user wants, it's just inefficient. However, that's an implementation detail. > on the other hand, we do complain about (eq x '(t)) in > bytecomp--warn-dodgy-eq-arg, because that almost certainly will not do > what the user intended. I think that's a different class of warning altogether, to be honest. Pip From debbugs-submit-bounces@debbugs.gnu.org Mon Jun 09 10:25:52 2025 Received: (at 78704) by debbugs.gnu.org; 9 Jun 2025 14:25:52 +0000 Received: from localhost ([127.0.0.1]:56162 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uOdRg-0005Uq-AM for submit@debbugs.gnu.org; Mon, 09 Jun 2025 10:25:52 -0400 Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:28995) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uOdRd-0005UF-JC for 78704@debbugs.gnu.org; Mon, 09 Jun 2025 10:25:50 -0400 Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 965E61002EC; Mon, 9 Jun 2025 10:25:43 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1749479142; bh=3klD+QyFYCYjARMcRci9+Y7+va6rIyWYCjeuYTerkGQ=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=DdWa18AoYqjLo5OXiTi5010+anjoANp//NU2iZyCKvqKx35O1NwyMSY68wz8Y8iTW Nb2ZA2a2Tf6yq9d0RQXOuXJQ6oGDcEf7N+ezVptKkI5KwPYx8K9lSWikDdKfi9SNou I263T6mPtilswbrb73UMbGYLeLQJfSBm210S0KJ7U4+2yBDDXwBrnwiH5n3zZzaYq9 eOT2Af62NaN/pJW/iheg9B/QeH3e990JZUnpMEm4WaCOtuMLrUrIVlhP7rT9CXT2Fv hcTHRSUzV8xz02AvL4Hml+BL243WpDBhryHWPyfyvX2tSbo0XaUugunARuU06gGaBv yAZk0+ijmRj1Q== Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id B091E100034; Mon, 9 Jun 2025 10:25:42 -0400 (EDT) Received: from asado (unknown [130.159.222.66]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 09E4F1203D7; Mon, 9 Jun 2025 10:25:40 -0400 (EDT) From: Stefan Monnier To: Pip Cet Subject: Re: bug#78704: [PATCH] Use `seq-do' instead of `seq-map' for side-effects In-Reply-To: <87cybd422p.fsf@protonmail.com> Message-ID: References: <87wm9p7e2l.fsf@shaf.tel> <861prvj1jr.fsf@gnu.org> <87jz5n62wg.fsf@protonmail.com> <87ecvvjrrs.fsf@shaf.tel> <87cybd422p.fsf@protonmail.com> Date: Mon, 09 Jun 2025 10:25:38 -0400 User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-SPAM-INFO: Spam detection results: 0 ALL_TRUSTED -1 Passed through trusted hosts only via SMTP BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain X-SPAM-LEVEL: X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 78704 Cc: Eli Zaretskii , 78704@debbugs.gnu.org, Nicolas Petton , Zach Shaftel X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > I believe it was, but it seems we never complained about discarding, > for example, (not (beep)), even though the "not" is superfluous. FWIW, we don't care about warning for all such cases. Instead we want to care about those cases that often enough reflect errors. Warnings are always delicate in practice because "pointless" code can show up very naturally from perfectly good code, just as the result of rewrites like inlining and macro-expansion. So it's not always desirable to be "more thorough" especially when the warning is about code that is not optimized enough (as opposed to dubious/invalid code) because you can end up with too many false positives. Stefan From debbugs-submit-bounces@debbugs.gnu.org Tue Jun 10 03:23:04 2025 Received: (at 78704) by debbugs.gnu.org; 10 Jun 2025 07:23:04 +0000 Received: from localhost ([127.0.0.1]:60269 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uOtK2-0005aN-Qr for submit@debbugs.gnu.org; Tue, 10 Jun 2025 03:23:04 -0400 Received: from mail-4316.protonmail.ch ([185.70.43.16]:54229) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uOtK0-0005Yv-7Y for 78704@debbugs.gnu.org; Tue, 10 Jun 2025 03:23:01 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1749540173; x=1749799373; bh=aqgJysubRVi0NOGNm9j3J50HRZGEw6sp+DlEPq+DevM=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=jf///77N/cqIZq0uhbcux/Q/9xLChUEQxTyKx5pzhzEdMhUdvoJFqRxkpEVG2KcLi JcX88P54n9lJ2eqqtCA96LLT6GJ+fnfQwgv3ZaSVXInp8eRkAUlfIomSsp3w2BwuQw fsU/98y88WXWem9fVx8m++RUxC7n9S31tSqEAf7aJ9bUFAuFyanxbXyzDk3DMKJ0XI a6zmimR3FFtkoWxhW2C+Jlhj9wCF55Ywo604/kqNdTplgz+ZwZwZEtIXmTbKVTv48b 9yanjlJ4d4bh4eMoQ4QUwFWup3MLEv6lwuQM8GWOTIHD/5dBYEcEdriIPv+DWtjDTS uZqSqzaTe0Uag== Date: Tue, 10 Jun 2025 07:22:49 +0000 To: Stefan Monnier From: Pip Cet Subject: Re: bug#78704: [PATCH] Use `seq-do' instead of `seq-map' for side-effects Message-ID: <87o6uw2h89.fsf@protonmail.com> In-Reply-To: References: <87wm9p7e2l.fsf@shaf.tel> <861prvj1jr.fsf@gnu.org> <87jz5n62wg.fsf@protonmail.com> <87ecvvjrrs.fsf@shaf.tel> <87cybd422p.fsf@protonmail.com> Feedback-ID: 112775352:user:proton X-Pm-Message-ID: 6f9d5b238025342ebf50354f51e49acbe92dc4ae MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 78704 Cc: Eli Zaretskii , 78704@debbugs.gnu.org, Nicolas Petton , Zach Shaftel X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) "Stefan Monnier" writes: >> I believe it was, but it seems we never complained about discarding, >> for example, (not (beep)), even though the "not" is superfluous. > > FWIW, we don't care about warning for all such cases. Instead we want > to care about those cases that often enough reflect errors. I don't think it's merely about the frequency, it's also about how hard it is to fix the bug, and how likely it is to do damage. Replacing mapconcat by mapc when the byte compiler tells you to is something that can be done without further thought (and maybe should be done automatically), but a constant or symbol being evaluated for effect sometimes represents a puzzling bug, in my experiments. While I realize that the Emacs Elisp code base might not be ideal (because most bugs that may have cropped up during development have presumably been fixex), it's what I've looked at so far; there are about 50 places I've changed locally to avoid the additional warnings, most of them harmless but a little confusing because of the unnecessary evaluation. Perhaps this one illustrates how tricky the bugs can be: diff --git a/lisp/international/ccl.el b/lisp/international/ccl.el index 5f766d8bef9..37538984df4 100644 --- a/lisp/international/ccl.el +++ b/lisp/international/ccl.el @@ -649,8 +649,8 @@ ccl-compile-loop =09 ;; this loop. =09 (while ccl-breaks =09 (ccl-embed-current-address (car ccl-breaks)) -=09 (setq ccl-breaks (cdr ccl-breaks)))) -=09 nil)))) +=09 (setq ccl-breaks (cdr ccl-breaks))))) + nil))) =20 (defun ccl-compile-break (cmd) "Compile BREAK statement." The fix seems obvious (though the modified code is still a little redundant), but the CCL compiler is well-tested, and there might be unintended consequences of fixing ccl-compile-loop to return t sometimes, even if that was the original intent. Should we live with this bug instead? Or this one: diff --git a/lisp/files.el b/lisp/files.el index 56d998410d3..a913a6c222b 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -7600,7 +7600,7 @@ recover-session-finish =09=09=09 (condition-case nil =09=09=09=09 (save-excursion (recover-file file)) =09=09=09=09 (error -=09=09=09=09 "Failed to recover `%s'" file))) +=09=09=09=09 (message "Failed to recover `%s'" file)))) =09=09=09 files =09=09=09 '("file" "files" "recover")) =09 (message "No files can be recovered from this session now"))) Is it right to use "message" here, which may be overwritten immediately by the next iteration of map-y-or-n-p? Or we could do without a message in this case, as we have done for almost 30 years. Here are the ones in lisp/emacs-lisp, to demonstrate that most of them are merely cosmetic: diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el index a076012cd30..87a5486404a 100644 --- a/lisp/emacs-lisp/cl-macs.el +++ b/lisp/emacs-lisp/cl-macs.el @@ -2260,7 +2260,7 @@ cl--self-tco-on-form ;; Apply self-tco to the function returned by FORM, assuming that ;; it will be bound to VAR. (pcase form - (`(function (lambda ,fargs . ,ebody)) form + (`(function (lambda ,fargs . ,ebody)) (pcase-let* ((`(,decls . ,body) (macroexp-parse-body ebody)) (`(,ofargs . ,obody) (cl--self-tco var fargs body))) `(function (lambda ,ofargs ,@decls . ,obody)))) diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el index c7e38015c7e..11bae960887 100644 --- a/lisp/emacs-lisp/comp.el +++ b/lisp/emacs-lisp/comp.el @@ -325,8 +325,9 @@ comp-vec--verify-idx (defsubst comp-vec-aref (vec idx) "Return the element of VEC whose index is IDX." (declare (gv-setter (lambda (val) - `(comp-vec--verify-idx ,vec ,idx) - `(puthash ,idx ,val (comp-vec-data ,vec))))) + `(progn + (comp-vec--verify-idx ,vec ,idx) + (puthash ,idx ,val (comp-vec-data ,vec)))))) (comp-vec--verify-idx vec idx) (gethash idx (comp-vec-data vec))) =20 diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el index 284e3acd959..e047d4ce1de 100644 --- a/lisp/emacs-lisp/edebug.el +++ b/lisp/emacs-lisp/edebug.el @@ -917,8 +917,8 @@ edebug-read-list =09(if (eq 'dot (edebug-next-token-class)) =09 (let (dotted-form) =09 (forward-char 1)=09=09; skip \. -=09 (setq dotted-form (edebug-read-storing-offsets stream)) -=09=09 elements (nconc elements dotted-form) +=09 (setq dotted-form (edebug-read-storing-offsets stream) +=09=09 elements (nconc elements dotted-form)) =09 (if (not (eq (edebug-next-token-class) 'rparen)) =09=09 (edebug-syntax-error "Expected `)'")) =09 (setq edebug-read-dotted-list (listp dotted-form)) > Warnings are always delicate in practice because "pointless" code can > show up very naturally from perfectly good code, just as the result of > rewrites like inlining and macro-expansion. So it's not always > desirable to be "more thorough" especially when the warning is about > code that is not optimized enough (as opposed to dubious/invalid code) > because you can end up with too many false positives. I agree completely. Not proposing to add a new warning to the byte compiler, at this point. I personally often ignore the byte compiler warnings because code I'm writing has unused arguments and variables, and those warnings drown out more serious ones. Pip From debbugs-submit-bounces@debbugs.gnu.org Tue Jun 10 08:11:36 2025 Received: (at 78704) by debbugs.gnu.org; 10 Jun 2025 12:11:36 +0000 Received: from localhost ([127.0.0.1]:36567 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uOxpH-00030A-UO for submit@debbugs.gnu.org; Tue, 10 Jun 2025 08:11:36 -0400 Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:63198) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uOxpF-0002zs-3b for 78704@debbugs.gnu.org; Tue, 10 Jun 2025 08:11:33 -0400 Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 4B6B210006B; Tue, 10 Jun 2025 08:11:27 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1749557486; bh=ahOdgS8zMhHyEmRzHbm93vNjT0pw7faNBzFdpBpzURo=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=Hg1VCIITQBG2L9/3M7az/Yz1KoeOkacFL1UyjWF7AexCatvkAxlO6m8vht1e4tQBN +0hGl+dc5+Dvprbfd6cl5xNrEn89Xh/5cWEDtx7Tpz8jZ4QsHd5V+c5gEIj/vKHkkM 2gVfbFdoDP0KXER5Qy3KOVYn9PLBJ8oxy8k/vbhiPGNZqes0Rfh/Dyz4FPdhjqxHRP GkOxkQde/YTH8JsMTL2R7InwVUmFhZQodPqmPmGwa0vt54J7wkpB4s7qLJa5i1RyMG j2y9urjVz5fnRejfQ+qPRZK5i20HnbLqlxHRlOj2FeGFMpUwqj/DvblN3d0hNT8IzW U2VdojRPTiYqA== Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 519A3100029; Tue, 10 Jun 2025 08:11:26 -0400 (EDT) Received: from asado (unknown [130.159.222.66]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id A27E41205D4; Tue, 10 Jun 2025 08:11:24 -0400 (EDT) From: Stefan Monnier To: Pip Cet Subject: Re: bug#78704: [PATCH] Use `seq-do' instead of `seq-map' for side-effects In-Reply-To: <87o6uw2h89.fsf@protonmail.com> Message-ID: References: <87wm9p7e2l.fsf@shaf.tel> <861prvj1jr.fsf@gnu.org> <87jz5n62wg.fsf@protonmail.com> <87ecvvjrrs.fsf@shaf.tel> <87cybd422p.fsf@protonmail.com> <87o6uw2h89.fsf@protonmail.com> Date: Tue, 10 Jun 2025 08:11:22 -0400 User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-SPAM-INFO: Spam detection results: 0 ALL_TRUSTED -1 Passed through trusted hosts only via SMTP BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain X-SPAM-LEVEL: X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 78704 Cc: Eli Zaretskii , 78704@debbugs.gnu.org, Nicolas Petton , Zach Shaftel X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > While I realize that the Emacs Elisp code base might not be ideal > (because most bugs that may have cropped up during development have > presumably been fixex), it's what I've looked at so far; there are about > 50 places I've changed locally to avoid the additional warnings, most of > them harmless but a little confusing because of the unnecessary > evaluation. Thanks. That kind of data is a good guide, IME. > @@ -7600,7 +7600,7 @@ recover-session-finish > (condition-case nil > (save-excursion (recover-file file)) > (error > - "Failed to recover `%s'" file))) > + (message "Failed to recover `%s'" file)))) > files > '("file" "files" "recover")) > (message "No files can be recovered from this session now"))) > > Is it right to use "message" here, which may be overwritten immediately > by the next iteration of map-y-or-n-p? Or we could do without a message > in this case, as we have done for almost 30 years. Better a message that gets hidden by a subsequent one than no message at all, I think. > I personally often ignore the byte compiler warnings because code I'm > writing has unused arguments and variables, and those warnings drown out > more serious ones. Hmm... I was hoping that an underscore is a lightweight enough way to indicate when a variable is unused to avoid being drowned in "unused args" warnings. Stefan