Merge pull request #17792 from JuliaLang/jb/kwargs_LtoR

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

NEWS.md

@@ -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
 --------------------
 
@@ -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
@@ -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

doc/manual/functions.rst

@@ -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

src/julia-syntax.scm

@@ -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)

test/keywordargs.jl

@@ -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