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

fix #17785, process all keyword args left-to-right #17792

Merged
merged 1 commit into from
Aug 9, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ This section lists changes that do not have deprecation warnings.

* Operations between `Float16` and `Integers` now return `Float16` instead of `Float32`. ([#17261])

* Keyword arguments are processed left-to-right: if the same keyword is specified more than
once, the rightmost occurrence takes precedence ([#17785]).

Library improvements
--------------------

Expand Down Expand Up @@ -616,6 +619,7 @@ Language tooling improvements
[#17037]: https://github.com/JuliaLang/julia/issues/17037
[#17075]: https://github.com/JuliaLang/julia/issues/17075
[#17132]: https://github.com/JuliaLang/julia/issues/17132
[#17261]: https://github.com/JuliaLang/julia/issues/17261
[#17266]: https://github.com/JuliaLang/julia/issues/17266
[#17300]: https://github.com/JuliaLang/julia/issues/17300
[#17323]: https://github.com/JuliaLang/julia/issues/17323
Expand All @@ -626,3 +630,4 @@ Language tooling improvements
[#17510]: https://github.com/JuliaLang/julia/issues/17510
[#17546]: https://github.com/JuliaLang/julia/issues/17546
[#17668]: https://github.com/JuliaLang/julia/issues/17668
[#17785]: https://github.com/JuliaLang/julia/issues/17785
7 changes: 7 additions & 0 deletions doc/manual/functions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,13 @@ tuple, explicitly after a semicolon. For example, ``plot(x, y;
``plot(x, y, width=2)``. This is useful in situations where the
keyword name is computed at runtime.

The nature of keyword arguments makes it possible to specify the same
argument more than once. For example, in the call
``plot(x, y; options..., width=2)`` it is possible that the ``options``
structure also contains a value for ``width``. In such a case the
rightmost occurrence takes precedence; in this example, ``width``
is certain to have the value ``2``.

.. _man-evaluation-scope-default-values:

Evaluation Scope of Default Values
Expand Down
97 changes: 54 additions & 43 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -1358,49 +1358,60 @@

;; lower function call containing keyword arguments
(define (lower-kw-call f kw pa)
(if (any (lambda (x) (and (pair? x) (eq? (car x) 'parameters)))
kw)
(error "more than one semicolon in argument list"))
(receive
(keys restkeys) (separate kwarg? kw)
(let ((keyargs (apply append
(map (lambda (a)
(if (not (symbol? (cadr a)))
(error (string "keyword argument is not a symbol: \""
(deparse (cadr a)) "\"")))
(if (vararg? (caddr a))
(error "splicing with \"...\" cannot be used for a keyword argument value"))
`((quote ,(cadr a)) ,(caddr a)))
keys))))
(if (null? restkeys)
`(call (call (core kwfunc) ,f) (call (top vector_any) ,@keyargs) ,f ,@pa)
(let ((container (make-ssavalue)))
`(block
(= ,container (call (top vector_any) ,@keyargs))
,@(map (lambda (rk)
(let* ((k (make-ssavalue))
(v (make-ssavalue))
(push-expr `(ccall 'jl_array_ptr_1d_push2 Void
(tuple Any Any Any)
,container
(|::| ,k (core Symbol))
,v)))
(if (vararg? rk)
`(for (= (tuple ,k ,v) ,(cadr rk))
,push-expr)
`(block (= (tuple ,k ,v) ,rk)
,push-expr))))
restkeys)
,(if (not (null? keys))
`(call (call (core kwfunc) ,f) ,container ,f ,@pa)
(let* ((expr_stmts (remove-argument-side-effects `(call ,f ,@pa)))
(pa (cddr (car expr_stmts)))
(stmts (cdr expr_stmts)))
`(block
,@stmts
(if (call (top isempty) ,container)
(call ,f ,@pa)
(call (call (core kwfunc) ,f) ,container ,f ,@pa)))))))))))
(let ((container (make-ssavalue)))
(let loop ((kw kw)
(initial-kw '()) ;; keyword args before any splats
(stmts '())
(has-kw #f)) ;; whether there are definitely >0 kwargs
(if (null? kw)
(if (null? stmts)
`(call (call (core kwfunc) ,f) (call (top vector_any) ,@(reverse initial-kw)) ,f ,@pa)
`(block
(= ,container (call (top vector_any) ,@(reverse initial-kw)))
,@(reverse stmts)
,(if has-kw
`(call (call (core kwfunc) ,f) ,container ,f ,@pa)
(let* ((expr_stmts (remove-argument-side-effects `(call ,f ,@pa)))
(pa (cddr (car expr_stmts)))
(stmts (cdr expr_stmts)))
`(block
,@stmts
(if (call (top isempty) ,container)
(call ,f ,@pa)
(call (call (core kwfunc) ,f) ,container ,f ,@pa)))))))
(let ((arg (car kw)))
(cond ((and (pair? arg) (eq? (car arg) 'parameters))
(error "more than one semicolon in argument list"))
((kwarg? arg)
(if (not (symbol? (cadr arg)))
(error (string "keyword argument is not a symbol: \""
(deparse (cadr arg)) "\"")))
(if (vararg? (caddr arg))
(error "splicing with \"...\" cannot be used for a keyword argument value"))
(if (null? stmts)
(loop (cdr kw) (list* (caddr arg) `(quote ,(cadr arg)) initial-kw) stmts #t)
(loop (cdr kw) initial-kw
(cons `(ccall 'jl_array_ptr_1d_push2 Void (tuple Any Any Any)
,container
(|::| (quote ,(cadr arg)) (core Symbol))
,(caddr arg))
stmts)
#t)))
(else
(loop (cdr kw) initial-kw
(cons (let* ((k (make-ssavalue))
(v (make-ssavalue))
(push-expr `(ccall 'jl_array_ptr_1d_push2 Void (tuple Any Any Any)
,container
(|::| ,k (core Symbol))
,v)))
(if (vararg? arg)
`(for (= (tuple ,k ,v) ,(cadr arg))
,push-expr)
`(block (= (tuple ,k ,v) ,arg)
,push-expr)))
stmts)
(or has-kw (not (vararg? arg)))))))))))

;; convert e.g. A'*B to Ac_mul_B(A,B)
(define (expand-transposed-op e ops)
Expand Down
9 changes: 9 additions & 0 deletions test/keywordargs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,12 @@ end
@test f9948(x=5) == 5
@test_throws UndefVarError f9948()
@test getx9948() == 3

# issue #17785 - handle all sources of kwargs left-to-right
g17785(; a=1, b=2) = (a, b)
let opts = (:a=>3, :b=>4)
@test g17785(; a=5, opts...) == (3, 4)
@test g17785(; opts..., a=5) == (5, 4)
@test g17785(; opts..., a=5, b=6) == (5, 6)
@test g17785(; b=0, opts..., a=5) == (5, 4)
end