changeset 5658:289cf21be887

Don't augment ENVIRONMENT when that's not indicated, #'macroexpand. This reflects better understanding on my part of the &environment macro keyword, and I've expanded the Lisp manual and docstrings to reflect that. lisp/ChangeLog addition: 2012-05-06 Aidan Kehoe <kehoea@parhasard.net> * cl-macs.el (block): Comment on why we can't use &environment here. * cl-macs.el (defmacro*): Document &environment in more detail. * cl-macs.el (macrolet): Use &environment, instead of referencing byte-compile-macro-environment directly. * cl-macs.el (symbol-macrolet): Ditto. * cl-macs.el (lexical-let): Ditto. * cl-macs.el (labels): Ditto. man/ChangeLog addition: 2012-05-06 Aidan Kehoe <kehoea@parhasard.net> * lispref/macros.texi (Expansion): Cross-reference to documentation of #'cl-prettyexpand, #'defmacro* when talking about #'macroexpand. tests/ChangeLog addition: 2012-05-06 Aidan Kehoe <kehoea@parhasard.net> * automated/lisp-tests.el: Use &environment appropriately in #'macrolet, instead of relying on #'macroexpand to guess what we mean.
author Aidan Kehoe <kehoea@parhasard.net>
date Sun, 06 May 2012 15:29:59 +0100
parents 2a870a7b86bd
children e63bb7b22c8f
files lisp/ChangeLog lisp/cl-macs.el man/ChangeLog man/lispref/macros.texi src/ChangeLog src/eval.c tests/ChangeLog tests/automated/lisp-tests.el
diffstat 8 files changed, 92 insertions(+), 37 deletions(-) [+]
line wrap: on
line diff
--- a/lisp/ChangeLog	Sun May 06 05:22:19 2012 +0100
+++ b/lisp/ChangeLog	Sun May 06 15:29:59 2012 +0100
@@ -1,3 +1,14 @@
+2012-05-06  Aidan Kehoe  <kehoea@parhasard.net>
+
+	* cl-macs.el (block): Comment on why we can't use &environment
+	here.
+	* cl-macs.el (defmacro*): Document &environment in more detail.
+	* cl-macs.el (macrolet): Use &environment, instead of referencing
+	byte-compile-macro-environment directly.
+	* cl-macs.el (symbol-macrolet): Ditto.
+	* cl-macs.el (lexical-let): Ditto.
+	* cl-macs.el (labels): Ditto.
+
 2012-05-06  Aidan Kehoe  <kehoea@parhasard.net>
 
 	* byte-optimize.el:
--- a/lisp/cl-macs.el	Sun May 06 05:22:19 2012 +0100
+++ b/lisp/cl-macs.el	Sun May 06 15:29:59 2012 +0100
@@ -229,8 +229,12 @@
    macro expansion time, reflects all the arguments supplied to the macro,
    as if it had been declared with a single &rest argument.
 
-   &environment specifies local semantics for various macros for use within
-   the expansion of BODY.  See the ENVIRONMENT argument to `macroexpand'.
+   &environment allows access to the macro environment at the time of
+   expansion; it is most relevant when it's necessary to force macro expansion
+   of the body of a form at the time of macro expansion of its top level.
+   &environment is followed by variable name, and this variable will be bound
+   to the value of the macro environment within BODY. See the ENVIRONMENT
+   argument to `macroexpand'.
 
 -- The macro arg list syntax allows for \"destructuring\" -- see also
    `destructuring-bind', which destructures exactly like `defmacro*', and
@@ -715,6 +719,8 @@
     ;; as such it can eliminate it if that's appropriate:
     (put (cdar cl-active-block-names) 'cl-block-name name)
     `(catch ',(cdar cl-active-block-names)
+      ;; Can't use &environment, since #'block is used in
+      ;; #'cl-transform-lambda.
       ,(cl-macroexpand-all body byte-compile-macro-environment))))
 
 ;;;###autoload
@@ -1693,7 +1699,7 @@
 	      '(cl-progv-after))))
 
 ;;;###autoload
-(defmacro* macrolet ((&rest macros) &body form)
+(defmacro* macrolet ((&rest macros) &body form &environment env)
   "Make temporary macro definitions.
 This is like `flet', but for macros instead of functions."
   (cl-macroexpand-all (cons 'progn form)
@@ -1704,10 +1710,10 @@
                          collect
                          (list* name 'lambda (cdr (cl-transform-lambda details
                                                                        name))))
-                       byte-compile-macro-environment)))
+                       env)))
 
 ;;;###autoload
