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

A setter type is the argument's type #7711

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 136 additions & 0 deletions spec/compiler/codegen/def_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -568,4 +568,140 @@ describe "Code gen: def" do
foo { |a, b| }
))
end

it "codegens setter (non-inlineable body, non-struct)" do
run(%(
class Foo
def x=(y)
'a'
'b'
end
end

foo = Foo.new
z = foo.x = 123
z
)).to_i.should eq(123)
end

it "codegens []= setter (non-inlineable body, non-struct)" do
run(%(
class Foo
def []=(x, y)
'a'
'b'
end
end

foo = Foo.new
z = foo["x"] = 123
z
)).to_i.should eq(123)
end

it "codegens setter (non-inlineable body, struct)" do
run(%(
class Foo
def x=(y)
'a'
'b'
end
end

struct Bar
@x = 123
def x; @x; end
end

foo = Foo.new
z = foo.x = Bar.new
z.x
)).to_i.should eq(123)
end

it "codegens []= setter (non-inlineable body, struct)" do
run(%(
class Foo
def []=(x, y)
'a'
'b'
end
end

struct Bar
@x = 123
def x; @x; end
end

foo = Foo.new
z = foo["x"] = Bar.new
z.x
)).to_i.should eq(123)
end

it "codegens setter (inlineable body, non-struct)" do
run(%(
class Foo
def x=(y)
'a'
end
end

foo = Foo.new
z = foo.x = 123
z
)).to_i.should eq(123)
end

it "codegens []= setter (inlineable body, non-struct)" do
run(%(
class Foo
def []=(x, y)
'a'
end
end

foo = Foo.new
z = foo["x"] = 123
z
)).to_i.should eq(123)
end

it "codegens setter (inlineable body, struct)" do
run(%(
class Foo
def x=(y)
'a'
end
end

struct Bar
@x = 123
def x; @x; end
end

foo = Foo.new
z = foo.x = Bar.new
z.x
)).to_i.should eq(123)
end

it "codegens []= setter (inlineable body, struct)" do
run(%(
class Foo
def []=(x, y)
'a'
end
end

struct Bar
@x = 123
def x; @x; end
end

foo = Foo.new
z = foo["x"] = Bar.new
z.x
)).to_i.should eq(123)
end
end
2 changes: 1 addition & 1 deletion spec/compiler/codegen/proc_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -882,7 +882,7 @@ describe "Code gen: proc" do
)).to_i.should eq(1)
end

it "doesn't crash when taking a proc pointer to a virtual type (#9823)" do
pending "doesn't crash when taking a proc pointer to a virtual type (#9823)" do
run(%(
abstract struct Parent
abstract def work(a : Int32, b : Int32)
Expand Down
28 changes: 28 additions & 0 deletions spec/compiler/semantic/def_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -498,4 +498,32 @@ describe "Semantic: def" do
ex.column_number.should eq(3)
ex.size.should eq(6)
end

it "types setter" do
assert_type(%(
class Foo
def x=(y)
'a'
end
end

foo = Foo.new
z = foo.x = 1
z
)) { int32 }
end

it "types []= setter" do
assert_type(%(
class Foo
def []=(x, y)
'a'
end
end

foo = Foo.new
z = foo[1] = 1
z
)) { int32 }
end
end
42 changes: 28 additions & 14 deletions src/compiler/crystal/codegen/call.cr
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ class Crystal::CodeGenVisitor
body = target_def.body

# Try to inline the call
if try_inline_call(target_def, body, self_type, call_args)
if try_inline_call(node, target_def, body, self_type, call_args)
return
end

Expand All @@ -454,22 +454,22 @@ class Crystal::CodeGenVisitor
# to read a constant value or the value of an instance variable.
# Additionally, not inlining instance variable getters changes the semantic
# a program, so we must always inline these.
def try_inline_call(target_def, body, self_type, call_args)
def try_inline_call(node, target_def, body, self_type, call_args)
return false if target_def.is_a?(External)

case body
when Nop, NilLiteral, BoolLiteral, CharLiteral, StringLiteral, NumberLiteral, SymbolLiteral
return true unless @needs_value

accept body
inline_call_return_value target_def, body
inline_call_return_value node, target_def, body, self_type, call_args
true
when Var
if body.name == "self"
return true unless @needs_value

@last = self_type.passed_as_self? ? call_args.first : type_id(self_type)
inline_call_return_value target_def, body
inline_call_return_value node, target_def, body, self_type, call_args
return true
else
false
Expand All @@ -478,15 +478,22 @@ class Crystal::CodeGenVisitor
return true unless @needs_value

read_instance_var(body.type, self_type, body.name, call_args.first)
inline_call_return_value target_def, body
inline_call_return_value node, target_def, body, self_type, call_args
true
else
false
end
end

def inline_call_return_value(target_def, body)
if target_def.type.nil_type?
def inline_call_return_value(node, target_def, body, self_type, call_args)
if node.setter?
# For a setter value (`foo.x = y`) the last value is `y`, which is also
# the last codegen argument
@last = call_args.last

assigned_arg_type = node.args.last.type
@last = box_value(@last, assigned_arg_type) if assigned_arg_type.passed_by_value?
elsif target_def.type.nil_type?
@last = llvm_nil
else
@last = upcast(@last, target_def.type, body.type)
Expand Down Expand Up @@ -548,23 +555,30 @@ class Crystal::CodeGenVisitor
end
end
else
# For a setter value (`foo.x = y`) the last value is `y`, which is also
# the last codegen argument
if node.is_a?(Call) && node.setter?
@last = call_args.last
type = node.args.last.type
end

case type
when .no_return?
unreachable
when .passed_by_value?
if @needs_value
union = alloca llvm_type(type)
store @last, union
@last = union
else
@last = llvm_nil
end
@last = @needs_value ? box_value(@last, type) : llvm_nil
end
end

@last
end

def box_value(value, type)
union = alloca llvm_type(type)
store value, union
union
end

def set_call_attributes(node : Call, target_def, self_type, is_closure, fun_type)
if external = target_def.c_calling_convention?
set_call_attributes_external(node, external)
Expand Down
11 changes: 9 additions & 2 deletions src/compiler/crystal/semantic/call.cr
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,15 @@ class Crystal::Call

@target_defs = matches

bind_to matches if matches
bind_to block.break if block
# For a setter like `foo.x = y` we'd like to use `y`'s type
if self.setter?
# uses args.last instead of args.first because for []= setters, the assigned
# value is the second (and last) arg. Other setters have only one arg.
bind_to args.last
else
bind_to matches if matches
bind_to block.break if block
end

if (parent_visitor = @parent_visitor) && matches
if parent_visitor.typed_def? && matches.any?(&.raises?)
Expand Down
14 changes: 14 additions & 0 deletions src/compiler/crystal/syntax/ast.cr
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,20 @@ module Crystal
Location.new(loc.filename, loc.line_number, loc.column_number + name_size)
end

def setter?
(
(
args.size == 1 &&
name.ends_with?('=') &&
!(33 <= name.byte_at(0) <= 64) # first letter must not be a symbol like `=` or `!`
) ||
(
name == "[]=" &&
args.size == 2
)
) && !named_args && !block && !block_arg
end

def_equals_and_hash obj, name, args, block, block_arg, named_args, global?
end

Expand Down