Skip to content

Commit

Permalink
fix JuliaLang#17785, process all keyword args left-to-right
Browse files Browse the repository at this point in the history 
Whatever comes later, whether splatted or not, takes precedence.
  • Loading branch information
@JeffBezanson @mfasi
JeffBezanson authored and mfasi committed Sep 5, 2016
1 parent c724db6 commit 7ebcab5
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 43 deletions.
5 changes: 5 additions & 0 deletions NEWS.md
View file
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
View file
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
View file
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
View file
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

0 comments on commit 7ebcab5

Please sign in to comment.