Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't incorrectly consider local as closured #4986

Merged
merged 1 commit into from Sep 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions spec/compiler/codegen/closure_spec.cr
Expand Up @@ -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
15 changes: 11 additions & 4 deletions src/compiler/crystal/codegen/fun.cr
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down