Skip to content

Commit

Permalink
Only evaluate fexpr in fexpr(kw=...) once (#21519)
Browse files Browse the repository at this point in the history
* Only evaluate `fexpr` in `fexpr(kw=...)` once

Lower e.g. `get_inner()(kw=1)` to
```
SSAValue(n) = get_inner()
((Core.kwfunc)(SSAValue(n)))((Base.vector_any)(:kw, 1), SSAValue(n))
```
instead of
```
((Core.kwfunc)(get_inner()))((Base.vector_any)(:kw, 1), get_inner())
```
as the latter would call `get_inner` twice.

Fixes #21518.
  • Loading branch information
martinholters authored and JeffBezanson committed Apr 27, 2017
1 parent 67fdcf2 commit 2d803f3
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 4 deletions.
11 changes: 7 additions & 4 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -1425,14 +1425,17 @@
(reverse a))))))

;; lower function call containing keyword arguments
(define (lower-kw-call f kw pa)
(define (lower-kw-call fexpr kw 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)
(let ((f (if (sym-ref? fexpr) fexpr (make-ssavalue))))
`(block
,@(if (eq? f fexpr) '() `((= ,f, fexpr)))
,(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)))
Expand All @@ -1446,8 +1449,8 @@
,@stmts
(if (call (top isempty) ,container)
(call ,f ,@pa)
(call (call (core kwfunc) ,f) ,container ,f ,@pa)))))))
(let ((arg (car kw)))
(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)
Expand Down
10 changes: 10 additions & 0 deletions test/keywordargs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -275,3 +275,13 @@ end
f21510(; a::ANY = 2) = a
@test f21510(a=:b) == :b
@test f21510() == 2

# issue #21518
let a = 0
f21518(;kw=nothing) = kw
g21518() = (a+=1; f21518)
g21518()(kw=1)
@test a == 1
g21518()(; :kw=>1)
@test a == 2
end

0 comments on commit 2d803f3

Please sign in to comment.