Implement minimal macro namespacing and add tests

This commit adds just enough namespacing to resolve a macro first in the
Hy compilation namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace it was defined.  Namespacing is accomplished by adding
a `module` attribute to `HySymbol`, so that `HyExpression`s can be checked
for this definition namespace attribute and their car symbol resolved per the
above.

As well, a test is included that covers module-level macros (i.e. via `__macros`
and `__tags__`), AST generated for `require`, and the non-local macro namespace
resolution described above.

Closes #1268, closes #1650, closes #1416.

hy/compiler.py

@@ -1752,7 +1752,8 @@ def compile_dict(self, m):
         return ret + asty.Dict(m, keys=keyvalues[::2], values=keyvalues[1::2])
 
 
-def hy_compile(tree, module, root=ast.Module, get_expr=False, include_require=True):
+def hy_compile(tree, module, root=ast.Module, get_expr=False,
+               include_require=True):
     """
     Compile a Hy tree into a Python AST tree.
 
@@ -1782,9 +1783,9 @@ def hy_compile(tree, module, root=ast.Module, get_expr=False, include_require=Tr
             module = types.ModuleType(module)
         else:
             module = importlib.import_module(ast_str(module, piecewise=True))
-    elif not inspect.ismodule(module):
-        raise TypeError('Invalid module type: {}'.format(type(module)))
 
+    if not inspect.ismodule(module):
+        raise TypeError('Invalid module type: {}'.format(type(module)))
 
     tree = wrap_value(tree)
     if not isinstance(tree, HyObject):

hy/importer.py

@@ -18,6 +18,7 @@
 
 from functools import partial
 from contextlib import contextmanager
+from warnings import warn
 
 from hy.errors import HyTypeError
 from hy.compiler import hy_compile, ast_str
@@ -383,6 +384,7 @@ def load_module(self, fullname=None):
                     if self.file:
                         self.file.close()
 
+            mod.__cached__ = cache_from_source(self.filename)
             mod.__loader__ = self
             return mod
 
@@ -417,8 +419,8 @@ def byte_compile_hy(self, fullname=None):
             if not sys.dont_write_bytecode:
                 try:
                     hyc_compile(code, module=fullname)
-                except IOError:
-                    pass
+                except IOError as e:
+                    warn('Could not write Hy bytecode: {}'.format(e))
             return code
 
         def get_code(self, fullname=None):

hy/macros.py

@@ -198,12 +198,22 @@ def macroexpand(tree, module, compiler=None, once=False):
         if not isinstance(tree, HyExpression) or tree == []:
             break
 
-        fn = tree[0]
-        if fn in ("quote", "quasiquote") or not isinstance(fn, HySymbol):
+        fn_symbol = tree[0]
+        if (fn_symbol in ("quote", "quasiquote") or
+            not isinstance(fn_symbol, HySymbol)):
             break
 
-        fn = mangle(fn)
-        m = module.__macros__.get(fn, None)
+        fn_name = mangle(fn_symbol)
+        expr_modules = [module]
+        # TODO: Use an ordered set?
+        # (e.g. https://code.activestate.com/recipes/576694/)
+        if hasattr(tree, 'module'):
+            expr_modules += [tree.module]
+
+        m = next((mod.__macros__[fn_name]
+                  for mod in expr_modules
+                  if fn_name in mod.__macros__),
+                 None)
         if not m:
             break
 
@@ -231,6 +241,21 @@ def macroexpand(tree, module, compiler=None, once=False):
         except Exception as e:
             msg = "expanding `" + str(tree[0]) + "': " + repr(e)
             raise HyMacroExpansionError(tree, msg)
+
+        if isinstance(obj, HyExpression):
+            macro_module = inspect.getmodule(m)
+            # TODO: We could also consider [temporarily] expanding the compiler
+            # module's namespace by including macros and/or tags from the
+            # expression's namespace (without overwriting, so that local scope
+            # is always preferred).
+            obj.module = macro_module
+
+        # TODO: Do we need to generate AST with module prefixes.
+        # If so, the `func` kwarg of `ast.Call` for `os.path.isfile`,
+        # would need to be as follows:
+        # `Attribute(value=Attribute(value=Name(id='os', ctx=Load()),
+        #                            attr='path', ctx=Load()),
+        #            attr='isfile', ctx=Load())`
         tree = replace_hy_obj(obj, tree)
 
         if once:
@@ -251,10 +276,23 @@ def tag_macroexpand(tag, tree, module):
     if not inspect.ismodule(module):
         module = importlib.import_module(module)
 
-    tag_macro = module.__tags__.get(tag, None)
+    expr_modules = [module]
+
+    if hasattr(tree, 'module'):
+        expr_modules += [tree.module]
+
+    tag_macro = next((mod.__tags__[tag]
+                      for mod in expr_modules
+                      if tag in mod.__tags__),
+                     None)
 
     if tag_macro is None:
         raise HyTypeError(tag, "'{0}' is not a defined tag macro.".format(tag))
 
     expr = tag_macro(tree)
+
+    if isinstance(expr, HyExpression):
+        macro_module = inspect.getmodule(tag_macro)
+        expr.module = macro_module
+
     return replace_hy_obj(expr, tree)

hy/models.py

@@ -32,11 +32,13 @@ class HyObject(object):
     Generic Hy Object model. This is helpful to inject things into all the
     Hy lexing Objects at once.
     """
+    # TODO: Use slots?
+    __properties__ = ["module", "start_line", "end_line", "start_column",
+                      "end_column"]
 
     def replace(self, other, recursive=False):
         if isinstance(other, HyObject):
-            for attr in ["start_line", "end_line",
-                         "start_column", "end_column"]:
+            for attr in self.__properties__:
                 if not hasattr(self, attr) and hasattr(other, attr):
                     setattr(self, attr, getattr(other, attr))
         else:
@@ -45,7 +47,8 @@ def replace(self, other, recursive=False):
         return self
 
     def __repr__(self):
-        return "%s(%s)" % (self.__class__.__name__, super(HyObject, self).__repr__())
+        return "%s(%s)" % (self.__class__.__name__,
+                           super(HyObject, self).__repr__())
 
 
 _wrappers = {}
@@ -247,7 +250,7 @@ def replace(self, other, recursive=True):
         if recursive:
             for x in self:
                 replace_hy_obj(x, other)
-        HyObject.replace(self, other)
+        super(HySequence, self).replace(other)
         return self
 
     def __add__(self, other):

tests/native_tests/native_macros.hy

@@ -329,3 +329,68 @@
     (except [e SystemExit]
       (assert (= (str e) "42"))))
   (setv --name-- oldname))
+
+(defn test-macro-from-module []
+  "Macros loaded from an external module, which itself `require`s macros, should
+ work without having to `require` the module's macro dependencies (due to
+ [minimal] macro namespace resolution).
+
+ In doing so we also confirm that a module's `__macros__` attribute is correctly
+ loaded and used.
+
+ Additionally, we confirm that `require` statements are executed via loaded bytecode."
+
+  ;; Remove any cached byte-code for the module.
+  (import os sys marshal types)
+  (import [hy.importer [cache_from_source]])
+
+  (setv pyc-file (cache_from_source
+                   (os.path.realpath
+                     (os.path.join
+                       "tests" "resources" "macro_with_require.hy"))))
+
+  (when (os.path.isfile pyc-file)
+    (os.unlink pyc-file)
+    (.clear sys.path_importer_cache)
+    (when (in  "tests.resources.macro_with_require" sys.modules)
+      (del (get sys.modules "tests.resources.macro_with_require"))
+      (__macros__.clear)
+      (__tags__.clear)))
+
+  (require [tests.resources.macro-with-require [test-module-macro]])
+  (assert (not (in "test_module_tag" __tags__)))
+
+  (require [tests.resources.macro-with-require [test-module-tag]])
+  (assert (in "test_module_tag" __tags__))
+
+  (assert (= 2 (test-module-macro 1)))
+  (assert (= 2 #test-module-tag 1))
+
+  ;; Make sure `require` doesn't add macros that aren't defined in the
+  ;; `require`d module (e.g. the module's `require`s).
+  (assert (not (in "let" __macros__)))
+
+  (assert (os.path.isfile pyc-file))
+
+  ;; Reload the module and clear the local macro context.
+  (.clear sys.path_importer_cache)
+  (del (get sys.modules "tests.resources.macro_with_require"))
+  (.clear __macros__)
+  (.clear __tags__)
+
+  ;; XXX: There doesn't seem to be a way--via standard import mechanisms--to
+  ;; ensure that an imported module used the cached bytecode.  We'll simply have
+  ;; to trust that the .pyc loading convention was followed.
+
+  (require [tests.resources.macro-with-require [test-module-macro]])
+  (assert (not (in "test_module_tag" __tags__)))
+
+  (require [tests.resources.macro-with-require [test-module-tag]])
+  (assert (in "test_module_tag" __tags__))
+
+  ;; Make sure the cached/byte-compiled `require` doesn't add external macros,
+  ;; too.
+  (assert (not (in "let" __macros__)))
+
+  (assert (= 2 (test-module-macro 1)))
+  (assert (= 2 #test-module-tag 1)))

tests/resources/macro_with_require.hy

@@ -0,0 +1,7 @@
+(require [hy.contrib.walk [let]])
+
+(defmacro test-module-macro [a]
+  `(let [b 1] (+ b ~a)))
+
+(deftag test-module-tag [a]
+  `(let [b 1] (+ b ~a)))