GNU bug report logs - #76730
29.3; gv-define-setter defect

Previous Next

Package: emacs;

Reported by: Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>

Date: Tue, 4 Mar 2025 03:17:01 UTC

Severity: normal

Tags: patch

Found in version 29.3

Done: Michael Heerdegen <michael_heerdegen <at> web.de>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#76730; Package emacs. (Tue, 04 Mar 2025 03:17:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Tue, 04 Mar 2025 03:17:02 GMT) Full text and rfc822 format available.

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

From: Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.3; gv-define-setter defect
Date: Mon, 03 Mar 2025 21:16:22 -0600
Spurious byte compiler errors when called for effect:

(decf (overlay-end (make-overlay 1 2)))
-->
(let* ((v (make-overlay 1 2)))
  (progn
    (move-overlay v (overlay-start v) (- (overlay-end v) 1))
    (- (overlay-end v) 1)))

because the redundant second (- (overlay-end v) 1) gets
Warning: value returned from (- (overlay-end v) 1) is unused

A better expansion such as

(let* ((v (make-overlay 1 2))
       (s (- (overlay-end v) 1)))
  (progn
    (move-overlay v (overlay-start v) s)
    s))

looks ok and should pacify the byte compiler.

		Peace
			--Devon

P.S.  Could be worse than mere spurious warnings:
lisp/emacs-lisp/gv.el

(gv-define-setter overlay-end (store ov)
  `(progn (move-overlay ,ov (overlay-start ,ov) ,store) ,store))

the expansion correctly evaluates OV only once,
yet incorrectly evaluates STORE twice!
Too clever for its own good?

P.P.S.  I won't ask why the warning points to the top-level form
rather than the offending line.

In GNU Emacs 29.3 (build 1, x86_64-apple-darwin18.7.0, NS appkit-1671.60
 Version 10.14.6 (Build 18G9323)) of 2024-03-24 built on
 builder10-14.lan
Windowing system distributor 'Apple', version 10.3.1671
System Description:  Mac OS X 10.14.6

Configured using:
 'configure --with-ns '--enable-locallisppath=/Library/Application
 Support/Emacs/${version}/site-lisp:/Library/Application
 Support/Emacs/site-lisp' --with-modules 'CFLAGS=-DFD_SETSIZE=10000
 -DDARWIN_UNLIMITED_SELECT' --with-x-toolkit=no'

Configured features:
ACL GLIB GMP GNUTLS JPEG JSON LIBXML2 MODULES NOTIFY KQUEUE NS PDUMPER
PNG RSVG SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER ZLIB

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

Major mode: Lisp Interaction

Minor modes in effect:
  text-scale-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-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
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util 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 pp cl-macs cl cl-print
byte-opt gv bytecomp byte-compile debug backtrace find-func trace
cl-extra shortdoc text-property-search help-fns radix-tree help-mode
cl-loaddefs cl-lib format-spec face-remap rmc iso-transl tooltip cconv
eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode mwheel term/ns-win ns-win ucs-normalize mule-util
term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-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 font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
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 emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads kqueue cocoa ns multi-tty make-network-process emacs)

Memory information:
((conses 16 59178 7115)
 (symbols 48 6664 0)
 (strings 32 19075 1819)
 (string-bytes 1 548309)
 (vectors 16 12773)
 (vector-slots 8 187827 13307)
 (floats 8 133 93)
 (intervals 56 471 3)
 (buffers 984 15))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76730; Package emacs. (Wed, 05 Mar 2025 13:55:02 GMT) Full text and rfc822 format available.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>
Cc: 76730 <at> debbugs.gnu.org
Subject: Re: bug#76730: 29.3; gv-define-setter defect
Date: Wed, 05 Mar 2025 14:55:34 +0100
Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net> writes:

> (gv-define-setter overlay-end (store ov)
>   `(progn (move-overlay ,ov (overlay-start ,ov) ,store) ,store))
>
> the expansion correctly evaluates OV only once,
> yet incorrectly evaluates STORE twice!
> Too clever for its own good?

