From 010986e8cabf7b3b308ffc0cd679cc8e6d8e036a Mon Sep 17 00:00:00 2001 From: "Brandon T. Willard" Date: Tue, 23 Oct 2018 14:06:22 -0400 Subject: [PATCH] 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 | 60 +++++++++++-- hy/models.py | 5 +- tests/native_tests/native_macros.hy | 124 +++++++++++++++++++++++++- tests/resources/macro_with_require.hy | 25 ++++++ tests/resources/macros.hy | 10 +++ 5 files changed, 216 insertions(+), 8 deletions(-) create mode 100644 tests/resources/macro_with_require.hy diff --git a/hy/macros.py b/hy/macros.py index f3b64ff..0a6bc1b 100644 --- a/hy/macros.py +++ b/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 - (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) @@ -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) diff --git a/hy/models.py b/hy/models.py index 943458e..2ff5efa 100644 --- a/hy/models.py +++ b/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", - "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: diff --git a/tests/native_tests/native_macros.hy b/tests/native_tests/native_macros.hy index ef6ae26..73b433b 100644 --- a/tests/native_tests/native_macros.hy +++ b/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)) diff --git a/tests/resources/macro_with_require.hy b/tests/resources/macro_with_require.hy new file mode 100644 index 0000000..ef25018 --- /dev/null +++ b/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)) diff --git a/tests/resources/macros.hy b/tests/resources/macros.hy index 300a790..4a17a05 100644 --- a/tests/resources/macros.hy +++ b/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)) \ No newline at end of file