Repository: hylang/hy

Issue:
`try` form not working in `defmacro`
The simplest possible `try` form?
```Python
=> (try None (except [] None))
try:
    _hy_anon_var_1 = None
except:
    _hy_anon_var_1 = None
_hy_anon_var_1
```
Useless, but it works. Put it in a simple `defmacro`, and
```Hy
=> (defmacro foo [] (try None (except [] None)) `())
  File "<input>", line 1, column 18

  (defmacro foo [] (try None (except [] None)) `())
                   ^-------------------------^
HyTypeError: b"Unknown expression in `try'"
```
What happened?

But it works fine if we substitute in the expected `None`
```Python
=> (defmacro foo [] None `())
import hy
from hy import HyExpression

def _hy_anon_fn_1():
    None
    return HyExpression([])
hy.macros.macro('foo')(_hy_anon_fn_1)
<function _hy_anon_fn_1 at 0x000001B059952488>
```

Discussion:
Kodiologist: > Not sure how to test this though...

The first thing to try would be `` can_compile("(defmacro foo [] (try None (except [] None)) `())") `` in `test_ast`. Be sure it fails without the change, of course.
schaefed: Added a small test. Not sure why travis-ci is failing, on my local machine `make d` with Python 3.6 terminates without errors...
schaefed: I just reverted the changes and travis passes, so my stuff is wrong. I noticed that pytest collected 397 items (I guess test cases ?) in travis while this number is 534 on my machine. Any idea where this discrepancy comes from and how I could reproduce the travis check locally?
gilch: Are you developing on windows? It might be related to #1289.
schaefed: No, I am on Linux
Kodiologist: There should indeed be 534 tests, and that's what Travis got as recently as [this job](https://travis-ci.org/hylang/hy/jobs/261923604). Odd.
gilch: So which tests are missing? Are all the native tests not getting executed on Arch either? 
schaefed: I had a look into the travis logs again and it seems, that the tests already fail during the collection phase of pytest. None of the test did actually run, as pytest gave up after two Errors during the search. That should be the reason why the numbers of collected items deviate.

I now also managed to reproduce the errors locally, within a fresh python2 based virtualenv (still passing all 533 expected tests on python3 though). Not sure if I am able to fix them, at least the first import related error seems to originate deep within the internals of the compiler. And this thing is basically a black box for me...

I think I can give it a few more tries within the next a couple of days... 
schaefed: With #1377 in place this now works as intended.
Kodiologist: Looks good to me. Would you please

- Put the new test in `test_ast.py`, rather than putting it in a different file and importing `can_compile` from `test_ast.py`
- Put the URL `https://github.com/hylang/hy/issues/1350` in a comment at the top of the test
- Update NEWS
schaefed: Points 2 and 3, sure.
> Put the new test in test_ast.py, rather than putting it in a different file and importing can_compile from test_ast.py
Well, I consider the problem to be a compiler, rather than an ast issue, that's why I placed the test into `test_compiler`. If you just dislike the `import` from another test file (which I certainly do), maybe the cleanest solution would be to put `can_compile` into a separate test helper module (maybe something like that already exists?). However, your game your rules, so, if you tell me to move the test, I do...
Kodiologist: So far as I can tell, the only reason `test_compiler.py` exists separately from `test_ast.py` is that historically they had different implementations: `test_ast.py` used Python's built-in facilities whereas `test_compiler.py` used Nose. There isn't a very meaningful difference in what is actually tested, although the names of the files suggest there might be such a difference.
schaefed: Thanks for the clarification.
Kodiologist: At any rate, given the tests that are already in the files, this test is more like the ones in `test_ast.py` and hence seems to belong there. In the future, we should probably move the handful of tests in `test_compiler.py` into `test_ast.py`, then get rid of the `compilers` directory, and perhaps rename `test_ast.py` to `test_compiler.py`.
schaefed: Done
Kodiologist: Okay, could you change the second commit message to explain what is tested, or just meld the commits together?
Kodiologist: Sure, use `git rebase -i master`, or `git reset HEAD~` followed by `git commit -a --amend`.

Patch:
diff --git a/NEWS b/NEWS
index 4a2a9e59..81be8122 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@ Changes from 0.13.0
      `(quit)` or `(exit)`
    * `exec` now works under Python 2
    * No TypeError from multi-arity defn returning values evaluating to None
+   * try form now possible in defmacro/deftag
 
    [ Misc. Improvements ]
    * `read`, `read_str`, and `eval` are exposed and documented as top-level
diff --git a/hy/compiler.py b/hy/compiler.py
index 4e30954b..51a86a9f 100755
--- a/hy/compiler.py
+++ b/hy/compiler.py
@@ -23,6 +23,7 @@ import codecs
 import ast
 import sys
 import keyword
+import copy
 
 from collections import defaultdict
 
@@ -2392,7 +2393,8 @@ class HyASTCompiler(object):
         """Compile-time hack: we want to get our new macro now
         We must provide __name__ in the namespace to make the Python
         compiler set the __module__ attribute of the macro function."""
-        hy.importer.hy_eval(expression,
+
+        hy.importer.hy_eval(copy.deepcopy(expression),
                             compile_time_ns(self.module_name),
                             self.module_name)
 


Test patch:
diff --git a/tests/compilers/test_ast.py b/tests/compilers/test_ast.py
index 1e863042..b5d4860b 100644
--- a/tests/compilers/test_ast.py
+++ b/tests/compilers/test_ast.py
@@ -617,3 +617,10 @@ def test_exec_star():
         assert code.body.s == "print(a + b)"
         assert code.globals.keys[0].s == "a"
         assert code.locals.keys[0].s == "b"
+
+
+def test_compiler_macro_tag_try():
+    """Check that try forms within defmacro/deftag are compiled correctly"""
+    # https://github.com/hylang/hy/issues/1350
+    can_compile("(defmacro foo [] (try None (except [] None)) `())")
+    can_compile("(deftag foo [] (try None (except [] None)) `())")