bug#42147: 28.0.50; pure vs side-effect-free, missing optimizations?

["Mattias Engdegård" <mattiase@acm.org>]

[-- Attachment #1: Type: text/plain, Size: 272 bytes --]

26 juli 2020 kl. 00.30 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> But yes, it's a problem because `C-h o kbd RET` says explicitly that it
> doesn't change the match data :-(

Motivation enough for them to save the match data then.
What about the attached patch?


[-- Attachment #2: 0001-Preserve-match-data-in-kbd-and-package-get-version.patch --]
[-- Type: application/octet-stream, Size: 3660 bytes --]

From ba851b70be4211695937fa7fbac7ee38bbbfa4aa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Sun, 26 Jul 2020 10:51:42 +0200
Subject: [PATCH] Preserve match data in 'kbd' and 'package-get-version'

* lisp/emacs-lisp/package.el (package-get-version):
* lisp/subr.el (kbd): Preserve match data, since these functions are
declared pure (see discussion in bug#42147).
---
 lisp/emacs-lisp/package.el | 39 +++++++++++++++++++-------------------
 lisp/subr.el               |  7 +++----
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index e6f54d206d..b87f568650 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -3936,25 +3936,26 @@ package-get-version
          (or (if (boundp 'byte-compile-current-file) byte-compile-current-file)
              load-file-name
              buffer-file-name)))
-    (cond
-     ((null file) nil)
-     ;; Packages are normally installed into directories named "<pkg>-<vers>",
-     ;; so get the version number from there.
-     ((string-match "/[^/]+-\\([0-9]\\(?:[0-9.]\\|pre\\|beta\\|alpha\\|snapshot\\)+\\)/[^/]+\\'" file)
-      (match-string 1 file))
-     ;; For packages run straight from the an elpa.git clone, there's no
-     ;; "-<vers>" in the directory name, so we have to fetch the version
-     ;; the hard way.
-     (t
-      (let* ((pkgdir (file-name-directory file))
-             (pkgname (file-name-nondirectory (directory-file-name pkgdir)))
-             (mainfile (expand-file-name (concat pkgname ".el") pkgdir)))
-        (when (file-readable-p mainfile)
-          (require 'lisp-mnt)
-          (with-temp-buffer
-            (insert-file-contents mainfile)
-            (or (lm-header "package-version")
-                (lm-header "version")))))))))
+    (save-match-data            ; Since the function is declared pure.
+      (cond
+       ((null file) nil)
+       ;; Packages are normally installed into directories named "<pkg>-<vers>",
+       ;; so get the version number from there.
+       ((string-match "/[^/]+-\\([0-9]\\(?:[0-9.]\\|pre\\|beta\\|alpha\\|snapshot\\)+\\)/[^/]+\\'" file)
+        (match-string 1 file))
+       ;; For packages run straight from the an elpa.git clone, there's no
+       ;; "-<vers>" in the directory name, so we have to fetch the version
+       ;; the hard way.
+       (t
+        (let* ((pkgdir (file-name-directory file))
+               (pkgname (file-name-nondirectory (directory-file-name pkgdir)))
+               (mainfile (expand-file-name (concat pkgname ".el") pkgdir)))
+          (when (file-readable-p mainfile)
+            (require 'lisp-mnt)
+            (with-temp-buffer
+              (insert-file-contents mainfile)
+              (or (lm-header "package-version")
+                  (lm-header "version"))))))))))
 
 \f
 ;;;; Quickstart: precompute activation actions for faster start up.
diff --git a/lisp/subr.el b/lisp/subr.el
index 10c37e9413..70a6ec7ab2 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -891,10 +891,9 @@ kbd
 `edmacro-mode').
 
 For an approximate inverse of this, see `key-description'."
-  ;; Don't use a defalias, since the `pure' property is true only for
-  ;; the calling convention of `kbd'.
-  (read-kbd-macro keys))
-(put 'kbd 'pure t)
+  (declare (pure t))
+  ;; A pure function is expected to preserve the match data.
+  (save-match-data (read-kbd-macro keys)))
 
 (defun undefined ()
   "Beep to tell the user this binding is undefined."
-- 
2.21.1 (Apple Git-122.3)