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.

Sign up for GitHub

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 #17240, evaluate keyword argument defaults in successive scopes #22958

Merged
merged 1 commit into from
Jul 26, 2017
Merged

fix #17240, evaluate keyword argument defaults in successive scopes #22958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ Language changes
* Declaring arguments as `x::ANY` to avoid specialization has been replaced
by `@nospecialize x`. ([#22666]).

* Keyword argument default values are now evaluated in successive scopes ---
the scope for each expression includes only previous keyword arguments, in
left-to-right order ([#17240]).

* The parsing of `1<<2*3` as `1<<(2*3)` is deprecated, and will change to
`(1<<2)*3` in a future version ([#13079]).

Expand Down
11 changes: 3 additions & 8 deletions doc/src/manual/functions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,8 @@ this example, `width` is certain to have the value `2`.

## Evaluation Scope of Default Values

Optional and keyword arguments differ slightly in how their default values are evaluated. When
optional argument default expressions are evaluated, only *previous* arguments are in scope. In
contrast, *all* the arguments are in scope when keyword arguments default expressions are evaluated.
When optional and keyword argument default expressions are evaluated, only *previous* arguments are in
scope.
For example, given this definition:

```julia
Expand All @@ -483,11 +482,7 @@ function f(x, a=b, b=1)
end
```

the `b` in `a=b` refers to a `b` in an outer scope, not the subsequent argument `b`. However,
if `a` and `b` were keyword arguments instead, then both would be created in the same scope and
the `b` in `a=b` would refer to the subsequent argument `b` (shadowing any `b` in an outer scope),
which would result in an undefined variable error (since the default expressions are evaluated
left-to-right, and `b` has not been assigned yet).
the `b` in `a=b` refers to a `b` in an outer scope, not the subsequent argument `b`.

## Do-Block Syntax for Function Arguments

Expand Down
92 changes: 42 additions & 50 deletions src/julia-syntax.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,12 +367,13 @@
`(block (method ,name) ,mdef (unnecessary ,name)) ;; return the function
mdef)))))

;; keyword default values that can be assigned right away. however, this creates
;; a quasi-bug (part of issue #9535) where it can be hard to predict when a
;; keyword argument will throw an UndefVarError.
(define (const-default? x)
(or (number? x) (string? x) (char? x) (and (pair? x) (memq (car x) '(quote inert)))
(eq? x 'true) (eq? x 'false)))
;; wrap expr in nested scopes assigning names to vals
(define (scopenest names vals expr)
(if (null? names)
expr
`(let (block
,(scopenest (cdr names) (cdr vals) expr))
(= ,(car names) ,(car vals)))))

(define empty-vector-any '(call (core AnyVector) 0))

Expand Down Expand Up @@ -434,24 +435,23 @@
(rkw (if (null? restkw) '() (symbol (string (car restkw) "..."))))
(mangled (symbol (string "#" (if name (undot-name name) 'call) "#"
(string (current-julia-module-counter)))))
(flags (map (lambda (x) (gensy)) vals)))
(tempnames (map (lambda (x) (gensy)) keynames)))
`(block
;; call with no keyword args
,(method-def-expr-
name positional-sparams (append pargl vararg)
`(block
,@prologue
,@(if (not ordered-defaults)
'()
(append! (map (lambda (kwname) `(local ,kwname)) keynames)
(map make-assignment keynames vals)))
;; call mangled(vals..., [rest_kw,] pargs..., [vararg]...)
(return (call ,mangled
,@(if ordered-defaults keynames vals)
,@(if (null? restkw) '() (list empty-vector-any))
,@(map arg-name pargl)
,@(if (null? vararg) '()
(list `(... ,(arg-name (car vararg))))))))
,(let (;; call mangled(vals..., [rest_kw,] pargs..., [vararg]...)
(ret `(return (call ,mangled
,@(if ordered-defaults keynames vals)
,@(if (null? restkw) '() (list empty-vector-any))
,@(map arg-name pargl)
,@(if (null? vararg) '()
(list `(... ,(arg-name (car vararg)))))))))
(if ordered-defaults
(scopenest keynames vals ret)
ret)))
#f)

;; call with keyword args pre-sorted - original method code goes here
Expand Down Expand Up @@ -483,14 +483,9 @@
,(if (any kwarg? pargl) (gensy) UNUSED)
(call (core kwftype) ,ftype)) (:: ,kw (core AnyVector)) ,@pargl ,@vararg)
`(block
;; initialize keyword args to their defaults, or set a flag telling
;; whether this keyword needs to be set.
,@(map (lambda (kwname) `(local ,kwname)) keynames)
,@(map (lambda (name dflt flag)
(if (const-default? dflt)
`(= ,name ,dflt)
`(= ,flag true)))
keynames vals flags)
;; temp variables that will be assigned if their corresponding keywords are passed.
;; `isdefined` is then used to check whether default values should be evaluated.
,@(map (lambda (v) `(local ,v)) tempnames)
,@(if (null? restkw) '()
`((= ,rkw ,empty-vector-any)))
;; for i = 1:(length(kw)>>1)
Expand All @@ -499,8 +494,8 @@
;; ii = i*2 - 1
(= ,ii (call (top -) (call (top *) ,i 2) 1))
(= ,elt (call (core arrayref) ,kw ,ii))
,(foldl (lambda (kvf else)
(let* ((k (car kvf))
,(foldl (lambda (kn else)
(let* ((k (car kn))
(rval0 `(call (core arrayref) ,kw
(call (top +) ,ii 1)))
;; note: if the "declared" type of a KW arg
Expand Down Expand Up @@ -528,13 +523,9 @@
,T)
T)))
rval0)))
;; if kw[ii] == 'k; k = kw[ii+1]::Type; end
`(if (comparison ,elt === (quote ,(decl-var k)))
(block
(= ,(decl-var k) ,rval)
,@(if (not (const-default? (cadr kvf)))
`((= ,(caddr kvf) false))
'()))
;; if kw[ii] == 'k; k_temp = kw[ii+1]::Type; end
`(if (comparison ,elt === (quote ,(cdr kn)))
(= ,(decl-var k) ,rval)
,else)))
(if (null? restkw)
;; if no rest kw, give error for unrecognized
Expand All @@ -547,21 +538,22 @@
,rkw (tuple ,elt
(call (core arrayref) ,kw
(call (top +) ,ii 1)))))
(map list vars vals flags))))
(map (lambda (k temp)
(cons (if (decl? k) `(,(car k) ,temp ,(caddr k)) temp)
(decl-var k)))
vars tempnames))))
;; set keywords that weren't present to their default values
,@(apply append
(map (lambda (name dflt flag)
(if (const-default? dflt)
'()
`((if ,flag (= ,name ,dflt)))))
keynames vals flags))
;; finally, call the core function
(return (call ,mangled
,@keynames
,@(if (null? restkw) '() (list rkw))
,@(map arg-name pargl)
,@(if (null? vararg) '()
(list `(... ,(arg-name (car vararg))))))))
,(scopenest keynames
(map (lambda (v dflt) `(if (isdefined ,v)
,v
,dflt))
tempnames vals)
`(return (call ,mangled ;; finally, call the core function
,@keynames
,@(if (null? restkw) '() (list rkw))
,@(map arg-name pargl)
,@(if (null? vararg) '()
(list `(... ,(arg-name (car vararg)))))))))
#f)
;; return primary function
,(if (not (symbol? name))
Expand Down Expand Up @@ -2590,7 +2582,7 @@
glob)
(if lam ;; update in-place the list of local variables in lam
(set-car! (cddr lam)
(append! (caddr lam) real-new-vars real-new-vars-def)))
(append real-new-vars real-new-vars-def (caddr lam))))
(insert-after-meta ;; return the new, expanded scope-block
(if (and (pair? body) (eq? (car body) 'block))
body
Expand Down
13 changes: 11 additions & 2 deletions test/keywordargs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,10 @@ f9948, getx9948 = let
getx() = x
h, getx
end
@test_throws UndefVarError f9948()
@test f9948() == 3
@test getx9948() == 3
@test f9948(x=5) == 5
@test_throws UndefVarError f9948()
@test f9948() == 3
@test getx9948() == 3

# issue #17785 - handle all sources of kwargs left-to-right
Expand Down Expand Up @@ -285,3 +285,12 @@ let a = 0
g21518()(; :kw=>1)
@test a == 2
end

# issue #17240 - evaluate default expressions in nested scopes
let a = 10
f17240(;a=a-1, b=2a) = (a, b)
@test f17240() == (9, 18)
@test f17240(a=2) == (2, 4)
@test f17240(b=3) == (9, 3)
@test f17240(a=2, b=1) == (2, 1)
end