It's plain wrong.  The worst thing here is that the return value is
incorrect, it is 1 too low:

  (decf (overlay-end (make-overlay 1 2))) --> 0  ; should be 1


Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76730; Package emacs. (Wed, 05 Mar 2025 15:38:02 GMT) Full text and rfc822 format available.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>
Cc: 76730 <at> debbugs.gnu.org
Subject: Re: bug#76730: 29.3; gv-define-setter defect
Date: Wed, 05 Mar 2025 16:38:18 +0100
[Message part 1 (text/plain, inline)]
Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net> writes:

> A better expansion such as
>
> (let* ((v (make-overlay 1 2))
>        (s (- (overlay-end v) 1)))
>   (progn
>     (move-overlay v (overlay-start v) s)
>     s))
>
> looks ok and should pacify the byte compiler.

Agreed.  This should do it:

[0001-Fix-overlay-start-and-overlay-end-gv-setters.patch (text/x-diff, inline)]
From 30d3ee71753aa9f8527a28b15295ccb421e7c8c0 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen <at> web.de>
Date: Wed, 5 Mar 2025 16:09:30 +0100
Subject: [PATCH] Fix overlay-start and overlay-end gv setters

This fixes Bug#76730.

* lisp/emacs-lisp/gv.el (overlay-start, overlay-end):
Avoid computing the set value twice.
---
 lisp/emacs-lisp/gv.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el
index b2390d65817..8e69c8d0447 100644
--- a/lisp/emacs-lisp/gv.el
+++ b/lisp/emacs-lisp/gv.el
@@ -400,9 +400,11 @@ keymap-parent
 (gv-define-simple-setter match-data set-match-data 'fix)
 (gv-define-simple-setter overlay-get overlay-put)
 (gv-define-setter overlay-start (store ov)
-  `(progn (move-overlay ,ov ,store (overlay-end ,ov)) ,store))
+  (macroexp-let2 nil store store
+    `(progn (move-overlay ,ov ,store (overlay-end ,ov)) ,store)))
 (gv-define-setter overlay-end (store ov)
-  `(progn (move-overlay ,ov (overlay-start ,ov) ,store) ,store))
+  (macroexp-let2 nil store store
+    `(progn (move-overlay ,ov (overlay-start ,ov) ,store) ,store)))
 (gv-define-simple-setter process-buffer set-process-buffer)
 (gv-define-simple-setter process-filter set-process-filter)
 (gv-define-simple-setter process-sentinel set-process-sentinel)
--
2.39.5

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

> P.P.S.  I won't ask why the warning points to the top-level form
> rather than the offending line.

This works well for me (in master).  Guess it has been improved since
29.3.


Michael.

Added tag(s) patch. Request was from Stefan Kangas <stefankangas <at> gmail.com> to control <at> debbugs.gnu.org. (Wed, 05 Mar 2025 17:48:01 GMT) Full text and rfc822 format available.

Reply sent to Michael Heerdegen <michael_heerdegen <at> web.de>:
You have taken responsibility. (Tue, 11 Mar 2025 18:51:02 GMT) Full text and rfc822 format available.

Notification sent to Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>:
bug acknowledged by developer. (Tue, 11 Mar 2025 18:51:02 GMT) Full text and rfc822 format available.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>
Cc: 76730-done <at> debbugs.gnu.org
Subject: Re: bug#76730: 29.3; gv-define-setter defect, [PATCH] Fix
 overlay-start and overlay-end gv setters
Date: Tue, 11 Mar 2025 19:51:44 +0100
I <michael_heerdegen <at> web.de> wrote:

> Agreed.  This should do it: [...]

Installed to master.  I think we are done here, so I'm closing this
report.

Thanks for reporting.


Michael.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 09 Apr 2025 11:24:09 GMT) Full text and rfc822 format available.

This bug report was last modified 71 days ago.

Previous Next


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