-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Function calls: keyword arguments are collected and passed via a new `Keywords` object: julia> f(kw::Keywords, args...) = (args, kw) # method added to generic function f julia> f(1, a=3, 2, b=4) ((1,2),Keywords(a=3, b=4)) Method definitions: here, a keyword argument creates a new local variable whose value is overridden if called with a keyword of the same name: julia> g(kw::Keywords, args..., x=0) = (args, kw, x) # method added to generic function g julia> g(1, a=3, 2, b=4) ((1,2),Keywords(a=3, b=4),0) julia> g(1, a=3, 2, b=4, x=7) ((1,2),Keywords(a=3, b=4, x=7),7) Internally, function g(kw::Keywords, a, b, x=0) ... end get converted to function g(kw::Keywords, a, b) x = get(kw, :x, 0) ... end Methods lacking the `::Keywords` catch-all only accept their specific keywords: julia> h(args..., x=0) = (args, x) # method added to generic function h julia> h(1, 2, x=7) ((1,2),7) julia> h(1, a=3, 2, x=7) ERROR: unrecognized keyword a in h at no file
- Loading branch information
Showing
5 changed files
with
75 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
|
||
type Keywords <: Associative{Symbol,Any} | ||
w::Dict{Symbol,Any} | ||
end | ||
Keywords(ks::(Symbol...), vs::(Any...)) = Keywords(Dict{Symbol,Any}(ks,vs)) | ||
Keywords() = Keywords(Dict{Symbol,Any}()) | ||
|
||
keywords(x) = convert(Keywords, x) | ||
convert(::Type{Keywords}, kw::Keywords) = kw | ||
convert(::Type{Keywords}, d::Dict{Symbol,Any}) = Keywords(d) | ||
convert(::Type{Keywords}, d::Dict) = | ||
Keywords((Symbol=>Any)[symbol(k) => v for (k,v) in d]) | ||
|
||
length(k::Keywords) = length(k.w) | ||
start(k::Keywords) = start(k.w) | ||
done(k::Keywords, i) = done(k.w, i) | ||
next(k::Keywords, i) = next(k.w, i) | ||
|
||
get(k::Keywords, s::Symbol, default::ANY) = | ||
has(k.w,s) ? ref(k.w,s) : default | ||
has(k::Keywords, s::Symbol) = has(k.w, s) | ||
ref(k::Keywords, s::Symbol) = ref(k.w, s) | ||
assign(k::Keywords, v::ANY, s::Symbol) = assign(k.w, v, s) | ||
|
||
function show(io::IO, k::Keywords) | ||
print(io, "Keywords(") | ||
first = true | ||
for (a,b) in k.w | ||
first || print(io, ", ") | ||
first = false | ||
print(io, a, '=', b) | ||
end | ||
print(io, ')') | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I played with your branch some. It's great to see your work here. This is my #1 feature request. Here's one thing I was hoping would work but didn't:
f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, should work now.
f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, Nick. Overall, I think this meets most of my needs.
I did do a little performance testing as shown below. Keyword args do slow things down. (This won't affect most of my uses for keyword args.)
f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cool. I was planning to do a version that doesn't use heap allocation, but this might be worth exploring, in the interest of not complicating lots of internals with keywords. On the assumption that the number of keywords will be fairly small, I might try using a simple array like {:a,3,:b,4} instead of a Dict, which should save a bunch of time and space.
We can also use double dispatch to intercept the types of the keyword arguments:
f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's another idea. We could define
and then lower calls as
g(KWCount(2), x, y, :a, 3, :b, 4)
. That way keywords are passed at the end of the normal varargs list, and at least some calls could happen with no allocation. Makes it trickier to deal with varargs however.f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, i'll give this a shot.
f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how
immutable
is going to work, but what about something like:Would they get passed on the stack?
f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it help to put the keyword symbol in the type? I was thinking of something like:
f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My function definitions are messed up, so that might be just a dead end.
f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, and my typedefs don't really work either, so this probably is a dead end. Seemed tantalizingly close, though.
f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The best code I could find that actually works is here, but it's not really that flexible. It seems like we need some sort of look-up, whether it's done something like that below (possibly runs faster) or with a Dict (a lot easier to implement and probably easier for the end user).
f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying my approach of passing a keyword count and keywords as varargs on branch
jb/kwargs
.f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, Jeff!
Here are some issues based on the examples I tried below:
g(x, i::Int = 4)
looks natural for this, but I don't know if it is feasible.f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't gotten to all the features yet. I will let your obvious enthusiasm for keyword args inspire me :)
f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I have addressed your points 1 and 3, and added support for using keywords and varargs together. The next feature is "rest keywords", i.e. collecting all unmatched keywords, and passing a block of keywords with
f(a, va...; keywords...)
.It is great that this approach uses only source-level rewriting. And everything is passed on the stack, so the overhead is similar to normal varargs, and we can leverage existing and future vararg-related optimizations. The big difference, of course, is the keyword matching code. But, that might as well be generated by the front end, since it is much easier that way, and code equivalent to that would have to run somewhere in any keyword arg implementation anyway.
By adding metadata describing keywords to a function's AST, we can gain useful printing of methods, and also the ability to optimize away keyword matching completely (I've isolated matching to its own method, so it is easier to skip). For known function calls, the overhead ought to be zero.
So far I only see one big downside, which is that screwy things happen to functions that don't support keywords, and have an Any-typed first argument:
We could potentially do something drastic like making the types of the keyword-related tags disjoint from the rest of the type hierarchy (not subtypes of
Any
!). Or we could add a check to every loosely typed function that throws an error ifisa(firstarg,NKeywords)
.f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took another crack at keywords, with an eye towards making them as fast as possible. The solution i came up with is a little goofy, splitting keywords into 2 types: fast/static & slow/dynamic. See ee32a01's commit message on mn/kwargs2 for details.
On
mn/kwargs2
@tshort'st1()
test runs about 10x faster thanjb/kwargs
, which in turn is about 25x faster thanmn/kwargs
.f4a2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that approach is going to work, since functions are not always called by their "official" names. It should be possible to pass keywords to function arguments:
It is also a problem that it generates
2^n
method definitions. We will have to recover the performance ofmn/kwargs2
by adding a compiler pass to statically process keyword args when possible. That way each call site is transformed in one of2^n
possible ways, instead of keeping all2^n
around explicitly.