Skip to content

Commit

Permalink
Implement minimal macro namespacing and add tests
Browse files Browse the repository at this point in the history
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#1268, closes hylang#1650, closes hylang#1416.
  • Loading branch information
brandonwillard authored and Kodiologist committed Oct 23, 2018
1 parent 11022d2 commit 2985cb5
Show file tree
Hide file tree
Showing 5 changed files with 216 additions and 8 deletions.
60 changes: 55 additions & 5 deletions hy/macros.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,38 @@ def empty_fn(*args, **kwargs):


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
(top-level) macros in `tree` until we no longer can.
Load the macros from the given `module`, then expand the (top-level) macros
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)
Expand All @@ -262,7 +290,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

Expand Down Expand Up @@ -290,6 +325,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:
Expand All @@ -310,10 +349,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)
5 changes: 3 additions & 2 deletions hy/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
"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:
Expand Down
124 changes: 123 additions & 1 deletion tests/native_tests/native_macros.hy
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
25 changes: 25 additions & 0 deletions tests/resources/macro_with_require.hy
Original file line number Diff line number Diff line change
@@ -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))
10 changes: 10 additions & 0 deletions tests/resources/macros.hy
Original file line number Diff line number Diff line change
@@ -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))))
Expand All @@ -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))

0 comments on commit 2985cb5

Please sign in to comment.