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

T.new finds #initialize of T's child when T is a parameter #3989

Closed
ezrast opened this issue Feb 3, 2017 · 5 comments
Closed

T.new finds #initialize of T's child when T is a parameter #3989

ezrast opened this issue Feb 3, 2017 · 5 comments

Comments

@ezrast
Copy link

ezrast commented Feb 3, 2017

This appears related to #2665. Feel free to mark as dupe.

The compiler won't let me instantiate a parameter type whose child defines #initialize with different arguments:

class Base
end

class Derived < Base
  def initialize(x : Int32)
  end
end

class Generic(T)
  def initialize
    T.new
  end
end

Generic(Base).new

Compiler output is wrong number of arguments for 'Derived#initialize' (given 0, expected 1) even though Derived#initialize should never get called, unless there's a nuance to the type system I'm missing.

$  crystal --version
Crystal 0.20.5 [ccf46c0] (2017-01-20)
@asterite
Copy link
Member

asterite commented Feb 7, 2017

Right now when you have a generic type, for example Array(Foo), that Foo is taken to be Foo or any of its subclasses. For example if you do:

class Bar < Foo
end

a = [] of Foo
a << Bar.new
a[0].foo # will look up `foo` in Foo and Bar

That's why T.new is looking up new in Base and any of its subclasses.

I don't think we'll change this, but I'll discuss it with the team.

@ezrast
Copy link
Author

ezrast commented Feb 8, 2017

That certainly makes sense for #foo because instance methods need dynamic dispatch, but in my example .new is a class method and T is known at compile-time to be Base, yes? The reason this seems so strange to me is because implementing Derived retroactively invalidates Generic(Base) as a type, even if the two types are in separate parts of the program and never used together. If I'm just not understanding the fundamentals of inheritance, feel free to disregard.

Just for my own edification, am I correct in thinking that this can only happen with .new? Or is it otherwise possible for a class method with a particular signature to be defined on a parent and not on its children?

@asterite
Copy link
Member

asterite commented Feb 8, 2017

@ezrast That's correct, new behaves a bit different than other methods (if a subclass defines a constructor you can't create an instance using a superclass constructor because that could lead to uninitialized data in the subclass). I don't think we'll change this.

That said, why do you need this? What's your use case behind this?

@ezrast
Copy link
Author

ezrast commented Feb 8, 2017

tl;dr: It's necessary for any collection type that needs to be responsible for instantiating its own members.

Lengthy example: If the instrumentation library I'm implementing were specced out differently, I may have written something a little like this:

# metric type that stores count and sum of observed values,
# allowing for a rolling average to be computed
class Summary
  getter count = 0
  getter sum = 0.0
  
  def observe(value : Float64)
    @count += 1
    @sum += value
  end
end

# example usage:
s1 = Summary.new
[1.0, 2.0, 4.0, 7.0].each { |value| s1.observe value }
s1.count # => 4
s1.sum # => 14.0

# as Summary, but also tracks observation counts on a per-bucket basis
class Histogram < Summary
  getter buckets = {} of Float64 => Int32
  
  def initialize(bucket_bounds : Array(Float64))
    bucket_bounds.each do |bound|
      @buckets[bound] = 0
    end
  end
    
  def observe(value : Float64)
    super
    @buckets.each_key do |bound|
      @buckets[bound] += 1 if value <= bound
    end
  end
end

# example usage:
h1 = Histogram.new([1.0, 5.0, 10.0])
[2.0, 4.0, 7.0, 11.0].each { |value| h1.observe value }
h1.buckets # => {1.0 => 0, 5.0 => 2, 10.0 => 3}
h1.count # => 4
h1.sum # => 24.0

# sometimes metrics of the same type need to be grouped
# together, tagged with arbitrary String labels
class LabeledMetric(MetricType)
    @metrics = Hash(String, MetricType).new
    
    def initialize(*args, **kwargs)
      # MetricType.new has to be called from inside this class,
      # because it's a requirement that all metrics in the collection
      # be initialized in the same way - i.e. histograms must all
      # use the same buckets.
      @metrics = Hash(String, MetricType).new do |hh, kk|
        hh[kk] = MetricType.new(*args, **kwargs)
      end
    end

    def [](label)
      @metrics[label]
    end
end
  
# example usage:
h2 = LabeledMetric(Histogram).new([1.0, 5.0, 10.0])
h2["foo"].observe 10.0
h2["foo"].observe 20.0
h2["bar"].observe 50.0
  
h2["foo"].count # => 1
h2["bar"].count # => 2

# but this doesn't work:
# s2 = LabeledMetric(Summary).new

This isn't actually blocking me in real life, because in the real library Histogram isn't quite a superset of Summary so they are siblings rather than parent/child, with no is_a? relationship between them (and their parent class is abstract, so it doesn't provide a conflicting .new either). But users of the library may want to subclass Summary for their own purposes, and doing so could break the LabeledMetric(Summary) type unexpectedly.

@asterite asterite added this to the Next milestone Feb 9, 2017
@asterite
Copy link
Member

asterite commented Feb 9, 2017

@ezrast Hey, that's a really nice use case, and it shows how sometimes one can be very expressive in the language. I don't know if other statically typed languages allow something like the above.

The good news is that this is fixable. Even though a generic container should allow subtypes, when you write T inside a method there's no reason for that to mean "T or any of its subclasses". It makes more sense for type restrictions. So this will be possible in the next version :-)

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

2 participants