Skip to content

Fix exit types of variables assigned inside while conditions #10759

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.

Sign up for GitHub

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

Merged
asterite merged 2 commits into from
Jun 8, 2021
Merged

Fix exit types of variables assigned inside while conditions #10759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
514cbc9
Fix types of variables assigned inside while conditions
HertzDevil May 28, 2021
29e365d
fixup
HertzDevil May 28, 2021
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
20 changes: 20 additions & 0 deletions spec/compiler/semantic/ssa_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,26 @@ describe "Semantic: ssa" do
") { int32 }
end

it "types a var that is declared in a while condition with break before re-assignment" do
assert_type(%(
while a = 'a'
break if 1 == 1
a = "hello"
end
a
)) { char }
end

it "types a var that is declared in a while condition with break after re-assignment" do
assert_type(%(
while a = 'a'
a = "hello"
break if 1 == 1
end
a
)) { union_of(char, string) }
end

it "types while with next" do
assert_type("
a = 1
Expand Down
64 changes: 64 additions & 0 deletions spec/compiler/semantic/while_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,70 @@ describe "Semantic: while" do
)) { int32 }
end

it "finds all while cond assign targets in expressions (#10350)" do
assert_type(%(
a = 1
while ((b = 1); a)
a = nil
b = "hello"
end
b
)) { int32 }
end

it "finds all while cond assign targets in expressions (2)" do
assert_type(%(
def foo(x, y)
true ? 1 : nil
end

while foo(a = 1, b = 1)
a = nil
b = "hello"
end

{a, b}
)) { tuple_of [int32, int32] }
end

it "finds all while cond assign targets in expressions (3)" do
assert_type(%(
while 1 == 1 ? (x = 1; 1 == 1) : false
x = 'a'
end
x
)) { nilable union_of(int32, char) }
end

it "finds all while cond assign targets in expressions (4)" do
assert_type(%(
x = ""
while 1 == 1 ? (x = 1; 1 == 1) : false
x = 'a'
end
x
)) { union_of(int32, char, string) }
end

it "finds all while cond assign targets in expressions (5)" do
assert_type(%(
while 1 == 1 ? (x = 1; 1 == 1) : false
x
x = 'a'
end
x
)) { nilable union_of(int32, char) }
end

it "finds all while cond assign targets in expressions (6)" do
assert_type(%(
while (x = true ? (y = 1) : 1; y = x; 1 == 1)
x = 'a'
end
{x, y}
)) { tuple_of [int32, int32] }
end

