LocalAccessValidator: relax restrictions to accept def f(); def g().

artiq/compiler/module.py

@@ -60,7 +60,7 @@ class Module:
         escape_validator.visit(src.typedtree)
         self.artiq_ir = artiq_ir_generator.visit(src.typedtree)
         dead_code_eliminator.process(self.artiq_ir)
-        # local_access_validator.process(self.artiq_ir)
+        local_access_validator.process(self.artiq_ir)
 
     def build_llvm_ir(self, target):
         """Compile the module to LLVM IR for the specified target."""

artiq/compiler/transforms/dead_code_eliminator.py

@@ -14,10 +14,11 @@ class DeadCodeEliminator:
             self.process_function(func)
 
     def process_function(self, func):
-        for block in func.basic_blocks:
-            if not any(block.predecessors()) and \
-                    not any([isinstance(use, ir.SetLocal) for use in block.uses]) and \
-                    block != func.entry():
+        for block in list(func.basic_blocks):
+            if not any(block.predecessors()) and block != func.entry():
+                for use in set(block.uses):
+                    if isinstance(use, ir.SetLocal):
+                        use.erase()
                 self.remove_block(block)
 
     def remove_block(self, block):

artiq/compiler/validators/local_access.py

@@ -16,11 +16,13 @@ class LocalAccessValidator:
             self.process_function(func)
 
     def process_function(self, func):
-        # Find all environments allocated in this func.
-        environments = []
+        # Find all environments and closures allocated in this func.
+        environments, closures = [], []
         for insn in func.instructions():
             if isinstance(insn, ir.Alloc) and ir.is_environment(insn.type):
                 environments.append(insn)
+            elif isinstance(insn, ir.Closure):
+                closures.append(insn)
 
         # Compute initial state of interesting environments.
         # Environments consisting only of internal variables (containing a ".")
@@ -82,6 +84,7 @@ class LocalAccessValidator:
                     # It's the entry block and it was never initialized.
                     return None
 
+                set_local_in_this_frame = False
                 if isinstance(insn, (ir.SetLocal, ir.GetLocal)) and \
                         "." not in insn.var_name:
                     env, var_name = insn.environment(), insn.var_name
@@ -91,24 +94,41 @@ class LocalAccessValidator:
                         if isinstance(insn, ir.SetLocal):
                             # We've just initialized it.
                             block_state[env][var_name] = True
+                            set_local_in_this_frame = True
                         else: # isinstance(insn, ir.GetLocal)
                             if not block_state[env][var_name]:
                                 # Oops, accessing it uninitialized.
                                 self._uninitialized_access(insn, var_name,
                                                            pred_at_fault(env, var_name))
 
-                # Creating a closure has no side effects. However, using a closure does.
-                for operand in insn.operands:
-                    if isinstance(operand, ir.Closure):
-                        env = operand.environment()
-                        # Make sure this environment has any interesting variables.
-                        if env in block_state:
-                            for var_name in block_state[env]:
-                                if not block_state[env][var_name]:
-                                    # A closure would capture this variable while it is not always
-                                    # initialized. Note that this check is transitive.
-                                    self._uninitialized_access(operand, var_name,
-                                                               pred_at_fault(env, var_name))
+                closures_to_check = []
+
+                if (isinstance(insn, (ir.SetLocal, ir.SetAttr, ir.SetElem)) and
+                        not set_local_in_this_frame):
+                    # Closures may escape via these mechanisms and be invoked elsewhere.
+                    if isinstance(insn.value(), ir.Closure):
+                        closures_to_check.append(insn.value())
+
+                if isinstance(insn, (ir.Call, ir.Invoke)):
+                    # We can't always trace the flow of closures from point of
+                    # definition to point of call; however, we know that, by transitiveness
+                    # of this analysis, only closures defined in this function can contain
+                    # uninitialized variables.
+                    #
+                    # Thus, enumerate the closures, and check all of them during any operation
+                    # that may eventually result in the closure being called.
+                    closures_to_check = closures
+
+                for closure in closures_to_check:
+                    env = closure.environment()
+                    # Make sure this environment has any interesting variables.
+                    if env in block_state:
+                        for var_name in block_state[env]:
+                            if not block_state[env][var_name]:
+                                # A closure would capture this variable while it is not always
+                                # initialized. Note that this check is transitive.
+                                self._uninitialized_access(closure, var_name,
+                                                           pred_at_fault(env, var_name))
 
             # Save the state.
             state[block] = block_state

lit-test/test/integration/finally.py

@@ -13,11 +13,6 @@ def f():
             print("f-finally")
     print("f-out")
 
-# CHECK-L: f-try
-# CHECK-L: f-finally
-# CHECK-L: f-out
-f()
-
 def g():
     x = True
     while x:
@@ -29,11 +24,6 @@ def g():
             print("g-finally")
     print("g-out")
 
-# CHECK-L: g-try
-# CHECK-L: g-finally
-# CHECK-L: g-out
-g()
-
 def h():
     try:
         print("h-try")
@@ -43,12 +33,6 @@ def h():
     print("h-out")
     return 20
 
-# CHECK-L: h-try
-# CHECK-L: h-finally
-# CHECK-NOT-L: h-out
-# CHECK-L: h 10
-print("h", h())
-
 def i():
     try:
         print("i-try")
@@ -59,12 +43,6 @@ def i():
     print("i-out")
     return 20
 
-# CHECK-L: i-try
-# CHECK-L: i-finally
-# CHECK-NOT-L: i-out
-# CHECK-L: i 30
-print("i", i())
-
 def j():
     try:
         print("j-try")
@@ -72,6 +50,28 @@ def j():
         print("j-finally")
     print("j-out")
 
+# CHECK-L: f-try
+# CHECK-L: f-finally
+# CHECK-L: f-out
+f()
+
+# CHECK-L: g-try
+# CHECK-L: g-finally
+# CHECK-L: g-out
+g()
+
+# CHECK-L: h-try
+# CHECK-L: h-finally
+# CHECK-NOT-L: h-out
+# CHECK-L: h 10
+print("h", h())
+
+# CHECK-L: i-try
+# CHECK-L: i-finally
+# CHECK-NOT-L: i-out
+# CHECK-L: i 30
+print("i", i())
+
 # CHECK-L: j-try
 # CHECK-L: j-finally
 # CHECK-L: j-out

lit-test/test/local_access/invalid_closure.py

@@ -0,0 +1,15 @@
+# RUN: %python -m artiq.compiler.testbench.signature +diag %s >%t
+# RUN: OutputCheck %s --file-to-check=%t
+
+if False:
+    t = 1
+
+# CHECK-L: ${LINE:+1}: error: variable 't' can be captured in a closure uninitialized
+l = lambda: t
+
+# CHECK-L: ${LINE:+1}: error: variable 't' can be captured in a closure uninitialized
+def f():
+    return t
+
+l()
+f()

lit-test/test/local_access/invalid_flow.py

@@ -18,10 +18,3 @@ else:
     t = 1
 # CHECK-L: ${LINE:+1}: error: variable 't' is not always initialized
 -t
-
-# CHECK-L: ${LINE:+1}: error: variable 't' can be captured in a closure uninitialized
-l = lambda: t
-
-# CHECK-L: ${LINE:+1}: error: variable 't' can be captured in a closure uninitialized
-def f():
-    return t