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

Compiler problem in 0.16.0 with generics #2558

Closed
mrkaspa opened this issue May 6, 2016 · 5 comments
Closed

Compiler problem in 0.16.0 with generics #2558

mrkaspa opened this issue May 6, 2016 · 5 comments
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler

Comments

@mrkaspa
Copy link

mrkaspa commented May 6, 2016

This code isn't working in the 0.16.0 version

abstract class Ord(A)
  abstract def cmp(a : A, b : A)
end

class IntOrd < Ord(Int32)
  def cmp(a, b)
    a > b
  end
end

class Sorter(A)
  getter :order

  def initialize(@order : Ord(A))
  end

  def sort(list : Array(A))
    i = 0
    j = list.size - 1
    aux = nil
    while i < j
      if order.cmp(list[i], list[j])
        aux = list[i]
        list[i] = list[j]
        list[j] = aux
      end
      i += 1
      j -= 1
    end
    list
  end
end

ls = [5,5,2]
puts Sorter(Int32).new(IntOrd.new).sort(ls)
puts ls

I got this

Error in ./demo.cr:60: instantiating 'Sorter(Int32):Class#new(IntOrd)'

puts Sorter(Int32).new(IntOrd.new).sort(ls)
                   ^~~

instantiating 'Sorter(Int32)#initialize(IntOrd)'

in ./demo.cr:39: instance variable '@order' of Sorter(Int32) must be Ord(Int32), not IntOrd

  def initialize(@order : Ord(A))
@asterite asterite added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler labels May 6, 2016
@asterite
Copy link
Member

asterite commented May 6, 2016

Yes :-(

In general, generic classes inheritance is broken and I wouldn't recommend using it right now. This is not a limitation in the language, it's just that the current implementation for this isn't good enough (at the time I did it I overlooked some things). I have to sit down with @waj and @bcardiff and fix all issues around generic classes inheritance.

Since we now have the new "global" type inference implemented, solving this is a next priority. At least the way I see it, it's where most bugs come from.

As a workaround, until we fix this, you can include a dummy module in Ord(A) and make @order be of that type. For example:

module OrdModule
end

abstract class Ord(A)
  include OrdModule

  abstract def cmp(a : A, b : A)
end

class IntOrd < Ord(Int32)
  def cmp(a, b)
    a > b
  end
end

class Sorter(A)
  @order : OrdModule

  getter :order

  def initialize(@order : Ord(A))
  end

  def sort(list : Array(A))
    i = 0
    j = list.size - 1
    aux = nil
    while i < j
      if order.cmp(list[i], list[j])
        aux = list[i]
        list[i] = list[j]
        list[j] = aux
      end
      i += 1
      j -= 1
    end
    list
  end
end

ls = [5, 5, 2]
puts Sorter(Int32).new(IntOrd.new).sort(ls)
puts ls

@plukevdh
Copy link
Contributor

plukevdh commented May 6, 2016

How does this affect built-in generics? For example @porras' mock library has the following class which is no longer valid Crystal code in 0.16.0:

module Mock
  class Arguments
    def initialize(arguments)
      @arguments = arguments.to_a
    end

    def_equals @arguments

    def to_s
      @arguments.inspect
    end

    def self.empty
      Empty.new
    end

    class Empty < Arguments
      def initialize
      end

      def ==(other)
        false
      end

      def ==(other : Empty)
        true
      end
    end
  end
end

It complains first that the Arguments::Empty subclass doesn't set the @arguments var, but even on resolving that, I have no idea how to specify the @arguments as an array of any possible type. In this case, I want it to be permissive or at least allow it to contain any type I give it. Any way around this for the time being?

@asterite
Copy link
Member

asterite commented May 6, 2016

Yes, in the future you will be able to have a variable of type Object, but not right now. I'm not sure it will be useful in that case, why not make Arguments be Arguments(T) and infer T from the arguments argument?

Also note that it doesn't make much sense to have Empty < Arguments... what happened with @arguments? You forgot to initialize it. So it will be nil too? Probably it should be a separate type, not inherit Arguments.

@plukevdh
Copy link
Contributor

plukevdh commented May 6, 2016

Okay, what about the case like

  class MethodStub
    getter :value

   # ...

    def and_return(@value)
      self
    end
  end

where @value could be anything and we aren't setting it in the initializer? Obviously this is a problem in the way the DSL is defined, but it's a fairly common pattern. Is there anyway around this for now?

@asterite
Copy link
Member

Closed in favor of #2665

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler
Projects
None yet
Development

No branches or pull requests

3 participants