diff --git a/CHANGELOG.md b/CHANGELOG.md index 202399ab1..942ef7146 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed * The compiler will issue a warning when adding any alias that might conflict with any other alias (#1045) + * The compiler is now capable of unrolling top level `do` forms (not including `do` forms emitted by macros) (#1028) ### Fixed * Fix a bug where Basilisp did not respect the value of Python's `sys.dont_write_bytecode` flag when generating bytecode (#1054) diff --git a/docs/runtime.rst b/docs/runtime.rst index ffa739791..0e73f39a8 100644 --- a/docs/runtime.rst +++ b/docs/runtime.rst @@ -111,6 +111,27 @@ This is roughly analogous to the Java classpath in Clojure. These values may be set manually, but are more often configured by some project management tool such as Poetry or defined in your Python virtualenv. These values may also be set via :ref:`cli` arguments. +Requiring Code Dynamically +########################## + +The Basilisp compiler attempts to verify the existence of Vars and Python module members during its analysis phase. +It typically does that by introspecting the runtime environment (Namespaces or Python modules). +Requiring a namespace in a highly dynamic context (e.g. from within a function call) and then immediately attempting to reference that value prevents the compiler from verifying references (which is an error). + +.. code-block:: + + ((fn [] + (require 'basilisp.set) + (basilisp.set/difference #{:b} #{:a}))) + ;; => error occurred during macroexpansion: unable to resolve symbol 'basilisp.set/difference' in this context + +In such cases, it may be preferable to use :lpy:fn:`requiring-resolve` to dynamically require and resolve the Var rather than fighting the compiler: + +.. code-block:: + + ((fn [] + ((requiring-resolve 'basilisp.set/difference) #{:b} #{:a}))) ;; => #{:b} + .. _namespace_imports: Namespace Imports diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index bc1c5c089..c378eef86 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -5068,7 +5068,7 @@ .. warning:: Reloading namespaces has many of the same limitations described for - :external:py:func:`importlib.reload_module`. Below is a non-exhaustive set of + :external:py:func:`importlib.reload`. Below is a non-exhaustive set of limitations of reloading Basilisp namespace: - Vars will be re-``def``'ed based on the current definition of the underlying diff --git a/src/basilisp/lang/compiler/__init__.py b/src/basilisp/lang/compiler/__init__.py index f09e76470..470b0077d 100644 --- a/src/basilisp/lang/compiler/__init__.py +++ b/src/basilisp/lang/compiler/__init__.py @@ -23,6 +23,7 @@ macroexpand, macroexpand_1, ) +from basilisp.lang.compiler.constants import SpecialForm from basilisp.lang.compiler.exception import CompilerException, CompilerPhase # noqa from basilisp.lang.compiler.generator import ( USE_VAR_INDIRECTION, @@ -34,6 +35,7 @@ from basilisp.lang.compiler.generator import gen_py_ast, py_module_preamble from basilisp.lang.compiler.generator import statementize as _statementize from basilisp.lang.compiler.optimizer import PythonASTOptimizer +from basilisp.lang.interfaces import ISeq from basilisp.lang.typing import CompilerOpts, ReaderForm from basilisp.lang.util import genname from basilisp.util import Maybe @@ -148,6 +150,18 @@ def _emit_ast_string( runtime.add_generated_python(to_py_str(module), which_ns=ns) +def _flatmap_forms(forms: Iterable[ReaderForm]) -> Iterable[ReaderForm]: + """Flatmap over an iterable of forms, unrolling any top-level `do` forms""" + for form in forms: + if isinstance(form, ISeq) and form.first == SpecialForm.DO: + yield from _flatmap_forms(form.rest) + else: + yield form + + +_sentinel = object() + + def compile_and_exec_form( form: ReaderForm, ctx: CompilerContext, @@ -166,34 +180,42 @@ def compile_and_exec_form( if not ns.module.__basilisp_bootstrapped__: _bootstrap_module(ctx.generator_context, ctx.py_ast_optimizer, ns) - final_wrapped_name = genname(wrapped_fn_name) - - lisp_ast = analyze_form(ctx.analyzer_context, form) - py_ast = gen_py_ast(ctx.generator_context, lisp_ast) - form_ast = list( - map( - _statementize, - itertools.chain( - py_ast.dependencies, - [_expressionize(GeneratedPyAST(node=py_ast.node), final_wrapped_name)], - ), + last = _sentinel + for unrolled_form in _flatmap_forms([form]): + final_wrapped_name = genname(wrapped_fn_name) + lisp_ast = analyze_form(ctx.analyzer_context, unrolled_form) + py_ast = gen_py_ast(ctx.generator_context, lisp_ast) + form_ast = list( + map( + _statementize, + itertools.chain( + py_ast.dependencies, + [ + _expressionize( + GeneratedPyAST(node=py_ast.node), final_wrapped_name + ) + ], + ), + ) ) - ) - ast_module = ast.Module(body=form_ast, type_ignores=[]) - ast_module = ctx.py_ast_optimizer.visit(ast_module) - ast.fix_missing_locations(ast_module) + ast_module = ast.Module(body=form_ast, type_ignores=[]) + ast_module = ctx.py_ast_optimizer.visit(ast_module) + ast.fix_missing_locations(ast_module) - _emit_ast_string(ns, ast_module) + _emit_ast_string(ns, ast_module) - bytecode = compile(ast_module, ctx.filename, "exec") - if collect_bytecode: - collect_bytecode(bytecode) - exec(bytecode, ns.module.__dict__) # pylint: disable=exec-used - try: - return getattr(ns.module, final_wrapped_name)() - finally: - del ns.module.__dict__[final_wrapped_name] + bytecode = compile(ast_module, ctx.filename, "exec") + if collect_bytecode: + collect_bytecode(bytecode) + exec(bytecode, ns.module.__dict__) # pylint: disable=exec-used + try: + last = getattr(ns.module, final_wrapped_name)() + finally: + del ns.module.__dict__[final_wrapped_name] + + assert last is not _sentinel, "Must compile at least one form" + return last def _incremental_compile_module( @@ -257,7 +279,7 @@ def compile_module( """ _bootstrap_module(ctx.generator_context, ctx.py_ast_optimizer, ns) - for form in forms: + for form in _flatmap_forms(forms): nodes = gen_py_ast( ctx.generator_context, analyze_form(ctx.analyzer_context, form) ) diff --git a/tests/basilisp/compiler_test.py b/tests/basilisp/compiler_test.py index 88a530665..b76528b7a 100644 --- a/tests/basilisp/compiler_test.py +++ b/tests/basilisp/compiler_test.py @@ -796,6 +796,14 @@ def test_deftype_interface_must_be_host_form(self, lcompile: CompileFn): (def Shape (python/type "Shape" #py () #py {})) (deftype* Circle [radius] :implements [Shape]))""", + compiler.CompilerException, + ), + ( + """ + ((fn [] + (def Shape (python/type "Shape" #py () #py {})) + (deftype* Circle [radius] + :implements [Shape])))""", runtime.RuntimeException, ), ], @@ -848,11 +856,9 @@ def test_deftype_allows_empty_dynamic_abstract_interface(self, lcompile: Compile compiler.CompilerException, ), ( - # TODO: it's currently a bug for the `(import* abc)` to appear - # in the same (do ...) block as the rest of this code. """ - (import* abc) (do + (import* abc) (def Shape (python/type "Shape" #py (abc/ABC) @@ -862,6 +868,21 @@ def test_deftype_allows_empty_dynamic_abstract_interface(self, lcompile: Compile (deftype* Circle [radius] :implements [Shape])) """, + compiler.CompilerException, + ), + ( + """ + ((fn [] + (import* abc) + (def Shape + (python/type "Shape" + #py (abc/ABC) + #py {"area" + (abc/abstractmethod + (fn []))})) + (deftype* Circle [radius] + :implements [Shape]))) + """, runtime.RuntimeException, ), ], @@ -889,11 +910,9 @@ def test_deftype_interface_must_implement_all_abstract_methods( compiler.CompilerException, ), ( - # TODO: it's currently a bug for the `(import* abc)` to appear - # in the same (do ...) block as the rest of this code. """ - (import* abc collections.abc) (do + (import* abc collections.abc) (def Shape (python/type "Shape" #py (abc/ABC) @@ -905,6 +924,23 @@ def test_deftype_interface_must_implement_all_abstract_methods( (area [this] (* 2 radius radius)) (call [this] :called))) """, + compiler.CompilerException, + ), + ( + """ + ((fn [] + (import* abc collections.abc) + (def Shape + (python/type "Shape" + #py (abc/ABC) + #py {"area" + (abc/abstractmethod + (fn []))})) + (deftype* Circle [radius] + :implements [Shape] + (area [this] (* 2 radius radius)) + (call [this] :called)))) + """, runtime.RuntimeException, ), ], @@ -1104,6 +1140,17 @@ def test_deftype_allows_artificially_abstract_super_type( :implements [^:abstract AABase] (some-method [this]) (other-method [this])))""", + compiler.CompilerException, + ), + ( + """ + ((fn [] + (def AABase + (python/type "AABase" #py () #py {"some_method" (fn [this])})) + (deftype* SomeAction [] + :implements [^:abstract AABase] + (some-method [this]) + (other-method [this]))))""", runtime.RuntimeException, ), ], @@ -1376,10 +1423,6 @@ def class_interface(self, lcompile: CompileFn): compiler.CompilerException, ), ( - # TODO: it's currently a bug for the `(import* abc)` to appear - # in the same (do ...) block as the rest of this code; - # but it's still working because `abc` was imported by the - # auto-used fixture for this class """ (do (import* abc) @@ -1393,6 +1436,22 @@ def class_interface(self, lcompile: CompileFn): (deftype* Point [x y z] :implements [WithClassMethod])) """, + compiler.CompilerException, + ), + ( + """ + ((fn [] + (import* abc) + (def WithClassMethod + (python/type "WithCls" + #py (abc/ABC) + #py {"make" + (python/classmethod + (abc/abstractmethod + (fn [cls])))})) + (deftype* Point [x y z] + :implements [WithClassMethod]))) + """, runtime.RuntimeException, ), ], @@ -1991,10 +2050,6 @@ def property_interface(self, lcompile: CompileFn): compiler.CompilerException, ), ( - # TODO: it's currently a bug for the `(import* abc)` to appear - # in the same (do ...) block as the rest of this code; - # but it's still working because `abc` was imported by the - # auto-used fixture for this class """ (do (import* abc) @@ -2008,6 +2063,22 @@ def property_interface(self, lcompile: CompileFn): (deftype* Point [x y z] :implements [WithProperty])) """, + compiler.CompilerException, + ), + ( + """ + ((fn [] + (import* abc) + (def WithProperty + (python/type "WithProp" + #py (abc/ABC) + #py {"a_property" + (python/property + (abc/abstractmethod + (fn [self])))})) + (deftype* Point [x y z] + :implements [WithProperty]))) + """, runtime.RuntimeException, ), ], @@ -2163,29 +2234,41 @@ def static_interface(self, lcompile: CompileFn): [ ( """ - (deftype* Point [x y z] - :implements [WithCls]) - """, + (deftype* Point [x y z] + :implements [WithCls]) + """, compiler.CompilerException, ), ( - # TODO: it's currently a bug for the `(import* abc)` to appear - # in the same (do ...) block as the rest of this code; - # but it's still working because `abc` was imported by the - # auto-used fixture for this class """ - (do - (import* abc) - (def WithStaticMethod - (python/type "WithStatic" - #py (abc/ABC) - #py {"do_static_method" - (python/staticmethod - (abc/abstractmethod - (fn [])))})) - (deftype* Point [x y z] - :implements [WithStaticMethod])) - """, + (do + (import* abc) + (def WithStaticMethod + (python/type "WithStatic" + #py (abc/ABC) + #py {"do_static_method" + (python/staticmethod + (abc/abstractmethod + (fn [])))})) + (deftype* Point [x y z] + :implements [WithStaticMethod])) + """, + compiler.CompilerException, + ), + ( + """ + ((fn [] + (import* abc) + (def WithStaticMethod + (python/type "WithStatic" + #py (abc/ABC) + #py {"do_static_method" + (python/staticmethod + (abc/abstractmethod + (fn [])))})) + (deftype* Point [x y z] + :implements [WithStaticMethod]))) + """, runtime.RuntimeException, ), ], @@ -4501,6 +4584,13 @@ def test_reify_interface_must_be_host_form(self, lcompile: CompileFn): (do (def Shape (python/type "Shape" #py () #py {})) (reify* :implements [Shape]))""", + compiler.CompilerException, + ), + ( + """ + ((fn [] + (def Shape (python/type "Shape" #py () #py {})) + (reify* :implements [Shape])))""", runtime.RuntimeException, ), ], @@ -4548,6 +4638,19 @@ def test_reify_allows_empty_dynamic_abstract_interface(self, lcompile: CompileFn (abc/abstractmethod (fn []))})) (reify* :implements [Shape]))""", + compiler.CompilerException, + ), + ( + """ + ((fn [] + (import* abc) + (def Shape + (python/type "Shape" + #py (abc/ABC) + #py {"area" + (abc/abstractmethod + (fn []))})) + (reify* :implements [Shape])))""", runtime.RuntimeException, ), ], @@ -4585,6 +4688,21 @@ def test_reify_interface_must_implement_all_abstract_methods( (reify* :implements [Shape] (area [this] (* 2 1 1)) (call [this] :called)))""", + compiler.CompilerException, + ), + ( + """ + (import* abc collections.abc) + ((fn [] + (def Shape + (python/type "Shape" + #py (abc/ABC) + #py {"area" + (abc/abstractmethod + (fn []))})) + (reify* :implements [Shape] + (area [this] (* 2 1 1)) + (call [this] :called))))""", runtime.RuntimeException, ), ], @@ -4792,6 +4910,16 @@ def test_reify_allows_artificially_abstract_super_type( (reify* :implements [^:abstract AABase] (some-method [this]) (other-method [this])))""", + compiler.CompilerException, + ), + ( + """ + ((fn [] + (def AABase + (python/type "AABase" #py () #py {"some_method" (fn [this])})) + (reify* :implements [^:abstract AABase] + (some-method [this]) + (other-method [this]))))""", runtime.RuntimeException, ), ], @@ -5352,6 +5480,20 @@ def property_interface(self, lcompile: CompileFn): (abc/abstractmethod (fn [self])))})) (reify* :implements [WithProperty]))""", + compiler.CompilerException, + ), + ( + """ + ((fn [] + (import* abc) + (def WithProperty + (python/type "WithProp" + #py (abc/ABC) + #py {"a_property" + (python/property + (abc/abstractmethod + (fn [self])))})) + (reify* :implements [WithProperty])))""", runtime.RuntimeException, ), ], @@ -5990,6 +6132,7 @@ def test_nested_bare_sym_will_not_resolve(self, lcompile: CompileFn): [ "(import* abc) abc", "(do (import* abc) abc)", + "(do (do ((fn [] (import* abc))) abc))", "((fn [] (import* abc) abc))", "((fn [] (import* abc))) abc", """ @@ -6005,6 +6148,12 @@ def test_nested_bare_sym_will_not_resolve(self, lcompile: CompileFn): (--call-- [this] (import* abc))) ((Importer)) abc""", + """ + (import* collections.abc) + (deftype* Importer [] + :implements [collections.abc/Callable] + (--call-- [this] (import* abc) abc/ABC)) + (do (do ((Importer)) abc))""", ], ) def test_imported_module_sym_resolves(self, lcompile: CompileFn, code: str): @@ -6020,6 +6169,8 @@ def test_imported_module_sym_resolves(self, lcompile: CompileFn, code: str): "(do (import* abc) abc/ABC)", "((fn [] (import* abc) abc/ABC))", "((fn [] (import* abc))) abc/ABC", + "(do ((fn [] (import* abc))) abc/ABC)", + "(do (do ((fn [] (import* abc))) abc/ABC))" """ (import* collections.abc) (deftype* Importer [] @@ -6033,6 +6184,18 @@ def test_imported_module_sym_resolves(self, lcompile: CompileFn, code: str): (--call-- [this] (import* abc))) ((Importer)) abc/ABC""", + """ + (import* collections.abc) + (deftype* Importer [] + :implements [collections.abc/Callable] + (--call-- [this] (import* abc) abc/ABC)) + (do ((Importer)) abc/ABC)""", + """ + (import* collections.abc) + (deftype* Importer [] + :implements [collections.abc/Callable] + (--call-- [this] (import* abc) abc/ABC)) + (do (do ((Importer)) abc/ABC))""", ], ) def test_sym_from_import_resolves(self, lcompile: CompileFn, code: str): @@ -6041,33 +6204,31 @@ def test_sym_from_import_resolves(self, lcompile: CompileFn, code: str): imported_ABC = lcompile(code) assert imported_ABC is ABC + @pytest.mark.parametrize( + "code", + [ + "(do ((fn [] (import* abc))) abc)", + """ + (import* collections.abc) + (deftype* Importer [] + :implements [collections.abc/Callable] + (--call-- [this] (import* abc) abc/ABC)) + (do ((Importer)) abc)""", + ], + ) + def test_module_from_import_resolves(self, lcompile: CompileFn, code: str): + import abc + + imported_abc = lcompile(code) + assert imported_abc is abc + @pytest.mark.parametrize( "code,ExceptionType", [ - ("(do ((fn [] (import* abc))) abc)", compiler.CompilerException), ("(if false (import* abc) nil) abc", NameError), ("(do (if false (import* abc) nil) abc)", NameError), - ("(do ((fn [] (import* abc))) abc/ABC)", compiler.CompilerException), ("(if false (import* abc) nil) abc/ABC", NameError), ("(do (if false (import* abc) nil) abc/ABC)", NameError), - ( - """ - (import* collections.abc) - (deftype* Importer [] - :implements [collections.abc/Callable] - (--call-- [this] (import* abc) abc/ABC)) - (do ((Importer)) abc)""", - compiler.CompilerException, - ), - ( - """ - (import* collections.abc) - (deftype* Importer [] - :implements [collections.abc/Callable] - (--call-- [this] (import* abc) abc/ABC)) - (do ((Importer)) abc/ABC)""", - compiler.CompilerException, - ), ], ) def test_unresolvable_imported_symbols( @@ -6078,9 +6239,6 @@ def test_unresolvable_imported_symbols( # applicable, so I'm not going to spend time making them work right now. # If an important use case arises for more complex import resolution, # then we can think about reworking the resolver. - # - # Perhaps if we can eventually unroll top-level `do` forms into individiual - # nodes, the cases not involving branching above can be resolved. with pytest.raises(ExceptionType): lcompile(code)