bug#27016: possible bug in `defsetf'

[npostavs]

tags 27016 + patch
quit

Michael Heerdegen <michael_heerdegen@web.de> writes:

> npostavs@users.sourceforge.net writes:
>
>> I hope someone will correct me if I've gotten mixed up again, but I
>> believe the byte-compiled case already works fine:
>>
>>     ~/src/emacs$ cat bug-27016-defsetf.el
>>     (require 'cl)
>>
>>     (defvar pair nil)
>>     (setq pair (cons 3 4))
>>     (when nil
>>       (defalias 'foobar 'cons)
>>       (defsetf foobar setcar))
>>     ~/src/emacs$ emacs -Q -batch -f batch-byte-compile bug-27016-defsetf.el
>>
>>     In toplevel form:
>>     bug-27016-defsetf.el:1:1:Warning: cl package required at runtime
>>     bug-27016-defsetf.el:3:1:Warning: global/dynamic var ‘pair’ lacks a 
>> prefix
>>     ~/src/emacs$ emacs -Q -batch -l bug-27016-defsetf.elc --eval '(setf 
>> (foobar pair) 0)'
>>     Symbol’s function definition is void: \(setf\ foobar\)
>
> Yes - if you use two separate Emacs instances.  The defsetf gets
> evaluated in the Emacs that is used to compile the code
> (unconditionally).
>
> So what you state is not suspicious for a problem with a surprising side
> effect when performing macroexpansion, right?

Right.

    ~/src/emacs$ emacs -Q -batch --eval '(byte-compile-file 
"bug-27016-defsetf.el")' -l bug-27016-defsetf.elc --eval  '(progn (setf (foobar 
pair) 0) (print pair))'

    In toplevel form:
    bug-27016-defsetf.el:1:1:Warning: cl package required at runtime
    bug-27016-defsetf.el:3:1:Warning: global/dynamic var ‘pair’ lacks a prefix

    (0 . 4)

You were indeed correct to point to that FIXME comment in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27016#28

Here's a patch for it:

>From 1c6585f06ce87b62c505a575661554b6d6b961c8 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 9 Jul 2017 15:56:50 -0400
Subject: [PATCH] Don't define gv expanders in compiler session (Bug#27016)

This prevents definitions being compiled from leaking into the current
Emacs doing the compilation.
* lisp/emacs-lisp/gv.el (gv-define-expander): Push the expander
definition into `byte-compile-macro-environment' instead of evaluating
at compile time.
(gv-get): Check `byte-compile-macro-environment' for gv-expander
definitions.
---
 lisp/emacs-lisp/gv.el | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el
index c5c12a6414..b916dda731 100644
--- a/lisp/emacs-lisp/gv.el
+++ b/lisp/emacs-lisp/gv.el
@@ -91,7 +91,10 @@ (defun gv-get (place do)
    ((not (consp place)) (signal 'gv-invalid-place (list place)))
    (t
     (let* ((head (car place))
-           (gf (function-get head 'gv-expander 'autoload)))
+           (gf (or (alist-get head (alist-get :gv-expands
+                                              (bound-and-true-p
+                                               
byte-compile-macro-environment)))
+                   (function-get head 'gv-expander 'autoload))))
       (if gf (apply gf do (cdr place))
         (let ((me (macroexpand-1 place
                                  ;; (append macroexpand-all-environment
@@ -146,12 +149,15 @@ (defmacro gv-define-expander (name handler)
 HANDLER is a function which takes an argument DO followed by the same
 arguments as NAME.  DO is a function as defined in `gv-get'."
   (declare (indent 1) (debug (sexp form)))
-  ;; Use eval-and-compile so the method can be used in the same file as it
-  ;; is defined.
-  ;; FIXME: Just like byte-compile-macro-environment, we should have something
-  ;; like byte-compile-symbolprop-environment so as to handle these things
-  ;; cleanly without affecting the running Emacs.
-  `(eval-and-compile (put ',name 'gv-expander ,handler)))
+  ;; Push onto `byte-compile-macro-environment' so the method can be
+  ;; used in the same file as it is defined.
+  (when (boundp 'byte-compile-macro-environment)
+    (push (cons :gv-expanders
+                (cons (cons name handler)
+                      (cdr (assq :gv-expanders
+                                 byte-compile-macro-environment))))
+          byte-compile-macro-environment))
+  `(put ',name 'gv-expander ,handler))
 
 ;;;###autoload
 (defun gv--defun-declaration (symbol name args handler &optional fix)
-- 
2.11.1