Skip to content
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.

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

Tuple covariance #357

Closed
asterite opened this issue Jan 27, 2015 · 12 comments
Closed

Tuple covariance #357

asterite opened this issue Jan 27, 2015 · 12 comments

Comments

@asterite
Copy link
Member

Maybe this should work:

class Foo
end

class Bar < Foo
end

a = [] of {Foo, String}
a << {Foo.new, "hi"} # no overload matches 'Array({Foo+, String})#<<' with types {Foo, String}
a << {Bar.new, "hey"} # no overload matches 'Array({Foo+, String})#<<' with types {Bar, String}

A workaround is to use a record:

class Foo
end

class Bar < Foo
end

record MyTuple, x, y

a = [] of MyTuple
a << MyTuple.new(Foo.new, "hi")
a << MyTuple.new(Bar.new, "hey")
@bcardiff
Copy link
Member

👍

@zamith
Copy link
Contributor

zamith commented Feb 4, 2015

@asterite Why doesn't it work in the first place? Shouldn't T in Array be {Foo, String}, and just work™?

Also, what does record do?

@asterite
Copy link
Member Author

asterite commented Feb 4, 2015

@zamith It doesn't work in the first place because the types don't match. It would seem that the types match at first, but when you write [] of {Foo, String}, since Foo is a class that has subclasses, that is actually typed as Array(Tuple(Foo+, String)) by the compiler, where Foo+ means "Foo or any of its subclasses". Then the method Array#<<(value : T) has T as a type restriction. When you restrict Bar by Foo the result you get is Bar (that is, the restriction was successful and the result of it is Bar). If you restrict String with Foo you get nil as result (no type matched), and that's where you would get a compile error. If you restrict Int32 by Int32 | String you would get Int32, etc. In this case, when you restrict {Foo, String} by {Foo+, String} you get nil. All of this logic is in restrictions.cr. For a tuple, the logic is specifically here. And this line is why it fails: if the types are not exactly the same, it fails. So it would seem that changing that line to return nil unless restricted would make it work, but then the codegen would have to know that you can assign a tuple of a more specific type to one less specific, doing the necessary casts (for classes, the representation of Foo and Foo+ is the same, but things get tricky if you have unions). All of that logic is in cast.cr.

You can try to fix this, is you want :-) But I'll have to explain you much, much more than this, specially because the compiler's code is not documented at all.

As for record, it's a macro that basically generates an immutable struct with fields with those names. Kind of like a tuple, but with names instead of using indices. That file is required by the prelude so it's available to all programs.

@zamith
Copy link
Contributor

zamith commented Feb 4, 2015

I think I understand the restriction part. The codegen not so much, there are so many overloads that I got lost. :P

You can go ahead an fix it if you have time, I'll then look at the solution.

@js-ojus
Copy link
Contributor

js-ojus commented Sep 7, 2015

@asterite You say "If you restrict String with Foo you get nil as result (no type matched), and that's where you would get a compile error."

Could you please elaborate why String, which is in the second position in the tuple, is being tried for restriction with Foo+, which is in the first? I am missing something, evidently, but I find this surprising. Thanks.

@kumpelblase2
Copy link
Contributor

@js-ojus I think it was just an example as to what the result of an unsuccesful restriction would be (Since it is evidently clear that Foo is not a String or vice versa). It doesn't actually happen in the example he brought up.

@asterite
Copy link
Member Author

Finally, this is now implemented.

I also made the union of tuple types of the same size be smarter and more convenient. Previously I was reluctant to use tuples in some situations, for example:

def foo
  if 1 == 2
    {true, "hello"}
  else
    {false, 1}
  end
end

typeof(foo) # => {Bool, Int32 | String}

# before this change: ({Bool, Int32} | {Bool, String})

I think now it's more correct because the intention was probably to return Bool as the first component and Int32 | String as the second one, not two different tuple types. I'd often use record because of this, but now it's not needed anymore :-)

@asterite
Copy link
Member Author

I think the logic for tuple covariance is wrong, it breaks the type system in some unintuitive ways. I'll kind of revert that change but only let it work for class types... I'll comment later after I do it, and explain the reasons.

@asterite asterite reopened this Apr 27, 2016
@asterite
Copy link
Member Author

I reverted the logic of merging unrelated tuple types into a common tuple type. It's wrong and makes things less obvious. For example:

tup = rand() < 0.5 ? {1, 2} : {true, false}
tup.class

The class must be {Int32, Int32} or {Bool, Bool}, it shouldn't be {Int32 | Bool, Int32 | Bool}. Preserving this type information is important, for example I'd want to do tup.is_a?({Int32, Int32}).

However, assigning a "smaller" tuple type to a "bigger" tuple type works, which was the thing this issue was all about:

a = [] of {Int32 | String, Int32}
a << {1, 2} # works

And the same goes for class inheritance (putting a {Bar} where {Foo} is expected, if Bar < Foo).

So this works like this (like before):

def foo
  if 1 == 2
    {true, "hello"}
  else
    {false, 1}
  end
end

typeof(foo) # => {Bool, Int32} | {Bool, String}

@asterite asterite reopened this Apr 27, 2016
@asterite
Copy link
Member Author

I decided to revert it here. This needs a lot more thought and it's far from trivial to implement.

@asterite
Copy link
Member Author

We discussed this feature with @waj and decided we'll implement this. The way this will work is that a union of two tuples with the same size will return in a single tuple type of unions of types in each position. For example:

def foo
  if condition
    {true, "hello"}
  else
    {'a', 1}
  end
end

typeof(foo) # => {Bool | Char, String | Int32}

I previously tried to implement that but stumbled upon some problems (#2524), though I stumbled upon the same problems with named tuples (#2573) and having them being compatible with regards to the keys and types, without having order matter. I fixed those problems for named tuples and the solution will be the same for tuples.

This change will be a breaking change, though I believe a small one, because of this:

def foo
  if 1 == 1
    {true, "hello"}
  else
    {'a', 1}
  end
end

tuple = foo()

# Before
typeof(tuple) # => {Bool, String} | {Char, Int32}
tuple.class #=> {Bool, String}
tuple.is_a?({Bool, String}) # => true

# After this change 
typeof(tuple) # => {Bool | String, Char | Int32}
tuple.class # => {Bool | String, Char | Int32}
tuple.is_a?({Bool, String}) # => false

In a way, type information is lost. However, we could change the meaning of is_a? to apply is_a? for each of the tuple's members, so the last is_a? will give true, and when doing if var.is_a?, the tuple elements will be downcasted:

if tuple.is_a?({Bool, String})
  typeof(tuple) # => {Bool, String}
end

We'll probably implement this last bit later.

@ozra
Copy link
Contributor

ozra commented May 10, 2016

I like the idea of changing the is_a? check on tuples accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants