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

Using named arguments breaks overloading for union types #4465

Closed
billiam105 opened this issue May 26, 2017 · 1 comment
Closed

Using named arguments breaks overloading for union types #4465

billiam105 opened this issue May 26, 2017 · 1 comment

Comments

@billiam105
Copy link

billiam105 commented May 26, 2017

I have two methods, each of which have the same name and number of arguments, but are annotated with different types. Generally this works well, but I found the following issue with union types:

  def do_something(value : Int32)
    value + 1 # something not defined for String
  end

  def do_something(value : String)
    value.split # something not defined for Int32
  end

  do_something 7.as(Int32 | String) # => 8
  do_something "7".as(Int32 | String) # => ["7"]

This works as written, but changing the calls to do_something value: 7.as(Int32 | String) causes the compiler to raise an error:

no overload matches 'String#+' with type Int32

This occurs in crystal 0.22.0, and the two versions can be seen here for the working version and here for the erroring version.

Please let me know if I can provide any more information.

@faultyserver
Copy link

Also interesting:

def do_something(value : Int32 | String)
  puts "hello"
end

def do_something(value : String)
  puts value.split
end


do_something value: 7.as(Int32 | String)
do_something value: "7".as(Int32 | String)

yields

Error in main.cr:10: instantiating 'do_something()'

do_something value: 7.as(Int32 | String) # => 8
^~~~~~~~~~~~

in main.cr:6: undefined method 'split' for Int32 (compile-time type is (Int32 | String))

  puts value.split

I would've expected this to at least use the first definition (with the Int32 | String restriction), but definitely didn't expect it to attempt the second.

Shuffling the order of the definitions also had no effect.

@asterite asterite added this to the Next milestone Jun 7, 2017
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

3 participants