Crystal: Precedence of overloaders with metaclass arguments

[vladfaust]

Code example:

_https://carc.in/#/r/672i_

def foo(x : Class)
  puts "1"
end

def foo(x : Int32.class)
  puts "2"
end

foo(Int32)

Expected:

2

Got:

1

I'm not sure if this a bug, I may be doing it wrong. It also could be affected by https://github.com/crystal-lang/crystal/pull/7255, please point me if it is.

[Sija]

Changing the order of definitions yields expected result.

[vladfaust]

Of course it does, @Sija.

My use-case:

generic_sql_shard/ext/uuid.cr (must be required early, because the ones below depend on it):

class UUID
  def to_db(db : Class)
    {% raise "Could not cast UUID to a generic DB value. Use specific adapter instead" %}
  end
end

And then in sqlite_shard/ext/uuid.cr:

class UUID
  def to_db(db : SQLite3.class)
    to_slice
  end
end

And in postgresql_shard/ext/uuid.cr:

class UUID
  def to_db(db : PG.class)
    to_s
  end
end

Please don't point me on "UUID can be stored as Bytes in PG as well", it's just an example and I have more types which are cast differently to different databases.

_Update_: I could have just not defined def to_db(db : Class) in the first place, but I'd like to catch the error and print a sensible message.

[bcardiff]

Another workaround would be to not put a type restriction in the first definition

def foo(x)
  puts "1"
end

def foo(x : Int32.class)
  puts "2"
end

foo(Int32) => 2

[vladfaust]

@bcardiff yes! Thanks. This is what I have now:

def to_db(db)
  {% raise "Cannot automatically cast UUID to a generic SQL value. An appropriate extension is required (e.g. \"sqlite_shard/ext/uuid\")" %}
end

Works good, the next step is to display the db argument like this:

def to_db(db : T.class) forall T
  {% raise "Cannot automatically cast UUID to a #{T} value. An appropriate extension is required (e.g. \"#{T.underscore}_shard/ext/uuid\")" %}
end

require "pg"
# require "pg_shard/ext/uuid" # Accidentally forgot to require
uuid.to_db(PG)

# Note: the ../ext/uuid must be required explicitly, because stdlib's UUID is required explicitly as well

And we're moving to https://github.com/crystal-lang/crystal/issues/6701 :slightly_smiling_face:

However, I have a feeling that these two issues are somehow related :thinking:

[asterite]

As a general note, the compiler will try its best to figure out which overload is stricter than order, but it's not guaranteed to always get it right. So the best thing to do is to order overloads the way you want the compiler to traverse them.

[vladfaust]

@asterite I don't get this line:

So the best thing to do is to order overloads the way you want the compiler to traverse them.

Doesn't the compiler look up from the last overload up to the original definition?

[asterite]

No, they are stored in order if the compiler can't figure out which one should come first.

[vladfaust]

So, in this case the compiler thinks that Int32.class is less strict than Class. Is it wrong?

[asterite]

the compiler thinks that Int32.class is less strict than Class

the compiler is _missing_ that logic. There's like a thousand checks to be done in overloads (classes, generic types, modules, inheritance, inclusion, splats, named arguments, etc.) that it's really hard to implement (or even define) how this should work. So the compiler probably can't compare Int32.class with Class to decide which one is stricter. Then it tries to match Int32 in the call with each overloads in order, starting from the first one, then the second one, etc. The first one matches (Int32 matches Class) then it uses that.

[bew]

Note: The real use-case will be fixed by #7536, is it worth keeping this issue open (to maybe fix one-day) after that?