it "doesn't fail on new variables inside typeof condition" do
assert_type(%(
def foo
Expand Down
216 changes: 93 additions & 123 deletions src/compiler/crystal/semantic/main_visitor.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2077,6 +2077,13 @@ module Crystal

filter_vars cond_type_filters

# `node.body` may reset this status, so we capture them in a set
# (we don't need the full MetaVars at the moment)
after_cond_vars_nil_if_read = Set(String).new
@vars.each do |name, var|
after_cond_vars_nil_if_read << name if var.nil_if_read?
end

@type_filters = nil
@block, old_block = nil, @block

Expand All @@ -2086,26 +2093,9 @@ module Crystal
node.body.accept self
end

# After while's body, bind variables *before* the condition to the
# ones after the body, because the loop will repeat.
#
# For example:
#
# x = exp
# # x starts with the type of exp
# while x = x.method
# # but after the loop, the x above (in x.method)
# # should now also get the type of x.method, recursively
# end
before_cond_vars.each do |name, before_cond_var|
var = @vars[name]?
before_cond_var.bind_to(var) if var && !var.same?(before_cond_var)
end

cond = node.cond.single_expression

endless_while = cond.true_literal?
merge_while_vars cond, endless_while, before_cond_vars_copy, before_cond_vars, after_cond_vars, @vars, node.break_vars
merge_while_vars endless_while, before_cond_vars_copy, before_cond_vars, after_cond_vars, after_cond_vars_nil_if_read, node.break_vars

@while_stack.pop
@block = old_block
Expand All @@ -2126,138 +2116,118 @@ module Crystal
end

# Here we assign the types of variables after a while.
def merge_while_vars(cond, endless, before_cond_vars_copy, before_cond_vars, after_cond_vars, while_vars, all_break_vars)
def merge_while_vars(endless, before_cond_vars_copy, before_cond_vars, after_cond_vars, after_cond_vars_nil_if_read, all_break_vars)
after_while_vars = MetaVars.new

cond_var = get_while_cond_assign_target(cond)

while_vars.each do |name, while_var|
@vars.each do |name, while_var|
before_cond_var = before_cond_vars[name]?
after_cond_var = after_cond_vars[name]?
after_while_vars[name] = after_while_var = MetaVar.new(name)

# If a variable was assigned in the condition, it has that type.
if cond_var && (cond_var.name == name) && after_cond_var && !after_cond_var.same?(before_cond_var)
after_while_var = MetaVar.new(name)
after_while_var.bind_to(after_cond_var)
after_while_var.nil_if_read = after_cond_var.nil_if_read?
after_while_vars[name] = after_while_var

# If there was a previous variable, we use that type merged
# with the last type inside the while.
elsif before_cond_var
after_while_var = MetaVar.new(name)

# If the loop is endless
if endless
# Suppose we have
#
# x = exp1
# while true
# x = exp2
# break if ...
# x = exp3
# break if ...
# x = exp4
# end
#
# Here the type of x after the loop will never be affected by
# `x = exp4`, because `x = exp2` must have been executed before the
# loop may exit at the first break. Therefore, if the x right before
# the first break is different from the last x, we don't use the
# latter's type upon exit (but exp2 itself may depend on exp4 if it
# refers to x).
break_var = all_break_vars.try &.dig?(0, name)
unless break_var && !break_var.same?(while_var)
after_while_var.bind_to(while_var)
after_while_var.nil_if_read = while_var.nil_if_read?
end
else
# We need to bind to the variable *before* the condition, even
# after before the variables that are used in the condition
# `before_cond_vars` are modified in the while body
after_while_var.bind_to(before_cond_vars_copy[name])
after_while_var.bind_to(while_var)
after_while_var.nil_if_read = before_cond_var.nil_if_read? || while_var.nil_if_read?
end
after_while_vars[name] = after_while_var

# We must also bind the variable before the condition, because
# its type now must also include the type at the exit of the while
# After while's body, bind variables *before* the condition to the
# ones after the body, because the loop will repeat.
#
# For example:
#
# x = exp
# # x starts with the type of exp
# while x = x.method
# # but after the loop, the x above (in x.method)
# # should now also get the type of x.method, recursively
# end
if before_cond_var && !before_cond_var.same?(while_var)
before_cond_var.bind_to(while_var)
end

# Otherwise, it's a new variable inside the while: used
# outside it must be nilable, unless the loop is endless.
else
after_while_var = MetaVar.new(name)

if endless
break_var = all_break_vars.try &.dig?(0, name)
unless break_var && !break_var.same?(while_var)
after_while_var.bind_to(while_var)
# If the loop is endless
if endless
# Suppose we have
#
# x = exp1
# while true
# x = exp2
# break if ...
# x = exp3
# break if ...
# x = exp4
# end
#
# Here the type of x after the loop will never be affected by
# `x = exp4`, because `x = exp2` must have been executed before the
# loop may exit at the first break. Therefore, if the x right before
# the first break is different from the last x, we don't use the
# latter's type upon exit (but exp2 itself may depend on exp4 if it
# refers to x).
break_var = all_break_vars.try &.dig?(0, name)
unless break_var && !break_var.same?(while_var)
after_while_var.bind_to(while_var)
if before_cond_var
after_while_var.nil_if_read = while_var.nil_if_read?
end
end

# In an endless loop if not all variable with the given name end up
# in a break it means that they can be nilable.
# If there wasn't a previous variable with the same name, the variable
# is newly defined inside the while.
unless before_cond_var
# If not all variables with the given name end up in a break it
# means that they can be nilable.
# Alternatively, if any var that ends in a break is nil-if-read then
# the resulting variable will be nil-if-read too.
if !all_break_vars.try(&.all? &.has_key?(name)) ||
all_break_vars.try(&.any? &.[name]?.try &.nil_if_read?)
after_while_var.nil_if_read = true
end
end
else
# If a variable was assigned in the condition, it has that type.
if after_cond_var && !after_cond_var.same?(before_cond_var)
after_while_var.bind_to(after_cond_var)

# If the variable after the condition is nil-if-read, that means the
# assignment inside the condition might not run upon loop exit, so
# the variable may receive the type inside the loop.
if after_cond_vars_nil_if_read.includes?(name)
after_while_var.nil_if_read = true
after_while_var.bind_to(while_var) if !after_cond_var.same?(while_var)
end

# If there was a previous variable, we use that type merged
# with the last type inside the while.
elsif before_cond_var
# We need to bind to the variable *before* the condition, even
# before the variables that are used in the condition
# `before_cond_vars` are modified in the while body
after_while_var.bind_to(before_cond_vars_copy[name])
after_while_var.bind_to(while_var)
after_while_var.nil_if_read = before_cond_var.nil_if_read? || while_var.nil_if_read?

# Otherwise, it's a new variable inside the while: used
# outside it must be nilable.
else
after_while_var.bind_to(while_var)
after_while_var.nil_if_read = true
end

after_while_vars[name] = after_while_var
end
end

@vars = after_while_vars

# We also need to merge types from breaks inside while.
if all_break_vars
all_break_vars.each do |break_vars|
break_vars.each do |name, break_var|
var = @vars[name]?
unless var
# Fix for issue #2441:
# it might be that a break variable is not present
# in the current vars after a while
var = new_meta_var(name)
var.bind_to(program.nil_var)
@meta_vars[name].bind_to(program.nil_var)
@vars[name] = var
end
var.bind_to(break_var)
all_break_vars.try &.each do |break_vars|
break_vars.each do |name, break_var|
after_while_var = after_while_vars[name]?
unless after_while_var
# Fix for issue #2441:
# it might be that a break variable is not present
# in the current vars after a while
after_while_var = new_meta_var(name)
after_while_var.bind_to(program.nil_var)
@meta_vars[name].bind_to(program.nil_var)
after_while_vars[name] = after_while_var
end
after_while_var.bind_to(break_var)
end
end
end

def get_while_cond_assign_target(node)
case node
when Assign
target = node.target
if target.is_a?(Var)
return target
end
when And
return get_while_cond_assign_target(node.left)
when Not
return get_while_cond_assign_target(node.exp)
when If
if node.and?
return get_while_cond_assign_target(node.cond)
end
when Call
return get_while_cond_assign_target(node.obj)
when Expressions
return unless node = node.single_expression?
return get_while_cond_assign_target(node)
end

nil
@vars = after_while_vars
end

# If we have:
Expand Down