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

Inconsistent variance in generics #1294

Closed
whorbowicz opened this issue Aug 27, 2015 · 4 comments
Closed

Inconsistent variance in generics #1294

whorbowicz opened this issue Aug 27, 2015 · 4 comments

Comments

@whorbowicz
Copy link

Introduction

I have started to play with the language and found something that looks inconsistent to me.
Let's create class hierarchy:

class GrandParent
end

class Parent < GrandParent
    def hello
        "Hello, I am a Parent."
    end
end

class Child < Parent
    def hello
        "Hello, I am a Child."
    end
end

and a method

def say_hello(p: Parent)
    puts p.hello
end

When I call this method passing instance of Parent or Child everything works as expected:

say_hello(Parent.new) # prints Hello, I am a Parent.
say_hello(Child.new) # prints Hello, I am a Child.

If I try to call
say_hello(GrandParent.new)
Compiler returns an error:

no overload matches 'say_hello' with types GrandParent
Overloads are:
 - say_hello(p : Parent)

say_hello(GrandParent.new) 
^~~~~~~~~

Now let's add types hierarchy with generic parameter:

class GrandWrapper(T)
    getter value
    def initialize(@value : T)
    end
end

class Wrapper(T) < GrandWrapper(T)
    def initialize(@value : T)
    end
end

class ChildWrapper(T) < Wrapper(T)
    def initialize(@value : T)
    end
end

and another method:

def say_hello(w: Wrapper(Parent))
    puts w.value.hello
end

Now I can call:

say_hello(Wrapper.new(Parent.new)) # prints Hello, I am a Parent.
say_hello(Wrapper.new(Child.new)) # prints Hello, I am a Child.
say_hello(ChildWrapper.new(Parent.new)) # prints Hello, I am a Parent.
say_hello(ChildWrapper.new(Child.new)) # prints Hello, I am a Child.

As expected I cannot compile any of following

say_hello(Wrapper.new("Fake parent"))
say_hello(ChildWrapper.new("Fake child"))
say_hello(GrandWrapper.new("Fake grandparent"))
say_hello(GrandWrapper.new(Parent.new))
say_hello(GrandWrapper.new(Child.new))
say_hello(GrandWrapper.new(GrandParent.new))
say_hello(Wrapper.new(GrandParent.new))

as compiler returns something like:

no overload matches 'say_hello' with types Wrapper(String)
Overloads are:
 - say_hello(p : Parent)
 - say_hello(w : Wrapper(Parent))

Issue

What is inconsistent for me however is that when I try:

say_hello(ChildWrapper.new(GrandParent.new))

I get completely different error:

Error in ./generics.cr:57: instantiating 'say_hello(ChildWrapper(GrandParent+))'

say_hello(ChildWrapper.new(GrandParent.new))
^~~~~~~~~

in ./generics.cr:40: undefined method 'hello' for GrandParent

 puts w.value.hello
              ^~~~~

================================================================================

GrandParent trace:

  macro getter (in /opt/crystal/src/object.cr:203):4

          def value
              ^~~~~

  macro getter (in /opt/crystal/src/object.cr:203):5

            @value
            ^~~~~~

  ./generics.cr:35

     def initialize(@value : T)
                    ^

  ./generics.cr:35

     def initialize(@value : T)
                    ^~~~~

  ./generics.cr:35

     def initialize(@value : T)
                    ^

Looks like method signature was 'updated' to say_hello(ChildWrapper(GrandParent+)) but then call to hello failed as it was called on GrandParent.

Checking further, the following compiles:

def print_type(w: Wrapper(Parent))
    puts typeof(w)
    puts typeof(w.value)
end

print_type ChildWrapper.new(GrandParent.new)

and prints

ChildWrapper(GrandParent+)
GrandParent

while print_type Wrapper.new(GrandParent.new)
does not compile:

Error in ./generics.cr:65: no overload matches 'print_type' with types Wrapper(GrandParent+)
Overloads are:
 - print_type(w : Wrapper(Parent))

print_type Wrapper.new(GrandParent.new)
^~~~~~~~~~

Summary

From the above it seems that contradiction exists:

puts (ChildWrapper.new(GrandParent.new).is_a?(Wrapper(GrandParent))) # true
puts (ChildWrapper.new(GrandParent.new).is_a?(Wrapper(Parent))) # true
puts (Wrapper.new(GrandParent.new).is_a?(Wrapper(Parent))) # false

This looks like a bug to me.

@asterite
Copy link
Member

You are right. And thanks for the detailed explanation!

Before fixing this I'd like to sit down and define this behaviour once and for all, because we can't fix it until we understand what we want.

One thing we definitely don't want is to deal with covariance, contravariance and other complications. So, even though in your example the type passes the type restriction, the code eventually doesn't compile. This is simpler than having to deal with extra type annotations for covariance.

I also would try not to rely to much on type checking and try to program using duck typing.

Of course, all of the above doesn't mean we shouldn't tackle this issue.

As a side note: what are your real generic types in your program?

@bcardiff
Copy link
Member

I think it's too soon to assert how we want to deal with covariance and contravariance. I would like to them in the future. All the formality around type system is important.

Actually 52b06ff is one case of how return types can be changed in subclasses :-). How it is implemented right now is a bit too far from classical subtyping rules, and is fine.

Another place where covariance is needed, but a workaround with record exists is, #357 .

I support the idea of not rushing to implement a whole formal system without having a use case. Trying first with how types/methods/macros are expanded and see if that, maybe, is enough.

But I think the formal implementation of covariance and contravariance will be needed.

@asterite
Copy link
Member

I was able to fix this. This line:

puts (ChildWrapper.new(GrandParent.new).is_a?(Wrapper(Parent)))

should definitely be false.

And:

say_hello(ChildWrapper.new(GrandParent.new))

should definitely not match.

The discussion about covariance/contravariance is something that we should definitely keep in mind, and solve. I opened #1297 to keep the discussion there.

@whorbowicz
Copy link
Author

That was fast :)
@asterite As I said in introduction I was playing with the code - I was reading Crystal docs about Generics and wanted to check if variance works. I just coded the example above (well back then the wrappers were Foo, Bar and Baz).

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

3 participants