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

Keyword arguments affect methods dispatch #9498

Open
yuyichao opened this issue Dec 30, 2014 · 8 comments

Comments

@yuyichao
Copy link
Contributor

commented Dec 30, 2014

So this is an issue that I find after figuring out how the keyword arguments are currently implemented in Julia. It will probably not be an issue anymore if #2773 is implemented.

The document on methods says

Methods are dispatched based only on positional arguments, with keyword arguments processed after the matching method is identified.

However, method dispatch actually behaves differently when doing a function call with or without keyword arguments. i.e.

julia> function f(::Integer)
       2
       end
f (generic function with 1 method)

julia> function f(::Number; kw...)
       1
       end
f (generic function with 2 methods)

julia> f(1)
2

julia> f(1; a = 2)
1

What happens here is that f.env.kwsorter only has one method defined and therefore when calling with keyword argument, f(::Integer) does not participate in method dispatch.

IMHO, there are several possible ways to fix it,

  1. Fix the document to include this behavior. This should be the easiest fix but will probably make the whole keyword argument/optional argument/multiple dispatch more confusing especially for someone who does not know how it all works behind the scene. (It's already quite confusing/surprising that anonymous function does not support keyword argument for someone (like me) that expects python-like keyword argument implementation.)

  2. Having an entry (that just throw an error) in env.kwsorter even for methods that does not take keyword arguments. This can also avoid the following confusing abuse of overriding method

    julia> function f(::Number; kw...)
           1
           end
    f (generic function with 1 method)
    
    julia> function f(::Number)
           2
           end
    f (generic function with 1 method)
    
    julia> f(1)
    2
    
    julia> f(1; a = 2)
    1

    This is probably the easiest way to fix the code and is consistent with the best long term behavior.

  3. Fix #2773 and let the method themselves handle keyword argument dispatch. Since #2773 is on 1.0 milestone, hopefully this will be eventually be implemented.

@vtjnash

This comment has been minimized.

Copy link
Member

commented Mar 27, 2016

see also #4469 (comment)

@JeffBezanson

This comment has been minimized.

Copy link
Member

commented Jul 18, 2017

I think we should add keyword arguments to the calling convention, replacing the existing kwsorter-based implementation.

Currently, defining a keyword method actually defines three methods:

  1. A method with a hidden name accepting all positional and keyword arguments, containing the full method code.
  2. A method for the function actually being defined, accepting only positional arguments and passing default keyword arg values to method (1).
  3. A method of the function's kwsorter function that does sorting and calls method (1).

My proposal merges methods (2) and (3):

  1. Keep method (1).
  2. Add a method like the existing method (2), but that also contains an Expr(:kwargs) to access a keyword argument container passed as a pointer via the calling convention, and then does sorting.
  3. jl_call_method_internal and its codegen throw an error if no kwarg-accepting entry point is available for a given kwarg call.
  4. Add Expr(:kwcall, ...) that has a slot for a keyword arg container.
  5. When inlining runs on such an expression, it substitutes the passed container for Expr(:kwargs) and will then be able to specialize the sorting code.

It would be possible to get the performance aspect of this just by passing a different kind of container to our existing kwsorter, and changing its lowering to make it easier to specialize. However our current implementation is also very complex, and we'd still need a separate fix for the semantic issue described here. Having 2 methods instead of 3 will also make reflection easier. So I think this redesign should be considered.

@martinholters

This comment has been minimized.

Copy link
Member

commented Nov 2, 2017

This shouldn't have been closed, yet, I guess. Or did I miss something?

@martinholters martinholters reopened this Nov 2, 2017

@JeffBezanson

This comment has been minimized.

Copy link
Member

commented Dec 19, 2017

Will have to punt on this for 1.0.

@sbromberger

This comment has been minimized.

@MrUrq MrUrq referenced this issue Feb 9, 2019

Merged

Smoothing #8

taqtiqa-mark added a commit to taqtiqa-mark/julia that referenced this issue May 14, 2019

Remove alerts. Add issue JuliaLang#9498 and example.
Added an example of issue JuliaLang#9498 - since it is not scheduled to be fixed until Julia 2.0.
Kept the example of ambiguous/last-seen default code being run because that is the core
use case for default values.

taqtiqa-mark added a commit to taqtiqa-mark/julia that referenced this issue May 15, 2019

Remove alerts. Add issue JuliaLang#9498 and example.
Added an example of issue JuliaLang#9498 - since it is not scheduled to be fixed until Julia 2.0.
Kept the example of ambiguous/last-seen default code being run because that is the core
use case for default values.
@DilumAluthge

This comment has been minimized.

Copy link

commented Jun 12, 2019

I think we should add keyword arguments to the calling convention, replacing the existing kwsorter-based implementation

Any chance that this will make it into a 1.x release?

@JeffBezanson

This comment has been minimized.

Copy link
Member

commented Jun 12, 2019

No, it would be a breaking change.

@bkamins

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2019

Just for a reference. The current behavior has the following consequence:

julia> f(x::Int; y=1) = 2
f (generic function with 1 method)

julia> f(x::Int) = 1
f (generic function with 1 method)

julia> f(100)
1

julia> f(100, y=1)
2

but

julia> g(x::Int) = 1
g (generic function with 1 method)

julia> g(x::Int; y=1) = 2
g (generic function with 1 method)

julia> g(100)
2

julia> g(100, y=1)
2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.