Clean it up

src/basilisp/lang/compiler/generator.py

@@ -659,9 +659,9 @@ def _def_to_py_ast(  # pylint: disable=too-many-branches
     defsym = node.name
     is_defn = False
     is_var_bound = node.var.is_bound
-    is_noop_redef_of_bound_var = is_var_bound and node.init is None
+    is_binding_def = node.init is not None
 
-    if node.init is not None:
+    if is_binding_def:
         # Since Python function definitions always take the form `def name(...):`,
         # it is redundant to assign them to the their final name after they have
         # been defined under a private alias. This codepath generates `defn`
@@ -683,10 +683,8 @@ def _def_to_py_ast(  # pylint: disable=too-many-branches
             is_defn = True
         else:
             def_ast = gen_py_ast(ctx, node.init)
-    elif is_noop_redef_of_bound_var:
-        def_ast = None
     else:
-        def_ast = GeneratedPyAST(node=ast.Constant(None))
+        def_ast = None
 
     ns_name = _load_attr(_NS_VAR_VALUE)
     def_name = ast.Call(
@@ -717,67 +715,47 @@ def _def_to_py_ast(  # pylint: disable=too-many-branches
 
     meta_ast = gen_py_ast(ctx, node.meta)
 
-    # For defn style def generation, we specifically need to generate the
-    # global declaration prior to emitting the Python `def` otherwise the
-    # Python compiler will throw an exception during compilation
-    # complaining that we assign the value prior to global declaration.
-    should_emit_global_decl = bool(node.top_level and not node.in_func_ctx)
-    if is_defn:
+    if is_binding_def:
         assert def_ast is not None, "def_ast must be defined at this point"
-        def_dependencies = list(
-            chain(
-                [ast.Global(names=[safe_name])] if should_emit_global_decl else [],
-                def_ast.dependencies,
-                [] if meta_ast is None else meta_ast.dependencies,
+        func = _INTERN_VAR_FN_NAME
+        extra_args = [ast.Name(id=safe_name, ctx=ast.Load())]
+        if is_defn:
+            # For defn style def generation, we specifically need to generate
+            # the global declaration prior to emitting the Python `def` otherwise
+            # the Python compiler will throw an exception during compilation
+            # complaining that we assign the value prior to global declaration.
+            def_dependencies = list(
+                chain(
+                    [ast.Global(names=[safe_name])] if node.in_func_ctx else [],
+                    def_ast.dependencies,
+                )
             )
-        )
-    elif is_noop_redef_of_bound_var:
-        # Re-def-ing previously bound Vars without providing a value is
-        # essentially a no-op, which should not modify the Var root.
-        assert def_ast is None, "def_ast is not defined at this point"
-        def_dependencies = list(
-            chain(
-                [ast.Global(names=[safe_name])] if should_emit_global_decl else [],
-                [] if meta_ast is None else meta_ast.dependencies,
+        else:
+            def_dependencies = list(
+                chain(
+                    def_ast.dependencies,
+                    [ast.Global(names=[safe_name])] if node.in_func_ctx else [],
+                    [
+                        ast.Assign(
+                            targets=[ast.Name(id=safe_name, ctx=ast.Store())],
+                            value=def_ast.node,
+                        )
+                    ],
+                )
             )
-        )
     else:
-        assert def_ast is not None, "def_ast must be defined at this point"
-        def_dependencies = list(
-            chain(
-                def_ast.dependencies,
-                [ast.Global(names=[safe_name])] if should_emit_global_decl else [],
-                [
-                    ast.Assign(
-                        targets=[ast.Name(id=safe_name, ctx=ast.Store())],
-                        value=def_ast.node,
-                    )
-                ],
-                [] if meta_ast is None else meta_ast.dependencies,
-            )
-        )
-
-    if is_noop_redef_of_bound_var:
-        return GeneratedPyAST(
-            node=ast.Call(
-                func=_INTERN_UNBOUND_VAR_FN_NAME,
-                args=[ns_name, def_name],
-                keywords=list(
-                    chain(
-                        dynamic_kwarg,
-                        []
-                        if meta_ast is None
-                        else [ast.keyword(arg="meta", value=meta_ast.node)],
-                    )
-                ),
-            ),
-            dependencies=def_dependencies,
-        )
+        assert def_ast is None, "def_ast is not defined at this point"
+        assert not is_defn, "defn defs must be binding defs"
+        # Re-def-ing previously bound Vars without providing a value is
+        # essentially a no-op, which should not modify the Var root.
+        func = _INTERN_UNBOUND_VAR_FN_NAME
+        extra_args = []
+        def_dependencies = [ast.Global(names=[safe_name])] if node.in_func_ctx else []
 
     return GeneratedPyAST(
         node=ast.Call(
-            func=_INTERN_VAR_FN_NAME,
-            args=[ns_name, def_name, ast.Name(id=safe_name, ctx=ast.Load())],
+            func=func,
+            args=list(chain([ns_name, def_name], extra_args)),
             keywords=list(
                 chain(
                     dynamic_kwarg,
@@ -787,7 +765,9 @@ def _def_to_py_ast(  # pylint: disable=too-many-branches
                 )
             ),
         ),
-        dependencies=def_dependencies,
+        dependencies=list(
+            chain(def_dependencies, [] if meta_ast is None else meta_ast.dependencies,)
+        ),
     )
 
 

src/basilisp/lang/compiler/optimizer.py

@@ -1,4 +1,6 @@
-from typing import Iterable, List, Optional
+from collections import deque
+from contextlib import contextmanager
+from typing import Iterable, List, Optional, Set
 
 import basilisp._pyast as ast
 
@@ -17,6 +19,23 @@ def _filter_dead_code(nodes: Iterable[ast.AST]) -> List[ast.AST]:
 
 
 class PythonASTOptimizer(ast.NodeTransformer):
+    __slots__ = ("_global_ctx",)
+
+    def __init__(self):
+        self._global_ctx = deque([set()])
+
+    @contextmanager
+    def _new_global_context(self):
+        self._global_ctx.append(set())
+        try:
+            yield
+        finally:
+            self._global_ctx.pop()
+
+    @property
+    def _global_context(self) -> Set[str]:
+        return self._global_ctx[-1]
+
     def visit_ExceptHandler(self, node: ast.ExceptHandler) -> Optional[ast.AST]:
         """Eliminate dead code from except handler bodies."""
         new_node = self.generic_visit(node)
@@ -39,7 +58,8 @@ def visit_Expr(self, node: ast.Expr) -> Optional[ast.Expr]:
 
     def visit_FunctionDef(self, node: ast.FunctionDef) -> Optional[ast.AST]:
         """Eliminate dead code from function bodies."""
-        new_node = self.generic_visit(node)
+        with self._new_global_context():
+            new_node = self.generic_visit(node)
         assert isinstance(new_node, ast.FunctionDef)
         return ast.copy_location(
             ast.FunctionDef(
@@ -52,6 +72,16 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> Optional[ast.AST]:
             new_node,
         )
 
+    def visit_Global(self, node: ast.Global) -> Optional[ast.Global]:
+        """"""
+        new_names = set(node.names) - self._global_context
+        self._global_context.update(new_names)
+        return (
+            ast.copy_location(ast.Global(names=list(new_names)), node)
+            if new_names
+            else None
+        )
+
     def visit_If(self, node: ast.If) -> Optional[ast.AST]:
         """Eliminate dead code from if/elif bodies.
 

tests/basilisp/compiler_test.py

@@ -285,8 +285,7 @@ def test_def_unbound(self, lcompile: CompileFn, ns: runtime.Namespace):
         lcompile("(def a)")
         var = Var.find_in_ns(sym.symbol(ns.name), sym.symbol("a"))
         assert var.root is None
-        # TODO: fix this
-        # assert not var.is_bound
+        assert not var.is_bound
 
     def test_def_number_of_elems(self, lcompile: CompileFn, ns: runtime.Namespace):
         with pytest.raises(compiler.CompilerException):