-(defmacro* symbol-macrolet ((&rest symbol-macros) &body form)
+(defmacro* symbol-macrolet ((&rest symbol-macros) &body form &environment env)
   "Make temporary symbol macro definitions.
 Elements in SYMBOL-MACROS look like (NAME EXPANSION).
 Within the body FORMs, a reference to NAME is replaced with its EXPANSION,
@@ -1717,11 +1723,11 @@
 			       for (name expansion) in symbol-macros
 			       do (check-type name symbol)
 			       collect (list (eq-hash name) expansion))
-			     byte-compile-macro-environment)))
+			     env)))
 
 (defvar cl-closure-vars nil)
 ;;;###autoload
-(defmacro lexical-let (bindings &rest body)
+(defmacro* lexical-let (bindings &rest body &environment env)
   "Like `let', but lexically scoped.
 The main visible difference is that lambdas inside BODY will create
 lexical closures as in Common Lisp."
@@ -1743,7 +1749,7 @@
 				    t))
 			  vars)
 		  (list '(defun . cl-defun-expander))
-		  byte-compile-macro-environment))))
+		  env))))
     (if (not (get (car (last cl-closure-vars)) 'used))
 	(list 'let (mapcar #'(lambda (x) (list (caddr x) (cadr x))) vars)
 	      (sublis (mapcar #'(lambda (x)
@@ -3888,7 +3894,7 @@
   (list 'progn form))
 
 ;;;###autoload
-(defmacro labels (bindings &rest body)
+(defmacro* labels (bindings &rest body &environment env)
   "Make temporary function bindings.
 
 This is like `flet', except the bindings are lexical instead of dynamic.
@@ -3908,8 +3914,7 @@
   ;; XEmacs; the byte-compiler has a much better implementation of `labels'
   ;; in `byte-compile-initial-macro-environment' that is used in compiled
   ;; code.
-  (let ((vars nil) (sets nil)
-        (byte-compile-macro-environment byte-compile-macro-environment))
+  (let ((vars nil) (sets nil))
     (while bindings
       (let ((var (gensym)))
 	(push var vars)
@@ -3919,9 +3924,8 @@
 	(push (list (car (pop bindings)) 'lambda '(&rest cl-labels-args)
 		       (list 'list* '(quote funcall) (list 'quote var)
 			     'cl-labels-args))
-		 byte-compile-macro-environment)))
-    (cl-macroexpand-all (list* 'lexical-let vars (cons (cons 'setq sets) body))
-			byte-compile-macro-environment)))
+              env)))
+    (cl-macroexpand-all `(lexical-let ,vars (setq ,@sets) ,@body) env)))
 
 ;;;###autoload
 (defmacro flet (functions &rest form)
--- a/man/ChangeLog	Sun May 06 05:22:19 2012 +0100
+++ b/man/ChangeLog	Sun May 06 15:29:59 2012 +0100
@@ -1,3 +1,9 @@
+2012-05-06  Aidan Kehoe  <kehoea@parhasard.net>
+
+	* lispref/macros.texi (Expansion):
+	Cross-reference to documentation of #'cl-prettyexpand, #'defmacro*
+	when talking about #'macroexpand.
+
 2012-05-04  Aidan Kehoe  <kehoea@parhasard.net>
 
 	* lispref/searching.texi (Regular Expressions):
--- a/man/lispref/macros.texi	Sun May 06 05:22:19 2012 +0100
+++ b/man/lispref/macros.texi	Sun May 06 15:29:59 2012 +0100
@@ -88,7 +88,9 @@
 this is unusual.
 
   You can see the expansion of a given macro call by calling
-@code{macroexpand}.
+@code{macroexpand}.  However, in normal use, @code{cl-prettyexpand} will be
+more helpful, since it expands @emph{all} the macros in the form, and prints
+the output with more readable indentation. @pxref{(cl)Efficiency Concerns}.
 
 @defun macroexpand form &optional environment
 @cindex macro expansion
@@ -106,9 +108,16 @@
 Normally there is no need for that, since a call to an inline function is
 no harder to understand than a call to an ordinary function.
 
-If @var{environment} is provided, it specifies an alist of macro
-definitions that shadow the currently defined macros.  Byte compilation
-uses this feature.
+If @var{environment} is provided, it specifies an alist of macro definitions
+that shadow the currently defined macros.  Byte compilation uses this feature.
+
+To access @var{environment} within the body of a macro, define the macro using
+@code{defmacro*} or @code{macrolet}, and use the @code{&environment} lambda
+list keyword.  This may be necessary if you need to force macro expansion of
+the body of a form at the same time as top-level macro expansion.
+@pxref{(cl)Argument Lists}.
+
+Macro expansion examples:
 
 @smallexample
 @group
--- a/src/ChangeLog	Sun May 06 05:22:19 2012 +0100
+++ b/src/ChangeLog	Sun May 06 15:29:59 2012 +0100
@@ -1,3 +1,14 @@
+012-05-06  Aidan Kehoe  <kehoea@parhasard.net>
+
+	* eval.c:
+	* eval.c (Fmacroexpand):
+	Don't prepend any supplied environment to
+	Vbyte_compile_macro_environment, leave that up to our callers
+	(that's what the &environment argument is for).
+	Document that one should normally access
+	byte-compile-macro-environment using the &environment lambda list
+	keyword.
+
 2012-05-04  Aidan Kehoe  <kehoea@parhasard.net>
 
 	* regex.c:
--- a/src/eval.c	Sun May 06 05:22:19 2012 +0100
+++ b/src/eval.c	Sun May 06 15:29:59 2012 +0100
@@ -1565,22 +1565,10 @@
   REGISTER Lisp_Object expander, sym, def, tem;
   int speccount = specpdl_depth ();
 
-  if (!NILP (environment) &&
-      !EQ (environment, Vbyte_compile_macro_environment))
-    {
-      if (NILP (Vbyte_compile_macro_environment))
-        {
-          specbind (Qbyte_compile_macro_environment, environment);
-        }
-      else
-        {
-          specbind (Qbyte_compile_macro_environment,
-                    nconc2 (Fcopy_list (environment),
-                            Vbyte_compile_macro_environment));
-        }
-    }
-
-  environment = Vbyte_compile_macro_environment;
+  if (!EQ (environment, Vbyte_compile_macro_environment))
+    {
+      specbind (Qbyte_compile_macro_environment, environment);
+    }
 
   while (1)
     {
@@ -7661,6 +7649,10 @@
 Alist of macros defined in the file being compiled.
 Each element looks like (MACRONAME . DEFINITION).  It is
 \(MACRONAME . nil) when a macro is redefined as a function.
+
+You should normally access this using the &environment argument to
+#'macrolet, #'defmacro* and friends, and not directly; see the documentation
+of those macros.
 */);
   Vbyte_compile_macro_environment = Qnil;
 
--- a/tests/ChangeLog	Sun May 06 05:22:19 2012 +0100
+++ b/tests/ChangeLog	Sun May 06 15:29:59 2012 +0100
@@ -1,3 +1,9 @@
+2012-05-06  Aidan Kehoe  <kehoea@parhasard.net>
+
+	* automated/lisp-tests.el:
+	Use &environment appropriately in #'macrolet, instead of relying
+	on #'macroexpand to guess what we mean.
+
 2012-05-04  Aidan Kehoe  <kehoea@parhasard.net>
 
 	* automated/regexp-tests.el (equal):
--- a/tests/automated/lisp-tests.el	Sun May 06 05:22:19 2012 +0100
+++ b/tests/automated/lisp-tests.el	Sun May 06 15:29:59 2012 +0100
@@ -2957,10 +2957,10 @@
         (append form (list 1 [hi there] 40 "this is a string" pi)))
        (with-second-arguments (&optional form)
          (append form (list pi e ''hello ''there [40 50 60])))
-       (with-both-arguments (&optional form)
+       (with-both-arguments (&optional form &environment env)
          (append form
-                 (macroexpand '(with-first-arguments))
-                 (macroexpand '(with-second-arguments)))))
+                 (macroexpand '(with-first-arguments) env)
+                 (macroexpand '(with-second-arguments) env))))
 
     (with-temp-buffer
       (Assert
@@ -2986,4 +2986,20 @@
       (Assert (not (funcall (intern "eq") #'bookend #'refer-to-bookend))
 	      "checking two mutually recursive functions compiled OK"))))
 
+;; Test macroexpand's handling of the ENVIRONMENT argument. We augmented it
+;; quietly for about four months, and this was incorrect.
+
+(Check-Error
+ void-variable
+ (macrolet
+     ((with-first-arguments (&optional form)
+        (append form (list 1 [hi there] 40 "this is a string" pi)))
+      (with-second-arguments (&optional form)
+        (append form (list pi e ''hello ''there [40 50 60])))
+      (with-both-arguments (&optional form)
+        (append form
+                (macroexpand '(with-first-arguments))
+                (macroexpand '(with-second-arguments)))))
+   (with-both-arguments (list))))
+
 ;;; end of lisp-tests.el