Skip to content

Commit 4fea203

Browse files
vtjnashvtjnash
authored
Merge pull request #22984 from JuliaLang/jn/18933
more predictable global binding resolution
2 parents 66a505d + 9e76be1 commit 4fea203

16 files changed

+199
-146
lines changed

‎NEWS.md

+9
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,15 @@ Language changes
4646

4747
+ `bits_type` => `primitive_type`
4848

49+
* The `global` keyword now only introduces a new binding if one doesn't already exist
50+
in the module.
51+
This means that assignment to a global (`global sin = 3`) may now throw the error:
52+
"cannot assign variable Base.sin from module Main", rather than emitting a warning.
53+
Additionally, the new bindings are now created before the statement is executed.
54+
For example, `f() = (global sin = "gluttony"; nothing)` will now resolve which module
55+
contains `sin` eagerly, rather than delaying that decision until `f` is run. ([#22984]).
56+
57+
4958
Breaking changes
5059
----------------
5160

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

‎doc/src/manual/modules.md

+3-3
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,8 @@ in other modules can be invoked as `Mod.@mac` or `@Mod.mac`.
214214
The syntax `M.x = y` does not work to assign a global in another module; global assignment is
215215
always module-local.
216216

217-
A variable can be "reserved" for the current module without assigning to it by declaring it as
218-
`global x` at the top level. This can be used to prevent name conflicts for globals initialized
219-
after load time.
217+
A variable name can be "reserved" without assigning to it by declaring it as `global x`.
218+
This prevents name conflicts for globals initialized after load time.
220219

221220
### Module initialization and precompilation
222221

@@ -283,6 +282,7 @@ const foo_data_ptr = Ref{Ptr{Void}}(0)
283282
function __init__()
284283
ccall((:foo_init, :libfoo), Void, ())
285284
foo_data_ptr[] = ccall((:foo_data, :libfoo), Ptr{Void}, ())
285+
nothing
286286
end
287287
```
288288

‎doc/src/manual/variables-and-scoping.md

+4-18
Original file line numberDiff line numberDiff line change
@@ -174,19 +174,6 @@ julia> x
174174
12
175175
```
176176

177-
Within soft scopes, the *global* keyword is never necessary, although allowed. The only case
178-
when it would change the semantics is (currently) a syntax error:
179-
180-
```jldoctest
181-
julia> let
182-
local j = 2
183-
let
184-
global j = 3
185-
end
186-
end
187-
ERROR: syntax: `global j`: j is local variable in the enclosing scope
188-
```
189-
190177
### Hard Local Scope
191178

192179
Hard local scopes are introduced by function definitions (in all their forms), struct type definition blocks,
@@ -336,8 +323,7 @@ constructing [closures](https://en.wikipedia.org/wiki/Closure_%28computer_progra
336323
have a private state, for instance the `state` variable in the following example:
337324

338325
```jldoctest
339-
julia> let
340-
state = 0
326+
julia> let state = 0
341327
global counter
342328
counter() = state += 1
343329
end;
@@ -483,13 +469,13 @@ julia> const e = 2.71828182845904523536;
483469
julia> const pi = 3.14159265358979323846;
484470
```
485471

486-
The `const` declaration is allowed on both global and local variables, but is especially useful
487-
for globals. It is difficult for the compiler to optimize code involving global variables, since
472+
The `const` declaration should only be used in global scope on globals.
473+
It is difficult for the compiler to optimize code involving global variables, since
488474
their values (or even their types) might change at almost any time. If a global variable will
489475
not change, adding a `const` declaration solves this performance problem.
490476

491477
Local constants are quite different. The compiler is able to determine automatically when a local
492-
variable is constant, so local constant declarations are not necessary for performance purposes.
478+
variable is constant, so local constant declarations are not necessary, and are currently just ignored.
493479

494480
Special top-level assignments, such as those performed by the `function` and `struct` keywords,
495481
are constant by default.

‎src/codegen.cpp

+9-2
Original file line numberDiff line numberDiff line change
@@ -3012,8 +3012,15 @@ static Value *global_binding_pointer(jl_codectx_t &ctx, jl_module_t *m, jl_sym_t
30123012
{
30133013
jl_binding_t *b = NULL;
30143014
if (assign) {
3015-
b = jl_get_binding_wr(m, s);
3015+
b = jl_get_binding_wr(m, s, 0);
30163016
assert(b != NULL);
3017+
if (b->owner != m) {
3018+
char *msg;
3019+
asprintf(&msg, "cannot assign variable %s.%s from module %s",
3020+
jl_symbol_name(b->owner->name), jl_symbol_name(s), jl_symbol_name(m->name));
3021+
emit_error(ctx, msg);
3022+
free(msg);
3023+
}
30173024
}
30183025
else {
30193026
b = jl_get_binding(m, s);
@@ -3748,7 +3755,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr)
37483755
jl_ptls_t ptls = jl_get_ptls_states();
37493756
jl_value_t *e = ptls->exception_in_transit;
37503757
// errors. boo. root it somehow :(
3751-
bnd = jl_get_binding_wr(ctx.module, (jl_sym_t*)jl_gensym());
3758+
bnd = jl_get_binding_wr(ctx.module, (jl_sym_t*)jl_gensym(), 1);
37523759
bnd->value = e;
37533760
bnd->constp = 1;
37543761
raise_exception(ctx, literal_pointer_val(ctx, e));

‎src/dump.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -1515,7 +1515,7 @@ static jl_value_t *jl_deserialize_value_module(jl_serializer_state *s)
15151515
jl_sym_t *name = (jl_sym_t*)jl_deserialize_value(s, NULL);
15161516
if (name == NULL)
15171517
break;
1518-
jl_binding_t *b = jl_get_binding_wr(m, name);
1518+
jl_binding_t *b = jl_get_binding_wr(m, name, 1);
15191519
b->value = jl_deserialize_value(s, &b->value);
15201520
jl_gc_wb_buf(m, b, sizeof(jl_binding_t));
15211521
if (b->value != NULL) jl_gc_wb(m, b->value);
@@ -1981,7 +1981,7 @@ static void jl_reinit_item(jl_value_t *v, int how, arraylist_t *tracee_list)
19811981
}
19821982
case 2: { // reinsert module v into parent (const)
19831983
jl_module_t *mod = (jl_module_t*)v;
1984-
jl_binding_t *b = jl_get_binding_wr(mod->parent, mod->name);
1984+
jl_binding_t *b = jl_get_binding_wr(mod->parent, mod->name, 1);
19851985
jl_declare_constant(b); // this can throw
19861986
if (b->value != NULL) {
19871987
if (!jl_is_module(b->value)) {

‎src/interpreter.c

+12-36
Original file line numberDiff line numberDiff line change
@@ -302,20 +302,15 @@ static jl_value_t *eval(jl_value_t *e, interpreter_state *s)
302302
assert(jl_expr_nargs(ex) != 1 || jl_is_symbol(fname));
303303

304304
if (jl_is_symbol(fname)) {
305-
jl_value_t **bp=NULL;
306-
jl_value_t *bp_owner=NULL;
307-
jl_binding_t *b=NULL;
308-
if (bp == NULL) {
309-
b = jl_get_binding_for_method_def(modu, fname);
310-
bp = &b->value;
311-
bp_owner = (jl_value_t*)modu;
312-
}
313-
jl_value_t *gf = jl_generic_function_def(fname, modu, bp, bp_owner, b);
305+
jl_value_t *bp_owner = (jl_value_t*)modu;
306+
jl_binding_t *b = jl_get_binding_for_method_def(modu, fname);
307+
jl_value_t **bp = &b->value;
308+
jl_value_t *gf = jl_generic_function_def(b->name, b->owner, bp, bp_owner, b);
314309
if (jl_expr_nargs(ex) == 1)
315310
return gf;
316311
}
317312

318-
jl_value_t *atypes=NULL, *meth=NULL;
313+
jl_value_t *atypes = NULL, *meth = NULL;
319314
JL_GC_PUSH2(&atypes, &meth);
320315
atypes = eval(args[1], s);
321316
meth = eval(args[2], s);
@@ -330,26 +325,10 @@ static jl_value_t *eval(jl_value_t *e, interpreter_state *s)
330325
sym = jl_globalref_name(sym);
331326
}
332327
assert(jl_is_symbol(sym));
333-
jl_binding_t *b = jl_get_binding_wr(modu, sym);
328+
jl_binding_t *b = jl_get_binding_wr(modu, sym, 1);
334329
jl_declare_constant(b);
335330
return (jl_value_t*)jl_nothing;
336331
}
337-
else if (ex->head == global_sym) {
338-
// create uninitialized mutable binding for "global x" decl
339-
// TODO: handle type decls
340-
size_t i, l = jl_array_len(ex->args);
341-
for (i = 0; i < l; i++) {
342-
jl_sym_t *gsym = (jl_sym_t*)args[i];
343-
jl_module_t *gmodu = modu;
344-
if (jl_is_globalref(gsym)) {
345-
gmodu = jl_globalref_mod(gsym);
346-
gsym = jl_globalref_name(gsym);
347-
}
348-
assert(jl_is_symbol(gsym));
349-
jl_get_binding_wr(gmodu, gsym);
350-
}
351-
return (jl_value_t*)jl_nothing;
352-
}
353332
else if (ex->head == abstracttype_sym) {
354333
if (inside_typedef)
355334
jl_error("cannot eval a new abstract type definition while defining another type");
@@ -368,7 +347,7 @@ static jl_value_t *eval(jl_value_t *e, interpreter_state *s)
368347
assert(jl_is_symbol(name));
369348
dt = jl_new_abstracttype(name, modu, NULL, (jl_svec_t*)para);
370349
w = dt->name->wrapper;
371-
jl_binding_t *b = jl_get_binding_wr(modu, (jl_sym_t*)name);
350+
jl_binding_t *b = jl_get_binding_wr(modu, (jl_sym_t*)name, 1);
372351
temp = b->value;
373352
check_can_assign_type(b, w);
374353
b->value = w;
@@ -416,7 +395,7 @@ static jl_value_t *eval(jl_value_t *e, interpreter_state *s)
416395
jl_symbol_name((jl_sym_t*)name));
417396
dt = jl_new_primitivetype(name, modu, NULL, (jl_svec_t*)para, nb);
418397
w = dt->name->wrapper;
419-
jl_binding_t *b = jl_get_binding_wr(modu, (jl_sym_t*)name);
398+
jl_binding_t *b = jl_get_binding_wr(modu, (jl_sym_t*)name, 1);
420399
temp = b->value;
421400
check_can_assign_type(b, w);
422401
b->value = w;
@@ -461,7 +440,7 @@ static jl_value_t *eval(jl_value_t *e, interpreter_state *s)
461440
0, args[5]==jl_true ? 1 : 0, jl_unbox_long(args[6]));
462441
w = dt->name->wrapper;
463442

464-
jl_binding_t *b = jl_get_binding_wr(modu, (jl_sym_t*)name);
443+
jl_binding_t *b = jl_get_binding_wr(modu, (jl_sym_t*)name, 1);
465444
temp = b->value; // save old value
466445
// temporarily assign so binding is available for field types
467446
check_can_assign_type(b, w);
@@ -573,17 +552,14 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, int start,
573552
s->locals[n-1] = rhs;
574553
}
575554
else {
576-
jl_module_t *m;
555+
jl_module_t *modu = s->module;
577556
if (jl_is_globalref(sym)) {
578-
m = jl_globalref_mod(sym);
557+
modu = jl_globalref_mod(sym);
579558
sym = (jl_value_t*)jl_globalref_name(sym);
580559
}
581-
else {
582-
m = s->module;
583-
}
584560
assert(jl_is_symbol(sym));
585561
JL_GC_PUSH1(&rhs);
586-
jl_binding_t *b = jl_get_binding_wr(m, (jl_sym_t*)sym);
562+
jl_binding_t *b = jl_get_binding_wr(modu, (jl_sym_t*)sym, 1);
587563
jl_checked_assignment(b, rhs);
588564
JL_GC_POP();
589565
}

‎src/julia-syntax.scm

+49-42
Original file line numberDiff line numberDiff line change
@@ -2717,8 +2717,8 @@
27172717
(let ((vi (var-info-for (cadr e) env)))
27182718
(vinfo:set-never-undef! vi #t)))
27192719
((=)
2720-
(let ((vi (var-info-for (cadr e) env)))
2721-
(if vi
2720+
(let ((vi (and (symbol? (cadr e)) (var-info-for (cadr e) env))))
2721+
(if vi ; if local or captured
27222722
(begin (if (vinfo:asgn vi)
27232723
(vinfo:set-sa! vi #f)
27242724
(vinfo:set-sa! vi #t))
@@ -2861,35 +2861,45 @@ f(x) = yt(x)
28612861
;; when doing this, the original value needs to be preserved, to
28622862
;; ensure the expression `a=b` always returns exactly `b`.
28632863
(define (convert-assignment var rhs0 fname lam interp)
2864-
(let* ((vi (assq var (car (lam:vinfo lam))))
2865-
(cv (assq var (cadr (lam:vinfo lam))))
2866-
(vt (or (and vi (vinfo:type vi))
2867-
(and cv (vinfo:type cv))
2868-
'(core Any)))
2869-
(closed (and cv (vinfo:asgn cv) (vinfo:capt cv)))
2870-
(capt (and vi (vinfo:asgn vi) (vinfo:capt vi))))
2871-
(if (and (not closed) (not capt) (equal? vt '(core Any)))
2872-
`(= ,var ,rhs0)
2873-
(let* ((rhs1 (if (or (ssavalue? rhs0) (simple-atom? rhs0)
2874-
(equal? rhs0 '(the_exception)))
2875-
rhs0
2876-
(make-ssavalue)))
2877-
(rhs (if (equal? vt '(core Any))
2878-
rhs1
2879-
(convert-for-type-decl rhs1 (cl-convert vt fname lam #f #f interp))))
2880-
(ex (cond (closed `(call (core setfield!)
2881-
,(if interp
2882-
`($ ,var)
2883-
`(call (core getfield) ,fname (inert ,var)))
2884-
(inert contents)
2885-
,rhs))
2886-
(capt `(call (core setfield!) ,var (inert contents) ,rhs))
2887-
(else `(= ,var ,rhs)))))
2888-
(if (eq? rhs1 rhs0)
2889-
`(block ,ex ,rhs0)
2890-
`(block (= ,rhs1 ,rhs0)
2891-
,ex
2892-
,rhs1))))))
2864+
(cond
2865+
((symbol? var)
2866+
(let* ((vi (assq var (car (lam:vinfo lam))))
2867+
(cv (assq var (cadr (lam:vinfo lam))))
2868+
(vt (or (and vi (vinfo:type vi))
2869+
(and cv (vinfo:type cv))
2870+
'(core Any)))
2871+
(closed (and cv (vinfo:asgn cv) (vinfo:capt cv)))
2872+
(capt (and vi (vinfo:asgn vi) (vinfo:capt vi))))
2873+
(if (and (not closed) (not capt) (equal? vt '(core Any)))
2874+
`(= ,var ,rhs0)
2875+
(let* ((rhs1 (if (or (ssavalue? rhs0) (simple-atom? rhs0)
2876+
(equal? rhs0 '(the_exception)))
2877+
rhs0
2878+
(make-ssavalue)))
2879+
(rhs (if (equal? vt '(core Any))
2880+
rhs1
2881+
(convert-for-type-decl rhs1 (cl-convert vt fname lam #f #f interp))))
2882+
(ex (cond (closed `(call (core setfield!)
2883+
,(if interp
2884+
`($ ,var)
2885+
`(call (core getfield) ,fname (inert ,var)))
2886+
(inert contents)
2887+
,rhs))
2888+
(capt `(call (core setfield!) ,var (inert contents) ,rhs))
2889+
(else `(= ,var ,rhs)))))
2890+
(if (eq? rhs1 rhs0)
2891+
`(block ,ex ,rhs0)
2892+
`(block (= ,rhs1 ,rhs0)
2893+
,ex
2894+
,rhs1))))))
2895+
((and (pair? var) (or (eq? (car var) 'outerref)
2896+
(eq? (car var) 'globalref)))
2897+
2898+
`(= ,var ,rhs0))
2899+
((ssavalue? var)
2900+
`(= ,var ,rhs0))
2901+
(else
2902+
(error (string "invalid assignment location \"" (deparse var) "\"")))))
28932903

28942904
;; replace leading (function) argument type with `typ`
28952905
(define (fix-function-arg-type te typ iskw namemap type-sp)
@@ -3076,9 +3086,7 @@ f(x) = yt(x)
30763086
((=)
30773087
(let ((var (cadr e))
30783088
(rhs (cl-convert (caddr e) fname lam namemap toplevel interp)))
3079-
(if (ssavalue? var)
3080-
`(= ,var ,rhs)
3081-
(convert-assignment var rhs fname lam interp))))
3089+
(convert-assignment var rhs fname lam interp)))
30823090
((local-def) ;; make new Box for local declaration of defined variable
30833091
(let ((vi (assq (cadr e) (car (lam:vinfo lam)))))
30843092
(if (and vi (vinfo:asgn vi) (vinfo:capt vi))
@@ -3120,10 +3128,10 @@ f(x) = yt(x)
31203128
(lam2 (if short #f (cadddr e)))
31213129
(vis (if short '(() () ()) (lam:vinfo lam2)))
31223130
(cvs (map car (cadr vis)))
3123-
(local? (lambda (s) (and (symbol? s)
3131+
(local? (lambda (s) (and lam (symbol? s)
31243132
(or (assq s (car (lam:vinfo lam)))
31253133
(assq s (cadr (lam:vinfo lam)))))))
3126-
(local (and lam (local? name)))
3134+
(local (local? name))
31273135
(sig (and (not short) (caddr e)))
31283136
(sp-inits (if (or short (not (eq? (car sig) 'block)))
31293137
'()
@@ -3200,7 +3208,7 @@ f(x) = yt(x)
32003208
(and (symbol? s)
32013209
(not (eq? name s))
32023210
(not (memq s capt-sp))
3203-
(or ;(local? s) ; TODO: make this work for local variables too?
3211+
(or ;(local? s) ; TODO: error for local variables
32043212
(memq s (lam:sp lam)))))))
32053213
(caddr methdef)
32063214
(lambda (e) (cadr e)))))
@@ -3326,7 +3334,7 @@ f(x) = yt(x)
33263334
;; numbered slots (or be simple immediate values), and then those will be the
33273335
;; only possible returned values.
33283336
(define (compile-body e vi lam)
3329-
(let ((code '())
3337+
(let ((code '()) ;; statements (emitted in reverse order)
33303338
(filename 'none)
33313339
(first-line #t)
33323340
(current-loc #f)
@@ -3628,13 +3636,12 @@ f(x) = yt(x)
36283636
(if (not (and (pair? code) (equal? (car code) e)))
36293637
(emit e)
36303638
#f))
3631-
((global) ; remove global declarations
3639+
((global) ; keep global declarations as statements
36323640
(if value (error "misplaced \"global\" declaration"))
36333641
(let ((vname (cadr e)))
3634-
(if (var-info-for vname vi)
3635-
;; issue #7264
3642+
(if (var-info-for vname vi) ;; issue #7264
36363643
(error (string "`global " vname "`: " vname " is local variable in the enclosing scope"))
3637-
#f)))
3644+
(emit e))))
36383645
((local-def) #f)
36393646
((local) #f)
36403647
((implicit-global) #f)

‎src/julia.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -1236,7 +1236,7 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding(jl_module_t *m, jl_sym_t *var);
12361236
JL_DLLEXPORT jl_binding_t *jl_get_binding_or_error(jl_module_t *m, jl_sym_t *var);
12371237
JL_DLLEXPORT jl_value_t *jl_module_globalref(jl_module_t *m, jl_sym_t *var);
12381238
// get binding for assignment
1239-
JL_DLLEXPORT jl_binding_t *jl_get_binding_wr(jl_module_t *m, jl_sym_t *var);
1239+
JL_DLLEXPORT jl_binding_t *jl_get_binding_wr(jl_module_t *m, jl_sym_t *var, int error);
12401240
JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m,
12411241
jl_sym_t *var);
12421242
JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var);

0 commit comments

Comments
 (0)