crystal icon indicating copy to clipboard operation
crystal copied to clipboard

Don't count failed signature matches as autocast matches

Open HertzDevil opened this issue 1 year ago • 0 comments

Supersedes #10701. The following description is adopted from that PR.

Consider the following:

def foo(x : Int8, y : Char); end
def foo(x : UInt8, y : String); end

foo(1, 'a')       # Error: ambiguous call, implicit cast of 1 matches all of Int8, UInt8
foo(y: 'a', x: 1) # okay

def bar(x : Char, y : Int8); end
def bar(x : String, y : UInt8); end

bar('a', 1)       # okay
bar(y: 1, x: 'a') # Error: ambiguous call, implicit cast of 1 matches all of Int8, UInt8

Each overload set is checked in that order, because neither overload is more restricted than the other. What happens here is:

  • Neither overload matches when autocasting is disabled.
  • Method lookup is repeated with autocasting enabled, and proceeds to check argument compatibility following argument order.
  • 1 successfully matches Int8 because 1 is within Int8's range. The compiler adds this to a list of partial autocast matches.
  • 'a' successfully matches Char, so there is a successful signature match. But the compiler must also check other defs to detect ambiguous autocasts.
  • 1 successfully matches UInt8 because 1 is within UInt8's range. The partial autocast match list now contains both integer types.
  • 'a' does not match String, so this signature match fails, but the compiler does not reset the list of partial autocast matches. Thus 1 is considered to be ambiguous, even though the UInt8 autocast match is associated with a call that fails.

This PR makes it so that autocast matches are added only after a signature match succeeds completely (with AutocastType#add_autocast_matches). Thus the autocasting behavior is no longer dependent on the argument order, and both error calls above become unambiguous:

def foo(x : Int8, y : Char); end
def foo(x : UInt8, y : String); end

foo(1, 'a') # okay, matches first overload

def bar(x : Char, y : Int8); end
def bar(x : String, y : UInt8); end

bar('a', 1) # okay, matches first overload

Does not fix https://github.com/crystal-lang/crystal/pull/8600#pullrequestreview-334682026, since in that case all signature matches succeed.

Note that splat restrictions never autocast:

def foo(*x : *{Int8}); end

foo(1) # Error: no overload matches

So two of the calls to Crystal::Type#restrict inside Crystal::CallSignature#match are not associated with any autocast checks.

HertzDevil avatar Jul 30 '22 17:07 HertzDevil