diff --git a/spec/compiler/codegen/case_spec.cr b/spec/compiler/codegen/case_spec.cr index 8daadf32fedd..5ef076d5eadd 100644 --- a/spec/compiler/codegen/case_spec.cr +++ b/spec/compiler/codegen/case_spec.cr @@ -129,4 +129,94 @@ describe "Code gen: case" do end CRYSTAL end + + it "does case when with a type union, doesn't resolve to virtual type (#9665)" do + run(<<-CRYSTAL).to_i.should eq(2) + class Foo; end + class Bar < Foo; end + class Baz < Foo; end + class Bang < Foo; end + + x = Bang.new || Bar.new + case x + when Bar | Baz + 1 + else + 2 + end + CRYSTAL + end + + it "does case when with a union alias, doesn't resolve to virtual type (#9665)" do + run(<<-CRYSTAL).to_i.should eq(2) + class Foo; end + class Bar < Foo; end + class Baz < Foo; end + class Bang < Foo; end + + alias BarBaz = Bar | Baz + + x = Bang.new || Bar.new + case x + when BarBaz + 1 + else + 2 + end + CRYSTAL + end + + it "does case when with a type union, matches a member (#9665)" do + run(<<-CRYSTAL).to_i.should eq(1) + class Foo; end + class Bar < Foo; end + class Baz < Foo; end + + x = Bar.new || Baz.new + case x + when Bar | Baz + 1 + else + 2 + end + CRYSTAL + end + + it "does case when with a flags enum union keeps value semantics (#9665)" do + run(<<-CRYSTAL).to_i.should eq(2) + require "prelude" + + @[Flags] + enum Color + Red + Green + Blue + end + + x = Color::Green + case x + when Color::Red | Color::Blue + 1 + else + 2 + end + CRYSTAL + end + + it "does case when with a numeric constant union keeps value semantics (#9665)" do + run(<<-CRYSTAL).to_i.should eq(2) + require "prelude" + + A = 1 + B = 2 + + x = 4 + case x + when A | B + 1 + else + 2 + end + CRYSTAL + end end diff --git a/spec/compiler/codegen/exception_spec.cr b/spec/compiler/codegen/exception_spec.cr index 9e725711a07b..ff4106e1c026 100644 --- a/spec/compiler/codegen/exception_spec.cr +++ b/spec/compiler/codegen/exception_spec.cr @@ -493,6 +493,51 @@ describe "Code gen: exception" do CRYSTAL end + it "rescues a union alias by its members, not their ancestor (#9665)" do + run(<<-CRYSTAL).to_i.should eq(2) + require "prelude" + + class FooErr < Exception; end + class BarErr < FooErr; end + class BazErr < FooErr; end + class QuuxErr < FooErr; end + + alias BarBaz = BarErr | BazErr + + x = 0 + begin + begin + raise QuuxErr.new + rescue BarBaz + x = 1 + end + rescue + x = 2 + end + x + CRYSTAL + end + + it "rescues a member through a union alias (#9665)" do + run(<<-CRYSTAL).to_i.should eq(1) + require "prelude" + + class FooErr < Exception; end + class BarErr < FooErr; end + class BazErr < FooErr; end + + alias BarBaz = BarErr | BazErr + + x = 0 + begin + raise BarErr.new + rescue BarBaz + x = 1 + end + x + CRYSTAL + end + it "receives exception object" do run(<<-CRYSTAL).to_string.should eq("Ex1") require "prelude" diff --git a/spec/compiler/codegen/is_a_spec.cr b/spec/compiler/codegen/is_a_spec.cr index a69fdc2b19c1..325b658809b9 100644 --- a/spec/compiler/codegen/is_a_spec.cr +++ b/spec/compiler/codegen/is_a_spec.cr @@ -901,6 +901,91 @@ describe "Codegen: is_a?" do CRYSTAL end + it "does is_a? with union type as alias, don't resolve to virtual type (#9665)" do + run(<<-CRYSTAL).to_b.should be_false + class A + end + + class B < A + end + + class C < A + end + + class D < A + end + + alias BorC = B | C + + x = D.new || C.new + x.is_a?(BorC) + CRYSTAL + end + + it "does is_a? with union type as alias of Union(X, Y), don't resolve to virtual type (#9665)" do + run(<<-CRYSTAL).to_b.should be_false + class A + end + + class B < A + end + + class C < A + end + + class D < A + end + + alias BorC = Union(B, C) + + x = D.new || C.new + x.is_a?(BorC) + CRYSTAL + end + + it "does is_a? with union type as alias of an alias, don't resolve to virtual type (#9665)" do + run(<<-CRYSTAL).to_b.should be_false + class A + end + + class B < A + end + + class C < A + end + + class D < A + end + + alias BorC = B | C + alias BorC2 = BorC + + x = D.new || C.new + x.is_a?(BorC2) + CRYSTAL + end + + it "does is_a? true for a union alias member's subclass (#9665)" do + run(<<-CRYSTAL).to_b.should be_true + class A + end + + class B < A + end + + class C < A + end + + class SubB < B + end + + alias BorC = B | C + + x = SubB.new.as(A) + x.is_a?(BorC) + CRYSTAL + end + it "restricts union metaclass to metaclass (#12295)" do run(<<-CRYSTAL).to_i.should eq(2) x = true ? Union(String | Int32) : String diff --git a/spec/compiler/semantic/restrictions_spec.cr b/spec/compiler/semantic/restrictions_spec.cr index 508b21ead459..4656aba7b6dd 100644 --- a/spec/compiler/semantic/restrictions_spec.cr +++ b/spec/compiler/semantic/restrictions_spec.cr @@ -1000,6 +1000,54 @@ describe "Restrictions" do CRYSTAL end + it "restricts against an alias of a union by its members, not their ancestor (#9665)" do + assert_error <<-CRYSTAL, "expected argument #1 to 'foo' to be Foo, not Quux" + class Foo; end + class Bar < Foo; end + class Baz < Foo; end + class Quux < Foo; end + + alias BarBaz = Bar | Baz + + def foo(x : BarBaz) + end + + foo(Quux.new) + CRYSTAL + end + + it "restricts against an alias of Union(X, Y) by its members, not their ancestor (#9665)" do + assert_error <<-CRYSTAL, "expected argument #1 to 'foo' to be Foo, not Quux" + class Foo; end + class Bar < Foo; end + class Baz < Foo; end + class Quux < Foo; end + + alias BarBaz = Union(Bar, Baz) + + def foo(x : BarBaz) + end + + foo(Quux.new) + CRYSTAL + end + + it "matches a member against an alias of a union restriction (#9665)" do + assert_type(<<-CRYSTAL) { int32 } + class Foo; end + class Bar < Foo; end + class Baz < Foo; end + + alias BarBaz = Bar | Baz + + def foo(x : BarBaz) + 1 + end + + foo(Bar.new) + CRYSTAL + end + it "should not let GenericChild(Base) pass as a GenericBase(Child) (#1294)" do assert_error <<-CRYSTAL, "expected argument #1 to 'foo' to be GenericBase(Child), not GenericChild(Base)" class Base diff --git a/src/compiler/crystal/semantic/literal_expander.cr b/src/compiler/crystal/semantic/literal_expander.cr index 8eaf046ff3d8..0b4d00a19962 100644 --- a/src/compiler/crystal/semantic/literal_expander.cr +++ b/src/compiler/crystal/semantic/literal_expander.cr @@ -921,7 +921,7 @@ module Crystal case cond when NilLiteral return IsA.new(right_side, Path.global("Nil")) - when Path, Generic + when Path, Generic, Union return IsA.new(right_side, cond) when Call obj = cond.obj @@ -937,6 +937,17 @@ module Crystal else # no special treatment end + + # A `when Foo | Bar` parses as a `|` call between type-like operands, + # not as a `Union` node. Turn it into an `is_a?` against the union so it + # behaves like `when Foo, Bar` and like `x.is_a?(Foo | Bar)`, instead of + # comparing the union value (its common ancestor) with `===` (#9665). + # If the operands turn out to be constants (e.g. enum members or numeric + # constants) rather than types, the `IsA` is reverted to the original + # `===` comparison during semantic analysis. + if union = case_when_union(cond) + return IsA.new(right_side, union) + end else # no special treatment end @@ -944,6 +955,42 @@ module Crystal Call.new(cond, "===", right_side) end + # If *node* is a `|` call chain between type-like operands (`Foo | Bar`, + # `Foo | Bar | Baz`, possibly with `.class` or aliases), returns the + # equivalent `Union` node. Otherwise returns `nil`. + private def case_when_union(node) + return nil unless node.is_a?(Call) + return nil unless node.name == "|" && node.args.size == 1 + obj = node.obj + arg = node.args.first + return nil unless obj + + left = case_when_union_operand(obj) + return nil unless left + right = case_when_union_operand(arg) + return nil unless right + + Union.new(left + right).at(node) + end + + # Returns the list of type nodes *node* contributes to a `case ... when` + # union, or `nil` if it isn't a type-like operand. + private def case_when_union_operand(node) : Array(ASTNode)? + case node + when Path, Generic, Union + [node] of ASTNode + when Call + obj = node.obj + if node.name == "class" && node.args.empty? && (obj.is_a?(Path) || obj.is_a?(Generic)) + [Metaclass.new(obj).at(obj)] of ASTNode + else + case_when_union(node).try { |union| union.types } + end + else + nil + end + end + macro check_implicit_obj(type) if cond.is_a?({{type}}) cond_obj = cond.is_a?(Not) ? cond.exp : cond.obj diff --git a/src/compiler/crystal/semantic/main_visitor.cr b/src/compiler/crystal/semantic/main_visitor.cr index 4018810f7e95..9a75f04e47bb 100644 --- a/src/compiler/crystal/semantic/main_visitor.cr +++ b/src/compiler/crystal/semantic/main_visitor.cr @@ -219,7 +219,11 @@ module Crystal # # It's different if from a virtual type we do `v.class.new` # because the class could be any in the hierarchy. - node.type = check_type_in_type_args(type.remove_alias_if_simple).devirtualize + # Inside `is_a?`, an alias of a union must keep the union's members + # rather than generalize to their common ancestor, so that it behaves + # like an inline union (#9665). + unaliased = @inside_is_a ? type.remove_alias_union_of : type.remove_alias_if_simple + node.type = check_type_in_type_args(unaliased).devirtualize node.target_type = type when ASTNode type.accept self unless type.type? @@ -1884,6 +1888,27 @@ module Crystal return false end + # A `when Foo | Bar` is expanded into an `is_a?` against a union, but if + # any member is a constant (an enum member or a numeric constant) rather + # than a type it must keep its original `(Foo | Bar) === x` value + # comparison. This happens after type resolution, so we can tell apart a + # union of constants from a union of types here (#9665). + if const.is_a?(Union) + members = const.types + if members.any? { |member| member.is_a?(Path) && member.target_const } + obj = node.obj.clone.at(node.obj) + combined = members.first.clone + members[1..].each do |member| + combined = Call.new(combined, "|", member.clone).at(node.const) + end + comp = Call.new(combined, "===", obj).at(node) + comp.accept self + node.syntax_replacement = comp + node.bind_to comp + return false + end + end + if needs_type_filters? && (var = get_expression_var(node.obj)) @type_filters = TypeFilters.new var, SimpleTypeFilter.new(node.const.type) end @@ -2886,6 +2911,21 @@ module Crystal if node_types = node.types types = node_types.map do |type| type.accept self + + # A `rescue ex : BarBaz` where `alias BarBaz = Bar | Baz` must catch + # the union's members, not their common ancestor, like the inline + # `rescue ex : Bar | Baz` (#9665). Re-resolve the alias keeping the + # members and update the node so codegen matches the same types. + if type.is_a?(Path) + target_type = type.target_type + if target_type.is_a?(AliasType) + unaliased = target_type.remove_alias_union_of + if unaliased != type.type.instance_type + type.type = unaliased.metaclass + end + end + end + instance_type = type.type.instance_type unless self.allowed_type_in_rescue? instance_type diff --git a/src/compiler/crystal/semantic/restrictions.cr b/src/compiler/crystal/semantic/restrictions.cr index 9877f1dabbf7..81e789ab4e06 100644 --- a/src/compiler/crystal/semantic/restrictions.cr +++ b/src/compiler/crystal/semantic/restrictions.cr @@ -827,7 +827,10 @@ module Crystal if self == other self else - restrict(other.remove_alias, context) + # An alias of a union must restrict against the union's members, not + # their common ancestor, so `def foo(x : BarBaz)` behaves like the + # inline `def foo(x : Bar | Baz)` (#9665). + restrict(other.remove_alias_union_of, context) end end diff --git a/src/compiler/crystal/semantic/type_lookup.cr b/src/compiler/crystal/semantic/type_lookup.cr index 9b30a9d77aa6..e38c41ea4de6 100644 --- a/src/compiler/crystal/semantic/type_lookup.cr +++ b/src/compiler/crystal/semantic/type_lookup.cr @@ -41,13 +41,13 @@ class Crystal::Type # ``` # # If `self` is `Foo` and `Bar(Baz)` is given, the result will be `Foo::Bar(Baz)`. - def lookup_type(node : ASTNode, self_type = self.instance_type, allow_typeof = true, free_vars : Hash(String, TypeVar)? = nil, find_root_generic_type_parameters = true) : Type - TypeLookup.new(self, self_type, true, allow_typeof, free_vars, find_root_generic_type_parameters).lookup(node).not_nil! + def lookup_type(node : ASTNode, self_type = self.instance_type, allow_typeof = true, free_vars : Hash(String, TypeVar)? = nil, find_root_generic_type_parameters = true, inside_is_a = false) : Type + TypeLookup.new(self, self_type, true, allow_typeof, free_vars, find_root_generic_type_parameters, inside_is_a: inside_is_a).lookup(node).not_nil! end # Similar to `lookup_type`, but returns `nil` if a type can't be found. - def lookup_type?(node : ASTNode, self_type = self.instance_type, allow_typeof = true, free_vars : Hash(String, TypeVar)? = nil, find_root_generic_type_parameters = true) : Type? - TypeLookup.new(self, self_type, false, allow_typeof, free_vars, find_root_generic_type_parameters).lookup(node) + def lookup_type?(node : ASTNode, self_type = self.instance_type, allow_typeof = true, free_vars : Hash(String, TypeVar)? = nil, find_root_generic_type_parameters = true, inside_is_a = false) : Type? + TypeLookup.new(self, self_type, false, allow_typeof, free_vars, find_root_generic_type_parameters, inside_is_a: inside_is_a).lookup(node) end # Similar to `lookup_type`, but the result might also be an ASTNode, for example when @@ -62,7 +62,7 @@ class Crystal::Type end private struct TypeLookup - def initialize(@root : Type, @self_type : Type, @raise : Bool, @allow_typeof : Bool, @free_vars : Hash(String, TypeVar)? = nil, @find_root_generic_type_parameters = true, @remove_alias = true) + def initialize(@root : Type, @self_type : Type, @raise : Bool, @allow_typeof : Bool, @free_vars : Hash(String, TypeVar)? = nil, @find_root_generic_type_parameters = true, @remove_alias = true, @inside_is_a = false) @in_generic_args = 0 # If we are looking types inside a non-instantiated generic type, @@ -138,7 +138,9 @@ class Crystal::Type type.process_value end end - type = type.remove_alias_if_simple if @remove_alias + if @remove_alias + type = @inside_is_a ? type.remove_alias_union_of : type.remove_alias_if_simple + end end type @@ -156,7 +158,11 @@ class Crystal::Type type.virtual_type end - program.type_merge(types) + if @inside_is_a + program.type_merge_union_of(types) + else + program.type_merge(types) + end end def lookup(node : Metaclass) @@ -299,6 +305,11 @@ class Crystal::Type # union types only when the type is instantiated. # TODO: check that everything is a type MixedUnionType.new(@root.program, type_vars.map(&.as(Type))) + elsif instance_type.is_a?(GenericUnionType) && @inside_is_a + # In the case of `exp.is_a?(Union(X, Y))` we make it work exactly + # like `exp.is_a?(X | Y)`, which won't resolve `X | Y` to the virtual + # parent type (#9665, #10244). + instance_type.instantiate(type_vars, type_merge_union_of: true) else instance_type.as(GenericType).instantiate(type_vars) end diff --git a/src/compiler/crystal/types.cr b/src/compiler/crystal/types.cr index fed6f8d68f01..85ddd68fc891 100644 --- a/src/compiler/crystal/types.cr +++ b/src/compiler/crystal/types.cr @@ -690,6 +690,14 @@ module Crystal self end + # Like `remove_alias`, but if the alias (transitively) resolves to a + # union, that union keeps its members instead of being merged into their + # common ancestor. Used inside `is_a?`, where `Foo | Bar` must not be + # generalized — mirroring how an inline union behaves there (#9665). + def remove_alias_union_of + self + end + def remove_indirection self end @@ -2777,6 +2785,25 @@ module Crystal @simple ? remove_alias : self end + def remove_alias_union_of + process_value + # For a recursive (non-simple) alias the value AST refers back to the + # alias itself, so re-resolving it would loop forever; fall back to the + # regular alias resolution there. + if @aliased_type && @simple + # `@aliased_type` was resolved with the regular `type_merge`, which + # generalizes a union to its common ancestor. Re-resolve the original + # value AST preserving the union's members instead, so an alias of a + # union behaves like an inline union inside `is_a?` (#9665). + namespace.lookup_type(@value, + allow_typeof: false, + find_root_generic_type_parameters: false, + inside_is_a: true) + else + remove_alias + end + end + def remove_indirection process_value if aliased_type = @aliased_type