Implement minimal macro namespacing and add tests

This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  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 couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

Closes hylang/hy#1268, closes hylang/hy#1650, closes hylang/hy#1416.

hy/macros.py

@@ -263,10 +263,38 @@ def make_empty_fn_copy(fn):
 
 
 def macroexpand(tree, module, compiler=None, once=False):
-    """Expand the toplevel macros for the `tree`.
+    """Expand the toplevel macros for the given Hy AST tree.
 
-    Load the macros from the given `module`, then expand the
+    Load the macros from the given `module`, then expand the (top-level) macros
-    (top-level) macros in `tree` until we no longer can.
+    in `tree` until we no longer can.
+
+    `HyExpression` resulting from macro expansions are assigned the module in
+    which the macro function is defined (determined using `inspect.getmodule`).
+    If the resulting `HyExpression` is itself macro expanded, then the
+    namespace of the assigned module is checked first for a macro corresponding
+    to the expression's head/car symbol.  If the head/car symbol of such a
+    `HyExpression` is not found among the macros of its assigned module's
+    namespace, the outer-most namespace--e.g.  the one given by the `module`
+    parameter--is used as a fallback.
+
+    Parameters
+    ----------
+    tree: HyObject or list
+        Hy AST tree.
+
+    module: str or types.ModuleType
+        Module used to determine the local namespace for macros.
+
+    compiler: HyASTCompiler, optional
+        The compiler object passed to expanded macros.
+
+    once: boolean, optional
+        Only expand the first macro in `tree`.
+
+    Returns
+    ------
+    out: HyObject
+        Returns a mutated tree with macros expanded.
     """
     if not inspect.ismodule(module):
         module = importlib.import_module(module)
@@ -283,7 +311,14 @@ def macroexpand(tree, module, compiler=None, once=False):
             break
 
         fn = mangle(fn)
-        m = module.__macros__.get(fn, None)
+        expr_modules = (([] if not hasattr(tree, 'module') else [tree.module])
+            + [module])
+
+        # Choose the first namespace with the macro.
+        m = next((mod.__macros__[fn]
+                  for mod in expr_modules
+                  if fn in mod.__macros__),
+                 None)
         if not m:
             break
 
@@ -311,6 +346,10 @@ 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):
+            obj.module = inspect.getmodule(m)
+
         tree = replace_hy_obj(obj, tree)
 
         if once:
@@ -331,10 +370,21 @@ 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 = (([] if not hasattr(tree, 'module') else [tree.module])
+        + [module])
+
+    # Choose the first namespace with the macro.
+    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):
+        expr.module = inspect.getmodule(tag_macro)
+
     return replace_hy_obj(expr, tree)

hy/models.py

@@ -32,11 +32,12 @@ class HyObject(object):
     Generic Hy Object model. This is helpful to inject things into all the
     Hy lexing Objects at once.
     """
+    __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",
+            for attr in self.__properties__:
-                         "start_column", "end_column"]:
                 if not hasattr(self, attr) and hasattr(other, attr):
                     setattr(self, attr, getattr(other, attr))
         else:

tests/native_tests/native_macros.hy

@@ -2,7 +2,8 @@
 ;; This file is part of Hy, which is free software licensed under the Expat
 ;; license. See the LICENSE.
 
-(import [hy.errors [HyTypeError]])
+(import pytest
+        [hy.errors [HyTypeError]])
 
 (defmacro rev [&rest body]
   "Execute the `body` statements in reverse"
@@ -329,3 +330,124 @@
     (except [e SystemExit]
       (assert (= (str e) "42"))))
   (setv --name-- oldname))
