From 4b8b70672f427eef60c383f2bc640016346f92ea Mon Sep 17 00:00:00 2001 From: "Brandon T. Willard" Date: Tue, 25 Sep 2018 16:12:54 -0500 Subject: [PATCH] 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 hylang/hy#1268, closes hylang/hy#1650, closes hylang/hy#1416. --- hy/compiler.py | 7 +-- hy/importer.py | 6 ++- hy/macros.py | 48 +++++++++++++++++--- hy/models.py | 11 +++-- tests/native_tests/native_macros.hy | 65 +++++++++++++++++++++++++++ tests/resources/macro_with_require.hy | 7 +++ 6 files changed, 130 insertions(+), 14 deletions(-) create mode 100644 tests/resources/macro_with_require.hy diff --git a/hy/compiler.py b/hy/compiler.py index 0058b329e..a5e4dceb4 100755 --- a/hy/compiler.py +++ b/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): diff --git a/hy/importer.py b/hy/importer.py index d6118504b..1ce3a06e5 100644 --- a/hy/importer.py +++ b/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): diff --git a/hy/macros.py b/hy/macros.py index 75d06260c..737157484 100644 --- a/hy/macros.py +++ b/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) diff --git a/hy/models.py b/hy/models.py index 943458e03..141d8a1fe 100644 --- a/hy/models.py +++ b/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): diff --git a/tests/native_tests/native_macros.hy b/tests/native_tests/native_macros.hy index e4436b262..648ed9069 100644 --- a/tests/native_tests/native_macros.hy +++ b/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))) diff --git a/tests/resources/macro_with_require.hy b/tests/resources/macro_with_require.hy new file mode 100644 index 000000000..3fc7757d8 --- /dev/null +++ b/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)))