Wonky type matching with recursive type aliases

[dawkot]

alias Magic = Int32 | Array(Magic)

def magic(x : Magic)
  case x
  when Int32 then puts "a"
  when Array(Int32) then puts "b"
  else puts "c"  
  end
end

magic 2_i32 # output: a
magic [2_i32, 3_i32] # output: b
puts
magic 2_i32.as Magic # output: a
magic [2_i32.as(Magic), 3_i32.as(Magic)] # output: c

[asterite]

This is expected behaviour. Array(Int32) is not an Array(Magic).

Every other core team member reading this: can we pretty please remove recursive types already? :-)

[dawkot]

Shouldn't the function change x's type from Array(Int32) to Array(Magic), though?
There's no Array(Int32) in Magic's definition.

[asterite]

Sorry, I might have misunderstood the issue.

Could you please describe what's the problem? It's not clear just by seeing a code snippet with no explanation about what you expect and what you thing is not behaving as expected.

[straight-shoota]

Actually, I don't know if this has anything to do with recursive aliases (they should probably removed nonetheless). It behaves the same if Magic is not recursive. And I don't see anything wrong in that example. An explanation would help to shed some light on the supposed issue here.

alias Magic = Int32 | Array(Int32)

def magic(x)
  case x
  when Int32 then puts "a"
  when Array(Int32) then puts "b"
  else puts "c"  
  end
end

magic 2_i32 # output: a
magic [2_i32, 3_i32] # output: b
puts
magic 2_i32.as Magic # output: a
magic [2_i32.as(Magic), 3_i32.as(Magic)] # output: c

[straight-shoota]

Shouldn't the function change x's type from Array(Int32) to Array(Magic), though?

No. [2_i32, 3_i32] is of type Array(Int32). It can't be coerced to Array(Magic) because that's an entirely different type.

[dawkot]

What I expected was to only have to include the types that are explicitly defined as part of the type alias in the case branches.
As to the second example, it doesn't compile with arg's type annotated.

[dawkot]

Maybe this is a better example:

alias Magic = Int32 | Array(Magic)

def magic(x : Magic)
  case x
  when Array(Magic) then puts "a"
  when Int32 then puts "b"
  else puts "c"  
  end
end

magic [2_i32, 3_i32] # output: c

[dawkot]

It's possible to actually go around the issue by matching on "Array" alone btw.

[asterite]

Please include in your example "I expected this to happen but this happens instead". Just seeing "output: c" doesn't tell us anything. For us, that's expected behavior.

[asterite]

But basically, [1, 2, 3].is_a?(Array(Magic)) is false, not true as I think you are expecting.

[dawkot]

So, for this example:

alias Magic = Int32 | Array(Magic)

def magic(x : Magic)
  case x
  when Array(Magic) then puts "a"
  when Int32 then puts "b"
  else puts "c"  
  end
end

magic [2_i32, 3_i32]

Because Magic only explicitly includes Int32 and Array(Magic) in it's definition, I expect the two case branches to match on every possible input.
Basically I expect every input that doesn't conform to either of the first two case branch to be rejected by the function itself, at compile time.

[asterite]

Yeah, I think it's a bit confusing that Magic is matched in the type restriction, but it's not matched in the case branch.

I think the method shouldn't be matched.

But then again, type restrictions sometimes work in a different way then checks performed by is_a?.

[asterite]

For example:

class Foo
end

class Bar < Foo
end

def magic(x : Array(Foo))
  case x
  when Array(Foo) then puts "is an array of Foo"
  else                 puts "not an array of Foo"
  end
end

magic [Bar.new] # This is Array(Bar)

The above compiles, because Array(Bar) matches the type restriction Array(Foo), because Array matches Array and Bar matches Foo (it's a subclass). However, x is strictly an Array(Bar) not an Array(Foo), so x.is_a?(Array(Foo)) gives false. So the resulting program prints "not an array of Foo", which is probably unexpected.

The reason it gives false is because of this:

class Foo
end

class Bar < Foo
end

def magic(x : Array(Foo))
  if x.is_a?(Array(Foo))
    x << Foo.new
  end
end

magic [Bar.new]

Imagine if x.is_a?(Array(Foo)) would give true. Then we could put a Foo instance inside an array of Bar, which would break the type system.

So maybe the type restriction shouldn't match? Well, the type restriction assumes we are not going to put anything inside the array, or generic type. But is_a? assumes otherwise. This is used in some code, for example:

# We want to match ranges of any integer types
def foo(x : Range(Int, Int))`
end

If we change that rule, we won't be able to generically define methods for ranges of ints, or for array of ints, etc.

That said, maybe that change is a good thing, and type restrictions should behave exactly as an is_a? check. Then we'll have to change all those Range(Int, Int) in the standard library to Range(Int32, Int32), but that's fine.

[asterite]

We need something like:

# Assume we are only going to read from the Array, so checks for subclasses of Foo work
def foo(x : Array(out Foo))
end

# Assume we are going to put things inside an Array, so Array(Bar) where Bar < Foo won't match
def foo(x : Array(in Foo))
end

This is present in C#, for example. However, it maybe makes the language too complex in my opinion. But all of this has to do with covariance and contravariance, which was never really solved/implemented well in Crystal.

[straight-shoota]

Okay, now the issue is visible. And again, this isn't specific to recursive aliases (maybe it's more obvious though).

What you want can be expressed with method overloads instead of case:

alias Magic = Int32 | Array(Magic)

def magic(x : Array(Magic))
  puts "a"
end

def magic(x : Int32)
  puts "b"
end

magic [2_i32, 3_i32] # => a

I have already brought the confusingly different behaviour of argument restrictions and is_a?(which is used incase`) up in #3803. But I don't think this can effectively be changed without providing proper means to specifcy covariance and contravariance behaviour. And that's probably gonna require a larger overhaul of the type system.

[asterite]

Related to #3803

[asterite]

Closing because this bug or weird behavior is already described in #3803