Codegen: don't incorrectly consider local as closured. Fixes #4948

crystal-lang · Sep 16, 2017 · 49f9d28 · 49f9d28
1 parent eb1f102
commit 49f9d28
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 4 deletions.
diff --git a/spec/compiler/codegen/closure_spec.cr b/spec/compiler/codegen/closure_spec.cr
@@ -661,4 +661,24 @@ describe "Code gen: closure" do
       foo2.call.x
       )).to_i.should eq(42)
   end
+
+  it "doesn't incorrectly consider local as closured (#4948)" do
+    codegen(%(
+      arg = 1
+
+      f1 = ->{
+        # Here 'local' isn't to be confused with
+        # the outer closured 'local'
+        local = 1
+        local + arg
+      }
+
+      arg = 2
+
+      local = 4_i64
+      f2 = ->{ local.to_i }
+
+      f1.call + f2.call
+    ))
+  end
 end
diff --git a/src/compiler/crystal/codegen/fun.cr b/src/compiler/crystal/codegen/fun.cr
@@ -94,7 +94,7 @@ class Crystal::CodeGenVisitor
 
         if is_closure
           clear_current_debug_location if @debug.line_numbers?
-          setup_closure_vars context.closure_vars.not_nil!
+          setup_closure_vars target_def.vars, context.closure_vars.not_nil!
         else
           context.reset_closure
         end
@@ -364,19 +364,26 @@ class Crystal::CodeGenVisitor
     end
   end
 
-  def setup_closure_vars(closure_vars, context = self.context, closure_ptr = fun_literal_closure_ptr)
+  def setup_closure_vars(def_vars, closure_vars, context = self.context, closure_ptr = fun_literal_closure_ptr)
     if context.closure_skip_parent
       parent_context = context.closure_parent_context.not_nil!
-      setup_closure_vars(parent_context.closure_vars.not_nil!, parent_context, closure_ptr)
+      setup_closure_vars(def_vars, parent_context.closure_vars.not_nil!, parent_context, closure_ptr)
     else
       closure_vars.each_with_index do |var, i|
+        # A closured var in this context might have the same name as
+        # a local var in another context, for example if the local var
+        # was defined before the closured var. In this case, don't
+        # consider the local var as closured.
+        def_var = def_vars.try &.[var.name]?
+        next if def_var && !def_var.closured?
+
         self.context.vars[var.name] = LLVMVar.new(gep(closure_ptr, 0, i, var.name), var.type)
       end
 
       if (closure_parent_context = context.closure_parent_context) &&
          (parent_vars = closure_parent_context.closure_vars)
         parent_closure_ptr = gep(closure_ptr, 0, closure_vars.size, "parent_ptr")
-        setup_closure_vars(parent_vars, closure_parent_context, load(parent_closure_ptr, "parent"))
+        setup_closure_vars(def_vars, parent_vars, closure_parent_context, load(parent_closure_ptr, "parent"))
       elsif closure_self = context.closure_self
         offset = context.closure_parent_context ? 1 : 0
         self_value = gep(closure_ptr, 0, closure_vars.size + offset, "self")