Skip to content

Commit 455f08f

Browse files
committed
more predictable global binding resolution [ci skip]
fix #18933 fix #17387 (for the syntactic case)
1 parent 04d76f3 commit 455f08f

8 files changed

+158
-84
lines changed

base/poll.jl

+1-2
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,7 @@ mutable struct _FDWatcher
145145
end
146146
end
147147

148-
global uvfinalize
149-
function uvfinalize(t::_FDWatcher)
148+
function Base.uvfinalize(t::_FDWatcher)
150149
if t.handle != C_NULL
151150
disassociate_julia_struct(t)
152151
ccall(:jl_close_uv, Void, (Ptr{Void},), t.handle)

src/interpreter.c

-16
Original file line numberDiff line numberDiff line change
@@ -319,22 +319,6 @@ static jl_value_t *eval(jl_value_t *e, interpreter_state *s)
319319
jl_declare_constant(b);
320320
return (jl_value_t*)jl_nothing;
321321
}
322-
else if (ex->head == global_sym) {
323-
// create uninitialized mutable binding for "global x" decl
324-
// TODO: handle type decls
325-
size_t i, l = jl_array_len(ex->args);
326-
for (i = 0; i < l; i++) {
327-
jl_sym_t *gsym = (jl_sym_t*)args[i];
328-
jl_module_t *gmodu = modu;
329-
if (jl_is_globalref(gsym)) {
330-
gmodu = jl_globalref_mod(gsym);
331-
gsym = jl_globalref_name(gsym);
332-
}
333-
assert(jl_is_symbol(gsym));
334-
jl_get_binding_wr(gmodu, gsym);
335-
}
336-
return (jl_value_t*)jl_nothing;
337-
}
338322
else if (ex->head == abstracttype_sym) {
339323
if (inside_typedef)
340324
jl_error("cannot eval a new abstract type definition while defining another type");

src/julia-syntax.scm

+97-60
Original file line numberDiff line numberDiff line change
@@ -2641,8 +2641,9 @@
26412641
(define (free-vars e)
26422642
(table.keys (free-vars- e (table))))
26432643

2644-
(define (analyze-vars-lambda e env captvars sp new-sp)
2644+
(define (analyze-vars-lambda e env captvars sp new-sp glob-assign)
26452645
(let* ((args (lam:args e))
2646+
(glob-assign (if (null? args) (table) glob-assign))
26462647
(locl (caddr e))
26472648
(allv (nconc (map arg-name args) locl))
26482649
(fv (let* ((fv (diff (free-vars (lam:body e)) allv))
@@ -2674,7 +2675,14 @@
26742675
(and (not (memq (vinfo:name v) allv))
26752676
(not (memq (vinfo:name v) glo))))
26762677
env))
2677-
cv (delete-duplicates (append new-sp sp)))
2678+
cv
2679+
(delete-duplicates (append new-sp sp))
2680+
glob-assign)
2681+
;; if we collected any assignments to globals
2682+
;; annotate them now at the toplevel
2683+
(if (null? args)
2684+
(let ((glob-decl (map (lambda (e) `(global ,e)) (table.keys glob-assign))))
2685+
(set-car! (cdddr e) (insert-after-meta (lam:body e) glob-decl))))
26782686
;; mark all the vars we capture as captured
26792687
(for-each (lambda (v) (vinfo:set-capt! v #t))
26802688
cv)
@@ -2689,26 +2697,31 @@
26892697
;; in-place to
26902698
;; (var-info-lst captured-var-infos ssavalues static_params)
26912699
;; where var-info-lst is a list of var-info records
2692-
(define (analyze-vars e env captvars sp)
2700+
(define (analyze-vars e env captvars sp glob-assign)
26932701
(if (or (atom? e) (quoted? e))
26942702
e
26952703
(case (car e)
26962704
((local-def) ;; a local that we know has an assignment that dominates all usages
26972705
(let ((vi (var-info-for (cadr e) env)))
26982706
(vinfo:set-never-undef! vi #t)))
26992707
((=)
2700-
(let ((vi (var-info-for (cadr e) env)))
2701-
(if vi
2702-
(begin (if (vinfo:asgn vi)
2703-
(vinfo:set-sa! vi #f)
2704-
(vinfo:set-sa! vi #t))
2705-
(vinfo:set-asgn! vi #t))))
2706-
(analyze-vars (caddr e) env captvars sp))
2708+
(if (not (ssavalue? (cadr e)))
2709+
(let ((vi (and (symbol? (cadr e)) (var-info-for (cadr e) env))))
2710+
(if vi ; if local or captured
2711+
(begin (if (vinfo:asgn vi)
2712+
(vinfo:set-sa! vi #f)
2713+
(vinfo:set-sa! vi #t))
2714+
(vinfo:set-asgn! vi #t))
2715+
(if (and (pair? (cadr e)) (eq? (caadr e) 'outerref))
2716+
(if (not (memq (cadadr e) sp)) ; if not a sparam
2717+
(put! glob-assign (cadadr e) #t)) ; it's a global
2718+
(put! glob-assign (cadr e) #t))))) ; symbol or global ref
2719+
(analyze-vars (caddr e) env captvars sp glob-assign))
27072720
((call)
27082721
(let ((vi (var-info-for (cadr e) env)))
27092722
(if vi
27102723
(vinfo:set-called! vi #t))
2711-
(for-each (lambda (x) (analyze-vars x env captvars sp))
2724+
(for-each (lambda (x) (analyze-vars x env captvars sp glob-assign))
27122725
(cdr e))))
27132726
((decl)
27142727
;; handle var::T declaration by storing the type in the var-info
@@ -2723,12 +2736,13 @@
27232736
"\" declared in inner scope")))
27242737
(vinfo:set-type! vi (caddr e))))))
27252738
((lambda)
2726-
(analyze-vars-lambda e env captvars sp '()))
2739+
(analyze-vars-lambda e env captvars sp '() glob-assign))
27272740
((with-static-parameters)
27282741
;; (with-static-parameters func_expr sp_1 sp_2 ...)
27292742
(assert (eq? (car (cadr e)) 'lambda))
27302743
(analyze-vars-lambda (cadr e) env captvars sp
2731-
(cddr e)))
2744+
(cddr e)
2745+
glob-assign))
27322746
((method)
27332747
(if (length= e 2)
27342748
(let ((vi (var-info-for (method-expr-name e) env)))
@@ -2738,15 +2752,28 @@
27382752
(vinfo:set-sa! vi #t))
27392753
(vinfo:set-asgn! vi #t)))
27402754
e)
2741-
(begin (analyze-vars (caddr e) env captvars sp)
2755+
(begin (analyze-vars (caddr e) env captvars sp glob-assign)
27422756
(assert (eq? (car (cadddr e)) 'lambda))
27432757
(analyze-vars-lambda (cadddr e) env captvars sp
2744-
(method-expr-static-parameters e)))))
2758+
(method-expr-static-parameters e)
2759+
glob-assign))))
27452760
((module toplevel) e)
2746-
(else (for-each (lambda (x) (analyze-vars x env captvars sp))
2761+
(else (for-each (lambda (x) (analyze-vars x env captvars sp glob-assign))
27472762
(cdr e))))))
27482763

2749-
(define (analyze-variables! e) (analyze-vars e '() '() '()) e)
2764+
(define (analyze-variables! e)
2765+
(let ((glob-assign (table)))
2766+
(analyze-vars e '() '() '() glob-assign)
2767+
;; if we collected any assignments to globals
2768+
;; annotate them now at the toplevel
2769+
(let ((glob-decl (map (lambda (e) `(global ,e)) (table.keys glob-assign))))
2770+
(if (null? glob-decl)
2771+
e
2772+
(insert-after-meta
2773+
(if (and (pair? e) (eq? (car e) 'block))
2774+
e
2775+
`(block ,e))
2776+
glob-decl)))))
27502777

27512778
;; pass 4: closure conversion
27522779

@@ -2841,35 +2868,45 @@ f(x) = yt(x)
28412868
;; when doing this, the original value needs to be preserved, to
28422869
;; ensure the expression `a=b` always returns exactly `b`.
28432870
(define (convert-assignment var rhs0 fname lam interp)
2844-
(let* ((vi (assq var (car (lam:vinfo lam))))
2845-
(cv (assq var (cadr (lam:vinfo lam))))
2846-
(vt (or (and vi (vinfo:type vi))
2847-
(and cv (vinfo:type cv))
2848-
'(core Any)))
2849-
(closed (and cv (vinfo:asgn cv) (vinfo:capt cv)))
2850-
(capt (and vi (vinfo:asgn vi) (vinfo:capt vi))))
2851-
(if (and (not closed) (not capt) (equal? vt '(core Any)))
2852-
`(= ,var ,rhs0)
2853-
(let* ((rhs1 (if (or (ssavalue? rhs0) (simple-atom? rhs0)
2854-
(equal? rhs0 '(the_exception)))
2855-
rhs0
2856-
(make-ssavalue)))
2857-
(rhs (if (equal? vt '(core Any))
2858-
rhs1
2859-
(convert-for-type-decl rhs1 (cl-convert vt fname lam #f #f interp))))
2860-
(ex (cond (closed `(call (core setfield!)
2861-
,(if interp
2862-
`($ ,var)
2863-
`(call (core getfield) ,fname (inert ,var)))
2864-
(inert contents)
2865-
,rhs))
2866-
(capt `(call (core setfield!) ,var (inert contents) ,rhs))
2867-
(else `(= ,var ,rhs)))))
2868-
(if (eq? rhs1 rhs0)
2869-
`(block ,ex ,rhs0)
2870-
`(block (= ,rhs1 ,rhs0)
2871-
,ex
2872-
,rhs1))))))
2871+
(cond
2872+
((symbol? var)
2873+
(let* ((vi (assq var (car (lam:vinfo lam))))
2874+
(cv (assq var (cadr (lam:vinfo lam))))
2875+
(vt (or (and vi (vinfo:type vi))
2876+
(and cv (vinfo:type cv))
2877+
'(core Any)))
2878+
(closed (and cv (vinfo:asgn cv) (vinfo:capt cv)))
2879+
(capt (and vi (vinfo:asgn vi) (vinfo:capt vi))))
2880+
(if (and (not closed) (not capt) (equal? vt '(core Any)))
2881+
`(= ,var ,rhs0)
2882+
(let* ((rhs1 (if (or (ssavalue? rhs0) (simple-atom? rhs0)
2883+
(equal? rhs0 '(the_exception)))
2884+
rhs0
2885+
(make-ssavalue)))
2886+
(rhs (if (equal? vt '(core Any))
2887+
rhs1
2888+
(convert-for-type-decl rhs1 (cl-convert vt fname lam #f #f interp))))
2889+
(ex (cond (closed `(call (core setfield!)
2890+
,(if interp
2891+
`($ ,var)
2892+
`(call (core getfield) ,fname (inert ,var)))
2893+
(inert contents)
2894+
,rhs))
2895+
(capt `(call (core setfield!) ,var (inert contents) ,rhs))
2896+
(else `(= ,var ,rhs)))))
2897+
(if (eq? rhs1 rhs0)
2898+
`(block ,ex ,rhs0)
2899+
`(block (= ,rhs1 ,rhs0)
2900+
,ex
2901+
,rhs1))))))
2902+
((and (pair? var) (or (eq? (car var) 'outerref)
2903+
(eq? (car var) 'globalref)))
2904+
2905+
`(= ,var ,rhs0))
2906+
((ssavalue? var)
2907+
`(= ,var ,rhs0))
2908+
(else
2909+
(error (string "invalid assignment location \"" (deparse var) "\"")))))
28732910

28742911
;; replace leading (function) argument type with `typ`
28752912
(define (fix-function-arg-type te typ iskw namemap type-sp)
@@ -3056,9 +3093,7 @@ f(x) = yt(x)
30563093
((=)
30573094
(let ((var (cadr e))
30583095
(rhs (cl-convert (caddr e) fname lam namemap toplevel interp)))
3059-
(if (ssavalue? var)
3060-
`(= ,var ,rhs)
3061-
(convert-assignment var rhs fname lam interp))))
3096+
(convert-assignment var rhs fname lam interp)))
30623097
((local-def) ;; make new Box for local declaration of defined variable
30633098
(let ((vi (assq (cadr e) (car (lam:vinfo lam)))))
30643099
(if (and vi (vinfo:asgn vi) (vinfo:capt vi))
@@ -3100,10 +3135,10 @@ f(x) = yt(x)
31003135
(lam2 (if short #f (cadddr e)))
31013136
(vis (if short '(() () ()) (lam:vinfo lam2)))
31023137
(cvs (map car (cadr vis)))
3103-
(local? (lambda (s) (and (symbol? s)
3138+
(local? (lambda (s) (and lam (symbol? s)
31043139
(or (assq s (car (lam:vinfo lam)))
31053140
(assq s (cadr (lam:vinfo lam)))))))
3106-
(local (and lam (local? name)))
3141+
(local (local? name))
31073142
(sig (and (not short) (caddr e)))
31083143
(sp-inits (if (or short (not (eq? (car sig) 'block)))
31093144
'()
@@ -3180,7 +3215,7 @@ f(x) = yt(x)
31803215
(and (symbol? s)
31813216
(not (eq? name s))
31823217
(not (memq s capt-sp))
3183-
(or ;(local? s) ; TODO: make this work for local variables too?
3218+
(or ;(local? s) ; TODO: error for local variables
31843219
(memq s (lam:sp lam)))))))
31853220
(caddr methdef)
31863221
(lambda (e) (cadr e)))))
@@ -3306,7 +3341,8 @@ f(x) = yt(x)
33063341
;; numbered slots (or be simple immediate values), and then those will be the
33073342
;; only possible returned values.
33083343
(define (compile-body e vi lam)
3309-
(let ((code '())
3344+
(let ((code '()) ;; statements (emitted in reverse order)
3345+
(glob-decl '()) ;; global decls will be collected in the prelude to code so they execute first
33103346
(filename 'none)
33113347
(first-line #t)
33123348
(current-loc #f)
@@ -3614,6 +3650,7 @@ f(x) = yt(x)
36143650
(if (var-info-for vname vi)
36153651
;; issue #7264
36163652
(error (string "`global " vname "`: " vname " is local variable in the enclosing scope"))
3653+
(if (null? (lam:args lam)) (set! glob-decl (cons e glob-decl))) ;; keep global decl in thunks
36173654
#f)))
36183655
((local-def) #f)
36193656
((local) #f)
@@ -3699,13 +3736,13 @@ f(x) = yt(x)
36993736
(body (cons 'body (filter (lambda (e)
37003737
(not (and (pair? e) (eq? (car e) 'newvar)
37013738
(has? di (cadr e)))))
3702-
stmts))))
3703-
(if arg-map
3704-
(insert-after-meta
3705-
body
3706-
(table.foldl (lambda (k v lst) (cons `(= ,v ,k) lst))
3707-
'() arg-map))
3708-
body))))
3739+
stmts)))
3740+
(prelude (if arg-map
3741+
(append! glob-decl
3742+
(table.foldl (lambda (k v lst) (cons `(= ,v ,k) lst))
3743+
'() arg-map))
3744+
glob-decl)))
3745+
(insert-after-meta body prelude))))
37093746

37103747
;; find newvar nodes that are unnecessary because (1) the variable is not
37113748
;; captured, and (2) the variable is assigned before any branches.

src/method.c

+7-1
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,14 @@ jl_value_t *jl_resolve_globals(jl_value_t *expr, jl_module_t *module, jl_svec_t
2525
}
2626
else if (jl_is_expr(expr)) {
2727
jl_expr_t *e = (jl_expr_t*)expr;
28+
if (e->head == global_sym) {
29+
// execute the side-effects of "global x" decl immediately:
30+
// creates uninitialized mutable binding in module for each global
31+
jl_toplevel_eval_flex(module, expr, 0, 1);
32+
expr = jl_nothing;
33+
}
2834
if (jl_is_toplevel_only_expr(expr) || e->head == const_sym || e->head == copyast_sym ||
29-
e->head == global_sym || e->head == quote_sym || e->head == inert_sym ||
35+
e->head == quote_sym || e->head == inert_sym ||
3036
e->head == line_sym || e->head == meta_sym || e->head == inbounds_sym ||
3137
e->head == boundscheck_sym || e->head == simdloop_sym) {
3238
// ignore these

src/toplevel.c

+25-1
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@ int jl_is_toplevel_only_expr(jl_value_t *e)
442442
((jl_expr_t*)e)->head == using_sym ||
443443
((jl_expr_t*)e)->head == export_sym ||
444444
((jl_expr_t*)e)->head == thunk_sym ||
445+
((jl_expr_t*)e)->head == global_sym ||
445446
((jl_expr_t*)e)->head == toplevel_sym);
446447
}
447448

@@ -530,9 +531,32 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int e
530531
return jl_nothing;
531532
}
532533
else if (ex->head == line_sym) {
533-
jl_lineno = jl_unbox_long(jl_exprarg(ex,0));
534+
jl_lineno = jl_unbox_long(jl_exprarg(ex, 0));
534535
return jl_nothing;
535536
}
537+
else if (ex->head == global_sym) {
538+
// create uninitialized mutable binding for "global x" decl
539+
size_t i, l = jl_array_len(ex->args);
540+
for (i = 0; i < l; i++) {
541+
jl_value_t *a = jl_exprarg(ex, i);
542+
if (!jl_is_symbol(a) && !jl_is_globalref(a))
543+
break;
544+
}
545+
if (i == l) {
546+
for (i = 0; i < l; i++) {
547+
jl_sym_t *gs = (jl_sym_t*)jl_exprarg(ex, i);
548+
jl_module_t *gm = m;
549+
if (jl_is_globalref(gs)) {
550+
gm = jl_globalref_mod(gs);
551+
gs = jl_globalref_name(gs);
552+
}
553+
assert(jl_is_symbol(gs));
554+
jl_get_binding_wr(gm, gs);
555+
}
556+
return jl_nothing;
557+
}
558+
// fall-through to expand to normalize the syntax
559+
}
536560

537561
jl_method_instance_t *li = NULL;
538562
jl_value_t *result;

test/codegen.jl

+6-3
Original file line numberDiff line numberDiff line change
@@ -167,19 +167,22 @@ end
167167
let was_gced = false
168168
@noinline make_tuple(x) = tuple(x)
169169
@noinline use(x) = ccall(:jl_breakpoint, Void, ())
170-
@noinline assert_not_gced() = @assert !was_gced
170+
@noinline assert_not_gced() = @test !was_gced
171171

172172
function foo22770()
173173
b = Ref(2)
174-
finalizer(b, x->(global was_gced; was_gced=true))
174+
finalizer(b, x -> was_gced = true)
175175
y = make_tuple(b)
176176
x = y[1]
177177
a = Ref(1)
178178
use(x); use(a); use(y)
179179
c = Ref(3)
180-
gc(); assert_not_gced();
180+
gc()
181+
assert_not_gced()
181182
use(x)
182183
use(c)
183184
end
184185
foo22770()
186+
gc()
187+
@test was_gced
185188
end

0 commit comments

Comments
 (0)