+
+(defn test-macro-namespace-resolution []
+  "Confirm that local versions of macro-macro dependencies do not shadow the
+versions from the macro's own module, but do resolve unbound macro references
+in expansions."
+
+  ;; `nonlocal-test-macro` is a macro used within
+  ;; `tests.resources.macro-with-require.test-module-macro`.
+  ;; Here, we introduce an equivalently named version in local scope that, when
+  ;; used, will expand to a different output string.
+  (defmacro nonlocal-test-macro [x]
+    (print "this is the local version of `nonlocal-test-macro`!"))
+
+  ;; Was the above macro created properly?
+  (assert (in "nonlocal_test_macro" __macros__))
+
+  (setv nonlocal-test-macro (get __macros__ "nonlocal_test_macro"))
+
+  (require [tests.resources.macro-with-require [*]])
+
+  ;; Make sure our local version wasn't overwritten by a faulty `require` of the
+  ;; one in tests.resources.macro-with-require.
+  (assert (= nonlocal-test-macro (get __macros__ "nonlocal_test_macro")))
+
+  (setv module-name-var "tests.native_tests.native_macros.test-macro-namespace-resolution")
+  (assert (= (+ "This macro was created in tests.resources.macros, "
+                "expanded in tests.native_tests.native_macros.test-macro-namespace-resolution "
+                "and passed the value 2.")
+             (test-module-macro 2)))
+  (assert (= (+ "This macro was created in tests.resources.macros, "
+                "expanded in tests.native_tests.native_macros.test-macro-namespace-resolution "
+                "and passed the value 2.")
+             #test-module-tag 2))
+
+  ;; Now, let's use a `require`d macro that depends on another macro defined only
+  ;; in this scope.
+  (defmacro local-test-macro [x]
+    (.format "This is the local version of `nonlocal-test-macro` returning {}!" x))
+
+  (assert (= "This is the local version of `nonlocal-test-macro` returning 3!"
+             (test-module-macro-2 3)))
+  (assert (= "This is the local version of `nonlocal-test-macro` returning 3!"
+             #test-module-tag-2 3)))
+
+(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."
+
+  (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"))))
+
+  ;; Remove any cached byte-code, so that this runs from source and
+  ;; gets evaluated in this module.
+  (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)))
+
+  ;; Ensure that bytecode isn't present when we require this module.
+  (assert (not (os.path.isfile pyc-file)))
+
+  (defn test-requires-and-macros []
+    (require [tests.resources.macro-with-require
+              [test-module-macro]])
+
+    ;; Make sure that `require` didn't add any of its `require`s
+    (assert (not (in "nonlocal-test-macro" __macros__)))
+    ;; and that it didn't add its tags.
+    (assert (not (in "test_module_tag" __tags__)))
+
+    ;; Now, require everything.
+    (require [tests.resources.macro-with-require [*]])
+
+    ;; Again, make sure it didn't add its required macros and/or tags.
+    (assert (not (in "nonlocal-test-macro" __macros__)))
+
+    ;; Its tag(s) should be here now.
+    (assert (in "test_module_tag" __tags__))
+
+    ;; The test macro expands to include this symbol.
+    (setv module-name-var "tests.native_tests.native_macros")
+    (assert (= (+ "This macro was created in tests.resources.macros, "
+                  "expanded in tests.native_tests.native_macros "
+                  "and passed the value 1.")
+               (test-module-macro 1)))
+
+    (assert (= (+ "This macro was created in tests.resources.macros, "
+                  "expanded in tests.native_tests.native_macros "
+                  "and passed the value 1.")
+               #test-module-tag 1)))
+
+  (test-requires-and-macros)
+
+  ;; Now that bytecode is present, reload the module, clear the `require`d
+  ;; macros and tags, and rerun the tests.
+  (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.
+  (test-requires-and-macros))

tests/resources/macro_with_require.hy

@@ -0,0 +1,25 @@
+;; Require all the macros and make sure they don't pollute namespaces/modules
+;; that require `*` from this.
+(require [tests.resources.macros [*]])
+
+(defmacro test-module-macro [a]
+  "The variable `macro-level-var' here should not bind to the same-named symbol
+in the expansion of `nonlocal-test-macro'."
+  (setv macro-level-var "tests.resources.macros.macro-with-require")
+  `(nonlocal-test-macro ~a))
+
+(deftag test-module-tag [a]
+  "The variable `macro-level-var' here should not bind to the same-named symbol
+in the expansion of `nonlocal-test-macro'."
+  (setv macro-level-var "tests.resources.macros.macro-with-require")
+  `(nonlocal-test-macro ~a))
+
+(defmacro test-module-macro-2 [a]
+  "The macro `local-test-macro` isn't in this module's namespace, so it better
+ be in the expansion's!"
+  `(local-test-macro ~a))
+
+(deftag test-module-tag-2 [a]
+  "The macro `local-test-macro` isn't in this module's namespace, so it better
+ be in the expansion's!"
+  `(local-test-macro ~a))

tests/resources/macros.hy

@@ -1,3 +1,5 @@
+(setv module-name-var "tests.resources.macros")
+
 (defmacro thread-set-ab []
   (defn f [&rest args] (.join "" (+ (, "a") args)))
   (setv variable (HySymbol (-> "b" (f))))
@@ -10,3 +12,11 @@
 
 (defmacro test-macro []
   '(setv blah 1))
+
+(defmacro nonlocal-test-macro [x]
+  "When called from `macro-with-require`'s macro(s), the first instance of
+`module-name-var` should resolve to the value in the module where this is
+defined, then the expansion namespace/module"
+  `(.format (+ "This macro was created in {}, expanded in {} "
+               "and passed the value {}.")
+            ~module-name-var module-name-var ~x))