diff --git a/spec/compiler/semantic/ssa_spec.cr b/spec/compiler/semantic/ssa_spec.cr index ef59d4c3b327..b9cd7451cd5b 100644 --- a/spec/compiler/semantic/ssa_spec.cr +++ b/spec/compiler/semantic/ssa_spec.cr @@ -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 diff --git a/spec/compiler/semantic/while_spec.cr b/spec/compiler/semantic/while_spec.cr index 68842527e829..32ba834bf2e7 100644 --- a/spec/compiler/semantic/while_spec.cr +++ b/spec/compiler/semantic/while_spec.cr @@ -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 diff --git a/src/compiler/crystal/semantic/main_visitor.cr b/src/compiler/crystal/semantic/main_visitor.cr index 0f25431041bc..0ace2fe74b32 100644 --- a/src/compiler/crystal/semantic/main_visitor.cr +++ b/src/compiler/crystal/semantic/main_visitor.cr @@ -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 @@ -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 @@ -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: