Crystal: The compiler does not enforce `abstract def method` when calling include module

Created on 28 Dec 2019  Â·  4Comments  Â·  Source: crystal-lang/crystal

See this example. For some reason, the compiler enforces abstract def unsafe_fetch, but does not enforce abstract def size. This snippet below causes stack overflow:

https://play.crystal-lang.org/#/r/8ao2

struct MyArray(T)
  @inner = Array(T).new
  include Indexable(T)

  # The compiler barks if this is missing
  def unsafe_fetch(index : Int) : T
    @inner[index]
  end

  # Indexable has `abstract def size`,
  # For some reason the compiler lets it through.
  # So when you call .size or [] it will just stack overflow
  # def size
  #  @inner.size
  # end
end

a = MyArray(String).new

a.size
bug topicsemantic

Most helpful comment

I think it's a mistake that Enumerable defines a #size method. Maybe it should be called #count like in Ruby. Then we have "aliases" like size and count in some cases, but maybe Ruby was right after all. I also wouldn't mind having take(10) for Array, Enumerable, etc., as an alias of first. Maybe a couple of aliases aren't that bad... but in the case of size vs count, count would be computed and involve some calculation, but size is just a cached value.

All 4 comments

Indexable includes Enumerable which defines size. Not a bug.

Looks like this is what happens:

→ size is implemented in Enumerable#size
→ which calls each to go though each element and count them, implemented in Iterable#each
→ which calls size which should be user implemented...
→ repeat → stack overflow

In this case, I think that Iterable should kinda disable previous implementation of size ?

I think it's a mistake that Enumerable defines a #size method. Maybe it should be called #count like in Ruby. Then we have "aliases" like size and count in some cases, but maybe Ruby was right after all. I also wouldn't mind having take(10) for Array, Enumerable, etc., as an alias of first. Maybe a couple of aliases aren't that bad... but in the case of size vs count, count would be computed and involve some calculation, but size is just a cached value.

What about the option of not implementing size at all, just mark it abstract and let whoever include the module implement size? It breaks backward compatibility in some-cases, though... we pre 1.0 right? :P

Was this page helpful?
0 / 5 - 0 ratings