Abstract def with union generic expects exact call, not overloads of each type in the union

[ysbaddaden]

The following example fails to compile because a call(Int32 | String) method is expected but doesn't exist, thought there are individual overloads for each type in the union, that could be considered to fulfill the condition:

module Foo(T)
  abstract def call(x : T)
end

class Bar
  include Foo(Int32 | String)

  def call(x : Int32)
  end

  def call(x : String)
  end
end

bar = Bar.new
bar.call(1)
bar.call("str")

An use-case is in Earl::Artist (see this sample for example) where the actor can receive an union of types thought a Channel, that can be dispatched to specific call methods, while still having a compile time check that the methods do exist.

Not a bug, but a nice to have. Feel free to close if we consider its a limitation of the language, which is acceptable.

[ysbaddaden]

A workaround is to replace the abstract def with an method that raises in a macro, with a bunch of macro-fu to detect which types are actually missing, to report a helpful message:

module Foo(T)
  def call(x : T)
    {% if T.union? %}
      {% types = [] of String %}

      {% for t in T.union_types %}
        {% for tt in t.stringify.split(" | ") %}
          {% types << tt %}
        {% end %}
      {% end %}

      {% for fn in @type.methods %}
        {% if fn.name == "call" && fn.args.size >= 1 %}
          {% types = types.reject do |type|
            fn.args[0].restriction.stringify.split(" | ").includes?(type)
          end %}
        {% end %}
      {% end %}

      {% for ancestor in @type.ancestors %}
        {% for fn in ancestor.methods %}
          {% if fn.name == "call" && fn.args.size >= 1 %}
            {% types = types.reject do |type|
              fn.args[0].restriction.stringify.split(" | ").includes?(type)
            end %}
          {% end %}
        {% end %}
      {% end %}

      {% raise "Error: method #{@type}#call(#{types.join(" | ").id}) must be defined" %}
    {% else %}
      {% raise "Error: method #{@type}#call(#{T}) must be defined" %}
    {% end %}
  end
end

class Bar
  include Foo(Int32 | String | Float64)

  def call(x : Int32 | Float64)
  end

  # def call(x : String)
  # end
end

bar = Bar.new
bar.call(2)       # => ok
bar.call(1.0)     # => ok
bar.call("str")   # => Error: method Bar#call(String) must be implemented

[asterite]

This is really hard to implement.

I think I'd prefer to remove abstract methods from the language. They have no use, you still get a compile error if you don't implement the necessary methods (via duck typing).

[bcardiff]

I think we can frame it for now as a known limitation.

From an implementation naïve perspective the abstract methods can be defined as a contract where a call with those arguments needs to be typable/instantiable. So, for all concrete types at least the compiler could check if those calls might work. This would allow unions to be used. A limitation for this is that every argument of the abstract methods would need to be typed maybe.

I understand that checking beforehand is harder.

[ysbaddaden]

@asterite I agree, thought abstract methods are useful for documentation purposes, as a place to describe a method that a descendant type must implement. For example in Earl to explicit that Agents must implement a #call method.

Maybe we don't need a formal validation that the methods are indeed implemented. In fact, it worked perfectly in Earl before you fixed the abstract checks when generics are involved! Not doing any validation, and letting the compiler fail when the method doesn't exist (with a hint about the abstract def) probably fits better into the language.

Furthermore, the formal validation of an abstract return type forced me to remove the Nil return type signature of the #call methods, since it would force all agents to explicitly set their return type as Nil... something that simple overrides don't have to!

So, yeah: I agree with removing the formal validation of abstract methods but would like to keep 'em for documentation and maybe to improve error reporting.

[RX14]

Surely it'd be fairly simple to implement a naive version which just checks there's an overload for each type in the union?

The validation should stay, I am confident in the ability for even harder things to be implemented in the crystal compiler.

[ysbaddaden]

I believe more and more that the formal validation of abstract methods doesn't make sense in Crystal. We never enforce types: they're a restriction, ,ever a certain truth. I can override a method x : String with a method x : Int32 in a descendant, and Crystal will only complain when usages become wrong.

Why should abstract methods be any different?

In fact, it's easy to break. For example, this fails to compile:

abstract class Foo
  abstract def x : Nil
end

class Bar < Foo
  def x : Int32
  end
end

p Bar.new.x

But the following compiles, despite obviously breaking the contract:

abstract class Foo
  abstract def x : Nil
end

class Bar < Foo
  def x : Nil # ok: fulfills the contract
  end
end

class Baz < Bar
  def x : Int32 # ok: simple override (oops)
    2
  end
end

p Baz.new.x

We can consider it a loophole, that it should be patched... Yet, why bother doing a formal verification when Crystal will detect issues, anyway? That's doing the work twice.

I used to believe the validation was nice, but the recent changes show me they're mostly a hindrance. I can't type the call method of Earl agents as Nil for documentation purposes, because it forces all implementations to mark the method as Nil. This ain't nice. I had to write an awful macro-fu to keep the nice error messages, and so on.

I believe that abstract methods should be about declaring that a method is expected to exist, and should have this signature, so it's documented and can help improve error messages, but nothing more.

[RX14]

We never enforce types: they're a restriction, ,ever a certain truth.

This is something which crystal should change. If people want to play loose with types, they can just choose not to restrict them. If the language is to move forwards, type restrictions need to become more than just restrictions, so that the compiler can use them as useful information. This is not to say they become mandatory, but where they are used they must be more than documentation.

[HertzDevil]

There is also the following example where no variance occurs at all between the restrictions:

class A; end
class B; end
class C; end
class D; end

abstract class Foo
  abstract def f(x : A | C)
  abstract def f(x : B | D)
end

# should be okay, Bar#f covers Foo#f
class Bar < Foo
  def f(x : A | B); end
  def f(x : C | D); end
end

We do exhaustiveness checks for case in statements already, so this might be doable if we factor out Crystal::ExhaustivenessChecker. And maybe Crystal::Cover too. (Case statements don't support all kinds of predicates, but they do at least support unions.)