Skip to content

Commit c6c3d72

Browse files
JeffBezansonJeffBezanson
authored
more precise types in keyword argument method lowering (#30926)
1 parent e5a7c1f commit c6c3d72

2 files changed

Lines changed: 105 additions & 56 deletions

File tree

‎src/julia-syntax.scm‎

Lines changed: 85 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -434,21 +434,19 @@
434434
(rkw (if (null? restkw) (make-ssavalue) (symbol (string (car restkw) "..."))))
435435
(mangled (symbol (string "#" (if name (undot-name name) 'call) "#"
436436
(string (current-julia-module-counter))))))
437-
`(block
437+
;; this is a hack: nest these statements inside a call so they get closure
438+
;; converted together, allowing all needed types to be defined before any methods.
439+
`(call (core ifelse) false false (block
440+
;; forward-declare function so its type can occur in the signature of the inner method below
441+
,@(if (or (symbol? name) (globalref? name)) `((method ,name)) '())
442+
438443
;; call with keyword args pre-sorted - original method code goes here
439444
,(method-def-expr-
440445
mangled sparams
441446
`((|::| ,mangled (call (core typeof) ,mangled)) ,@vars ,@restkw
442447
;; strip type off function self argument if not needed for a static param.
443448
;; then it is ok for cl-convert to move this definition above the original def.
444-
,(if (decl? (car not-optional))
445-
(if (any (lambda (sp)
446-
(expr-contains-eq (car sp) (caddr (car not-optional))))
447-
positional-sparams)
448-
(car not-optional)
449-
(decl-var (car not-optional)))
450-
(car not-optional))
451-
,@(cdr not-optional) ,@vararg)
449+
,@not-optional ,@vararg)
452450
(insert-after-meta `(block
453451
,@stmts)
454452
annotations)
@@ -529,7 +527,7 @@
529527
(list `(... ,(arg-name (car vararg)))))))))))
530528
;; return primary function
531529
,(if (not (symbol? name))
532-
'(null) name)))))
530+
'(null) name))))))
533531

534532
;; prologue includes line number node and eventual meta nodes
535533
(define (extract-method-prologue body)
@@ -2765,7 +2763,7 @@ f(x) = yt(x)
27652763

27662764
(define (convert-lambda lam fname interp capt-sp)
27672765
(let ((body (add-box-inits-to-body
2768-
lam (cl-convert (cadddr lam) fname lam (table) #f interp))))
2766+
lam (cl-convert (cadddr lam) fname lam (table) (table) #f interp))))
27692767
`(lambda ,(lam:args lam)
27702768
(,(clear-capture-bits (car (lam:vinfo lam)))
27712769
()
@@ -2828,7 +2826,7 @@ f(x) = yt(x)
28282826
(make-ssavalue)))
28292827
(rhs (if (equal? vt '(core Any))
28302828
rhs1
2831-
(convert-for-type-decl rhs1 (cl-convert vt fname lam #f #f interp))))
2829+
(convert-for-type-decl rhs1 (cl-convert vt fname lam #f #f #f interp))))
28322830
(ex (cond (closed `(call (core setfield!)
28332831
,(if interp
28342832
`($ ,var)
@@ -2851,14 +2849,17 @@ f(x) = yt(x)
28512849
(else
28522850
(error (string "invalid assignment location \"" (deparse var) "\"")))))
28532851

2852+
(define (rename-sig-types ex namemap)
2853+
(pattern-replace
2854+
(pattern-set
2855+
(pattern-lambda (call (core (-/ Typeof)) name)
2856+
(get namemap name __)))
2857+
ex))
2858+
28542859
;; replace leading (function) argument type with `typ`
28552860
(define (fix-function-arg-type te typ iskw namemap type-sp)
28562861
(let* ((typapp (caddr te))
2857-
(types (pattern-replace
2858-
(pattern-set
2859-
(pattern-lambda (call (core (-/ Typeof)) name)
2860-
(get namemap name __)))
2861-
(cddr typapp)))
2862+
(types (rename-sig-types (cddr typapp) namemap))
28622863
(closure-type (if (null? type-sp)
28632864
typ
28642865
`(call (core apply_type) ,typ ,@type-sp)))
@@ -2883,7 +2884,12 @@ f(x) = yt(x)
28832884
(cadr e))
28842885
e))))
28852886
(let ((e2 (lift- e)))
2886-
(cons e2 (apply append (reverse top))))))
2887+
(let ((stmts (apply append (reverse top))))
2888+
;; move all type definitions first
2889+
(receive (structs others)
2890+
(separate (lambda (x) (and (pair? x) (eq? (car x) 'thunk)))
2891+
stmts)
2892+
(cons e2 (append structs others)))))))
28872893

28882894
(define (first-non-meta blk)
28892895
(let loop ((xs (cdr blk)))
@@ -3061,25 +3067,25 @@ f(x) = yt(x)
30613067
(and cv (vinfo:asgn cv) (vinfo:capt cv)))))
30623068

30633069
(define (toplevel-preserving? e)
3064-
(and (pair? e) (memq (car e) '(if block trycatch tryfinally))))
3070+
(and (pair? e) (memq (car e) '(if elseif block trycatch tryfinally))))
30653071

3066-
(define (map-cl-convert exprs fname lam namemap toplevel interp)
3072+
(define (map-cl-convert exprs fname lam namemap defined toplevel interp)
30673073
(if toplevel
30683074
(map (lambda (x)
3069-
(let ((tl (lift-toplevel (cl-convert x fname lam namemap
3075+
(let ((tl (lift-toplevel (cl-convert x fname lam namemap defined
30703076
(and toplevel (toplevel-preserving? x))
30713077
interp))))
30723078
(if (null? (cdr tl))
30733079
(car tl)
30743080
`(block ,@(cdr tl) ,(car tl)))))
30753081
exprs)
3076-
(map (lambda (x) (cl-convert x fname lam namemap #f interp)) exprs)))
3082+
(map (lambda (x) (cl-convert x fname lam namemap defined #f interp)) exprs)))
30773083

3078-
(define (cl-convert e fname lam namemap toplevel interp)
3084+
(define (cl-convert e fname lam namemap defined toplevel interp)
30793085
(if (and (not lam)
30803086
(not (and (pair? e) (memq (car e) '(lambda method macro)))))
30813087
(if (atom? e) e
3082-
(cons (car e) (map-cl-convert (cdr e) fname lam namemap toplevel interp)))
3088+
(cons (car e) (map-cl-convert (cdr e) fname lam namemap defined toplevel interp)))
30833089
(cond
30843090
((symbol? e)
30853091
(define (new-undef-var name)
@@ -3098,7 +3104,7 @@ f(x) = yt(x)
30983104
(val (if (equal? typ '(core Any))
30993105
val
31003106
`(call (core typeassert) ,val
3101-
,(cl-convert typ fname lam namemap toplevel interp)))))
3107+
,(cl-convert typ fname lam namemap defined toplevel interp)))))
31023108
`(block
31033109
,@(if (eq? box access) '() `((= ,access ,box)))
31043110
,undefcheck
@@ -3125,7 +3131,7 @@ f(x) = yt(x)
31253131
((quote top core globalref outerref line break inert module toplevel null meta) e)
31263132
((=)
31273133
(let ((var (cadr e))
3128-
(rhs (cl-convert (caddr e) fname lam namemap toplevel interp)))
3134+
(rhs (cl-convert (caddr e) fname lam namemap defined toplevel interp)))
31293135
(convert-assignment var rhs fname lam interp)))
31303136
((local-def) ;; make new Box for local declaration of defined variable
31313137
(let ((vi (assq (cadr e) (car (lam:vinfo lam)))))
@@ -3174,7 +3180,7 @@ f(x) = yt(x)
31743180
(sp-inits (if (or short (not (eq? (car sig) 'block)))
31753181
'()
31763182
(map-cl-convert (butlast (cdr sig))
3177-
fname lam namemap toplevel interp)))
3183+
fname lam namemap defined toplevel interp)))
31783184
(sig (and sig (if (eq? (car sig) 'block)
31793185
(last sig)
31803186
sig))))
@@ -3191,14 +3197,26 @@ f(x) = yt(x)
31913197
(list-tail (car (lam:vinfo lam2)) (length (lam:args lam2))))
31923198
(lambda-optimize-vars! lam2)))
31933199
(if (not local) ;; not a local function; will not be closure converted to a new type
3194-
(cond (short e)
3200+
(cond (short (if (has? defined (cadr e))
3201+
e
3202+
(begin
3203+
(put! defined (cadr e) #t)
3204+
`(toplevel-butfirst
3205+
;; wrap in toplevel-butfirst so it gets moved higher along with
3206+
;; closure type definitions
3207+
,e
3208+
(thunk (lambda () (() () 0 ()) (block (return ,e))))))))
31953209
((null? cvs)
31963210
`(block
31973211
,@sp-inits
3198-
(method ,name ,(cl-convert sig fname lam namemap toplevel interp)
3212+
(method ,name ,(cl-convert
3213+
;; anonymous functions with keyword args generate global
3214+
;; functions that refer to the type of a local function
3215+
(rename-sig-types sig namemap)
3216+
fname lam namemap defined toplevel interp)
31993217
,(let ((body (add-box-inits-to-body
32003218
lam2
3201-
(cl-convert (cadddr lam2) 'anon lam2 (table) #f interp))))
3219+
(cl-convert (cadddr lam2) 'anon lam2 (table) (table) #f interp))))
32023220
`(lambda ,(cadr lam2)
32033221
(,(clear-capture-bits (car vis))
32043222
,@(cdr vis))
@@ -3209,13 +3227,13 @@ f(x) = yt(x)
32093227
(newlam (compact-and-renumber (linearize (car exprs)) 'none 0)))
32103228
`(toplevel-butfirst
32113229
(block ,@sp-inits
3212-
(method ,name ,(cl-convert sig fname lam namemap toplevel interp)
3230+
(method ,name ,(cl-convert sig fname lam namemap defined toplevel interp)
32133231
,(julia-bq-macro newlam)))
32143232
,@top-stmts))))
32153233

32163234
;; local case - lift to a new type at top level
3217-
(let* ((exists (get namemap name #f))
3218-
(type-name (or exists
3235+
(let* ((exists (get defined name #f))
3236+
(type-name (or (get namemap name #f)
32193237
(and name
32203238
(symbol (string "#" name "#" (current-julia-module-counter))))))
32213239
(alldefs (expr-find-all
@@ -3224,8 +3242,9 @@ f(x) = yt(x)
32243242
(eq? (method-expr-name ex) name)))
32253243
(lam:body lam)
32263244
identity
3227-
(lambda (x) (and (pair? x) (not (eq? (car x) 'lambda))))))
3228-
(all-capt-vars (delete-duplicates
3245+
(lambda (x) (and (pair? x) (not (eq? (car x) 'lambda)))))))
3246+
(and name (put! namemap name type-name))
3247+
(let* ((all-capt-vars (delete-duplicates
32293248
(apply append ;; merge captured vars from all definitions
32303249
cvs
32313250
(map (lambda (methdef)
@@ -3261,8 +3280,10 @@ f(x) = yt(x)
32613280
(memq (car e) '(= method))
32623281
(eq? (cadr e) s)))
32633282
(caddr e))))
3264-
(error (string "local variable " s
3265-
" cannot be used in closure declaration"))
3283+
(if (has? namemap s)
3284+
#f
3285+
(error (string "local variable " s
3286+
" cannot be used in closure declaration")))
32663287
#t)
32673288
#f)))
32683289
(caddr e)
@@ -3306,7 +3327,7 @@ f(x) = yt(x)
33063327
(append (map (lambda (gs tvar)
33073328
(make-assignment gs `(call (core TypeVar) ',tvar (core Any))))
33083329
closure-param-syms closure-param-names)
3309-
`((method #f ,(cl-convert arg-defs fname lam namemap toplevel interp)
3330+
`((method #f ,(cl-convert arg-defs fname lam namemap defined toplevel interp)
33103331
,(convert-lambda lam2
33113332
(if iskw
33123333
(caddr (lam:args lam2))
@@ -3337,23 +3358,26 @@ f(x) = yt(x)
33373358
(filter (lambda (vi)
33383359
(not (memq (car vi) moved-vars)))
33393360
(car (lam:vinfo lam)))))
3340-
`(toplevel-butfirst
3341-
,(if exists
3342-
'(null)
3343-
(convert-assignment name mk-closure fname lam interp))
3344-
,@(if exists
3345-
'()
3346-
(begin (and name (put! namemap name type-name))
3347-
typedef))
3348-
,@(map (lambda (v) `(moved-local ,v)) moved-vars)
3349-
,@sp-inits
3350-
,@mk-method)))))
3361+
(if (or exists (and short (pair? alldefs)))
3362+
`(toplevel-butfirst
3363+
(null)
3364+
,@sp-inits
3365+
,@mk-method)
3366+
(begin
3367+
(put! defined name #t)
3368+
`(toplevel-butfirst
3369+
,(convert-assignment name mk-closure fname lam interp)
3370+
,@typedef
3371+
,@(map (lambda (v) `(moved-local ,v)) moved-vars)
3372+
,@sp-inits
3373+
,@mk-method))))))))
33513374
((lambda) ;; happens inside (thunk ...) and generated function bodies
33523375
(for-each (lambda (vi) (vinfo:set-asgn! vi #t))
33533376
(list-tail (car (lam:vinfo e)) (length (lam:args e))))
33543377
(let ((body (map-cl-convert (cdr (lam:body e)) 'anon
33553378
(lambda-optimize-vars! e)
33563379
(table)
3380+
(table)
33573381
(null? (cadr e)) ;; only toplevel thunks have 0 args
33583382
interp)))
33593383
`(lambda ,(cadr e)
@@ -3362,7 +3386,7 @@ f(x) = yt(x)
33623386
(block ,@body))))
33633387
;; remaining `::` expressions are type assertions
33643388
((|::|)
3365-
(cl-convert `(call (core typeassert) ,@(cdr e)) fname lam namemap toplevel interp))
3389+
(cl-convert `(call (core typeassert) ,@(cdr e)) fname lam namemap defined toplevel interp))
33663390
;; remaining `decl` expressions are only type assertions if the
33673391
;; argument is global or a non-symbol.
33683392
((decl)
@@ -3372,14 +3396,19 @@ f(x) = yt(x)
33723396
(else
33733397
(if (or (symbol? (cadr e)) (and (pair? (cadr e)) (eq? (caadr e) 'outerref)))
33743398
(error "type declarations on global variables are not yet supported"))
3375-
(cl-convert `(call (core typeassert) ,@(cdr e)) fname lam namemap toplevel interp))))
3399+
(cl-convert `(call (core typeassert) ,@(cdr e)) fname lam namemap defined toplevel interp))))
33763400
;; `with-static-parameters` expressions can be removed now; used only by analyze-vars
33773401
((with-static-parameters)
3378-
(cl-convert (cadr e) fname lam namemap toplevel interp))
3379-
(else (cons (car e)
3380-
(map-cl-convert (cdr e) fname lam namemap toplevel interp))))))))
3381-
3382-
(define (closure-convert e) (cl-convert e #f #f #f #f #f))
3402+
(cl-convert (cadr e) fname lam namemap defined toplevel interp))
3403+
(else
3404+
(if (eq? (car e) 'struct_type)
3405+
;; struct_type has the effect of defining a name, so we don't try to
3406+
;; emit a defining (method x) expr.
3407+
(put! defined (cadr e) #t))
3408+
(cons (car e)
3409+
(map-cl-convert (cdr e) fname lam namemap defined toplevel interp))))))))
3410+
3411+
(define (closure-convert e) (cl-convert e #f #f #f #f #f #f))
33833412

33843413
;; pass 5: convert to linear IR
33853414

‎test/syntax.jl‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1812,3 +1812,23 @@ end
18121812
eval(Expr(:const, :_var_30877))
18131813
@test !isdefined(@__MODULE__, :_var_30877)
18141814
@test isconst(@__MODULE__, :_var_30877)
1815+
1816+
# anonymous kw function in value position at top level
1817+
f30926 = function (;k=0)
1818+
k
1819+
end
1820+
@test f30926(k=2) == 2
1821+
1822+
if false
1823+
elseif false
1824+
g30926(x) = 1
1825+
end
1826+
@test !isdefined(@__MODULE__, :g30926)
1827+
1828+
@testset "closure conversion in testsets" begin
1829+
p = (2, 3, 4)
1830+
@test p == (2, 3, 4)
1831+
identity(p)
1832+
allocs = @allocated identity(p)
1833+
@test allocs == 0
1834+
end

0 commit comments

Comments
 (0)