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

crystal choose wrong initialize method #4086

Closed
kostya opened this issue Feb 27, 2017 · 2 comments · Fixed by #10901
Closed

crystal choose wrong initialize method #4086

kostya opened this issue Feb 27, 2017 · 2 comments · Fixed by #10901
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:generics

Comments

@kostya
Copy link
Contributor

kostya commented Feb 27, 2017

class A
  def initialize
  end
end

class A::B(T) < A
  def initialize(@v : T)
  end
end

class A::C::B(T) < A
  def initialize(@v : T, @v2 : Int32)
  end
end

A::C::B(Int32).new(1, 10)
wrong number of arguments for 'A::B(Int32)#initialize' (given 2, expected 1)
Overloads are:
 - A::B(T)#initialize(v : T)

  def initialize(@v : T, @v2 : Int32)
  ^
@asterite asterite added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler labels Mar 2, 2017
@asterite
Copy link
Member

asterite commented Mar 3, 2017

Closing as duplicate of #2665 (generics inheritance is still broken)

@HertzDevil
Copy link
Contributor

HertzDevil commented Jul 7, 2021

A::C::B.new is expanded into the following:

class A::C::B(T) < A
  def self.new(v : T, v2 : Int32)
    obj = B(T).allocate
    obj.initialize(v, v2)
    ::GC.add_finalizer(obj) if obj.responds_to?(:finalize)
    obj
  end
end

Here path lookup for B(T) in the constructor returns A::B(T) because for some reason lookup proceeds in A::C::B's superclass first. It actually can happen without generics at all:

class A
  class B
  end

  module C
    class B < A
      def self.foo
        # lookup tries the following types in the given order:
        # B relative to ::A::C::B (direct namespace child)
        # B relative to ::A (ancestors)
        # B relative to ::A::C (enclosing namespace)
        B
      end
    end
  end
end

A::C::B.foo # => A::B
# same code in Ruby produces A::C::B

We can either use the fully qualified name (::A::C::B(T)) in the expanded new, or change the path lookup rules to always consider the current scope before the parent types; that is, only for the first component of the path, if a direct child cannot be found but this component is equal to the last component of the current scope's name, we use that immediately and skip the parents. (The namespace lookup will always succeed and find the current scope again, but that is skipped here too.)

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:generics
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants