add defmacro! and fix macro expansion error message (#1172)

* added defmacro!

* revert #924

#924 had an error and should never have been merged in the first place. (see #903)

* put back import getargspec

Without the `formatargspec` this time.

* Give better error message on failed macro expansion

Better error messages work most of the time. In cases where there are
parameters that aren't valid in Python, error message shown is rather
ugly. But this is better than no error messages at all and such
macros with strange parameter names are rather rare.

* fix flake8 errors

* Minor English improvements

docs/language/api.rst

@@ -696,6 +696,35 @@ For example, ``g!a`` would become ``(gensym "a")``.
 
    Section :ref:`using-gensym`
 
+.. _defmacro!:
+
+defmacro!
+---------
+
+``defmacro!`` is like ``defmacro/g!`` plus automatic once-only evaluation for
+``o!`` parameters, which are available as the equivalent ``g!`` symbol.
+
+For example,
+
+.. code-block:: clj
+
+    => (defn expensive-get-number [] (print "spam") 14)
+    => (defmacro triple-1 [n] `(+ n n n))
+    => (triple-1 (expensive-get-number))  ; evals n three times
+    spam
+    spam
+    spam
+    42
+    => (defmacro/g! triple-2 [n] `(do (setv ~g!n ~n) (+ ~g!n ~g!n ~g!n)))
+    => (triple-2 (expensive-get-number))  ; avoid repeats with a gensym
+    spam
+    42
+    => (defmacro! triple-3 [o!n] `(+ ~g!n ~g!n ~g!n))
+    => (triple-3 (expensive-get-number))  ; easier with defmacro!
+    spam
+    42
+
+
 defreader
 ---------
 

hy/core/macros.hy

@@ -221,6 +221,14 @@
        (let ~gensyms
          ~@body))))
 
+(defmacro defmacro! [name args &rest body]
+  "Like defmacro/g! plus automatic once-only evaluation for o!
+   parameters, which are available as the equivalent g! symbol."
+  (setv os (list-comp s [s args] (.startswith s "o!"))
+        gs (list-comp (HySymbol (+ "g!" (cut s 2))) [s os]))
+  `(defmacro/g! ~name ~args
+     `(do (setv ~@(interleave ~gs ~os))
+          ~@~body)))
 
 (if-python2
   (defmacro/g! yield-from [expr]

hy/macros.py

@@ -52,8 +52,14 @@ def macro(name):
 
     """
     def _(fn):
-        argspec = getargspec(fn)
-        fn._hy_macro_pass_compiler = argspec.keywords is not None
+        try:
+            argspec = getargspec(fn)
+            fn._hy_macro_pass_compiler = argspec.keywords is not None
+        except Exception:
+            # An exception might be raised if fn has arguments with
+            # names that are invalid in Python.
+            fn._hy_macro_pass_compiler = False
+
         module_name = fn.__module__
         if module_name.startswith("hy.core"):
             module_name = None
@@ -140,12 +146,24 @@ def load_macros(module_name):
 
 
 def make_empty_fn_copy(fn):
-    argspec = getargspec(fn)
-    formatted_args = formatargspec(*argspec)
-    fn_str = 'lambda {}: None'.format(
-        formatted_args.lstrip('(').rstrip(')'))
+    try:
+        # This might fail if fn has parameters with funny names, like o!n. In
+        # such a case, we return a generic function that ensures the program
+        # can continue running. Unfortunately, the error message that might get
+        # raised later on while expanding a macro might not make sense at all.
+
+        argspec = getargspec(fn)
+        formatted_args = formatargspec(*argspec)
+
+        fn_str = 'lambda {}: None'.format(
+            formatted_args.lstrip('(').rstrip(')'))
+        empty_fn = eval(fn_str)
+
+    except Exception:
+
+        def empty_fn(*args, **kwargs):
+            None
 
-    empty_fn = eval(fn_str)
     return empty_fn
 
 
@@ -194,6 +212,7 @@ def macroexpand_1(tree, compiler):
                     msg = "expanding `" + str(tree[0]) + "': "
                     msg += str(e).replace("<lambda>()", "", 1).strip()
                     raise HyMacroExpansionError(tree, msg)
+
                 try:
                     obj = wrap_value(m(*ntree[1:], **opts))
                 except HyTypeError as e:

tests/native_tests/native_macros.hy

@@ -202,6 +202,42 @@
   (setv macro2 "(defmacro/g! two-point-zero [] `(+ (float 1) 1.0))")
   (assert (import_buffer_to_ast macro2 "foo")))
 
+(defn test-defmacro! []
+  ;; defmacro! must do everything defmacro/g! can
+  (import ast)
+  (import [astor.codegen [to_source]])
+  (import [hy.importer [import_buffer_to_ast]])
+  (setv macro1 "(defmacro! nif [expr pos zero neg]
+        `(let [~g!res ~expr]
+           (cond [(pos? ~g!res) ~pos]
+                 [(zero? ~g!res) ~zero]
+                 [(neg? ~g!res) ~neg])))
+
+    (print (nif (inc -1) 1 0 -1))
+    ")
+  ;; expand the macro twice, should use a different
+  ;; gensym each time
+  (setv _ast1 (import_buffer_to_ast macro1 "foo"))
+  (setv _ast2 (import_buffer_to_ast macro1 "foo"))
+  (setv s1 (to_source _ast1))
+  (setv s2 (to_source _ast2))
+  (assert (in ":res_" s1))
+  (assert (in ":res_" s2))
+  (assert (not (= s1 s2)))
+
+  ;; defmacro/g! didn't like numbers initially because they
+  ;; don't have a startswith method and blew up during expansion
+  (setv macro2 "(defmacro! two-point-zero [] `(+ (float 1) 1.0))")
+  (assert (import_buffer_to_ast macro2 "foo"))
+
+  (defmacro! foo! [o!foo] `(do ~g!foo ~g!foo))
+  ;; test that o! becomes g!
+  (assert (= "Hy" (foo! "Hy")))
+  ;; test that o! is evaluated once only
+  (setv foo 40)
+  (foo! (+= foo 1))
+  (assert (= 41 foo)))
+
 
 (defn test-if-not []
   (assert (= (if-not True :yes :no)