type inference weirdness

[cromega]

Sorry for the very generic issue title, I've bumped into this rather confusing situation, and I have no idea what causes it so I couldn't really come up with a better one. Let the snippets do the talking:

abstract class Animal; end

class Dog < Animal; end

class Cat < Animal; end

class House
  @animals : Array(Animal)

  def initialize(*animals : Animal)
    @animals = animals.to_a
  end
end

p House.new(Cat.new, Cat.new)

$ crystal run animals.cr
Showing last frame. Use --error-trace for full trace.

In animals.cr:11:5

 11 | @animals = animals.to_a
      ^-------
Error: instance variable '@animals' of House must be Array(Animal), not Array(Cat)

If I change one of the cats into a Dog, it compiles.

Crystal 0.34.0 [4401e90f0] (2020-04-06)

LLVM: 8.0.0
Default target: x86_64-unknown-linux-gnu
(Running in WSL)

[cromega]

I read the entry about variance and covariance in the handbook. It says the type of the array needs to be set explicitly. However, in my case the type of the input splat is defined as Animal.

[jhass]

So what's happening is that you're effectively passing an Tuple(Cat, Cat). This passes the argument restriction of Tuple(*Animal), but performs no casting on it. Then Tuple#to_a does Array(Union(*T)).build, so Array(Cat).build effectively. This however is not assignable to Array(Animal). Yes the language is a bit inconsistent there, see #3803. Adding a Dog to the mix, constructs a Union(Cat, Dog) which the compiler simplifies to the parent type, Animal+, hence the observed effect of it working then.

Anyways, for now you can do @animals = Array(Animal).new(animals.size) {|i| animals[i] } to workaround this.

[cromega]

@jhass Thanks for the the explanation, it makes sense. At least in a "now I understand what's happening" way. Shall I keep the issue open?

[jhass]

I'm not sure, let's see what others think. It is indeed not very actionable and mostly covered by #3803 and all the related issues we already have open I think.

[straight-shoota]

I don't think this can be changed in a fundamental way. Maybe when we get to revisit generics and improve covariance. But we could make the workaround easier by adding an optional argument to #to_a, so you could call animals.to_a(Animal) instead of having to re-implement the method.

[cromega]

animals.to_a(Animal) would be great, it communicates the intent a lot better than the workaround.

Additionally, if the inconsistency can not be resolved easily, perhaps a warning could be printed if someone attempts something similar? That way at least I would see the same thing on the console regardless of what parameters I pass into the splat (instead of compilation error in one case and works fine in the other)

[RX14]

Tuple#map returns a tuple, and Tuple#to_a doesn't accept a block for transforming it. I wonder if there's a space for #to_a taking a block, or whether animals.map(&.as(Animal)).to_a is fast enough (on the stack) that there doesn't need to be a new method.

[straight-shoota]

I suppose #to_a(type) could actually be generally useful for any Enumerable, not just Tuple.

module Enumerable
  def to_a(type : U.class = T) forall U
    ary = [] of U
    each { |e| ary << e }
    ary
  end
end

class Foo
end

class Bar < Foo
end

class Baz < Foo
end

typeof([Bar.new, Bar.new].to_a(Foo)) # => Array(Foo)
typeof([Bar.new, Bar.new].to_a)      # => Array(Bar)

[HertzDevil]

On master there is a different way to convert any Enumerable to an Array of appropriate type:

abstract class Animal; end

class Dog < Animal; end

class Cat < Animal; end

class House
  @animals : Array(Animal)

  def initialize(*animals : Animal)
    @animals = [*animals] of Animal
  end
end

House.new(Cat.new, Cat.new) # => #<House:... @animals=[#<Cat:...>, #<Cat:...>]>

Similarly Set(Animal){*animals} creates a Set. They all roughly expand to the same code as the to_a(type) above.

However, if the argument is additionally an Indexable (like in this case), the following is slightly faster:

Array(Animal).new(animals.size) do |i|
  animals.unsafe_fetch(i)
end

For some reason Crystal itself doesn't seem to use that outside Deque.new(array : Array(T)).