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 1 commit
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
68 changes: 68 additions & 0 deletions spec/compiler/codegen/def_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -568,4 +568,72 @@ 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, 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, 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
14 changes: 14 additions & 0 deletions spec/compiler/semantic/def_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -486,4 +486,18 @@ describe "Semantic: def" do
foo
), "there's no self in this scope"
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
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 @@ -393,7 +393,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 @@ -419,37 +419,44 @@ 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
return 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
end
when InstanceVar
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
return true
end

false
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, depending
# on whether `foo` is passed as self or not, is the first or second codegen argument
@last = call_args[self_type.try(&.passed_as_self?) ? 1 : 0]

first_arg_type = node.args.first.type
@last = box_value(@last, first_arg_type) if first_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 @@ -510,23 +517,30 @@ class Crystal::CodeGenVisitor
end
end
else
# For a setter value (`foo.x = y`) the last value is `y`, which, depending
# on whether `foo` is passed as self or not, is the first or second codegen argument
if node.is_a?(Call) && node.setter?
@last = call_args[self_type.try(&.passed_as_self?) ? 1 : 0]
type = node.args.first.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
9 changes: 7 additions & 2 deletions src/compiler/crystal/semantic/call.cr
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,13 @@ 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?
bind_to args.first
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for later: it should probably be bind_to args.last instead to handle []=

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
8 changes: 8 additions & 0 deletions src/compiler/crystal/syntax/ast.cr
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,7 @@ module Crystal
property? global : Bool
property? expansion = false
property? has_parentheses : Bool
property? setter = false

def initialize(@obj, @name, @args = [] of ASTNode, @block = nil, @block_arg = nil, @named_args = nil, global = false, @name_column_number = 0, has_parentheses = false)
@global = !!global
Expand Down Expand Up @@ -589,6 +590,7 @@ module Crystal
clone = Call.new(@obj.clone, @name, @args.clone, @block.clone, @block_arg.clone, @named_args.clone, @global, @name_column_number, @has_parentheses)
clone.name_size = name_size
clone.expansion = expansion?
clone.setter = setter?
clone
end

Expand All @@ -602,6 +604,12 @@ module Crystal
Location.new(loc.filename, loc.line_number, name_column_number + name_size)
end

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

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

Expand Down