Skip to content

Commit 9095265

Browse files
vtjnashKristofferC
vtjnash
authored and
KristofferC
committed
using/import: ensure world update after each observable operation (#57467)
Fix #57316 (cherry picked from commit a70818c)
1 parent 015c535 commit 9095265

File tree

4 files changed

+50
-42
lines changed

4 files changed

+50
-42
lines changed

‎src/julia.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2077,10 +2077,10 @@ JL_DLLEXPORT jl_value_t *jl_checked_assignonce(jl_binding_t *b, jl_module_t *mod
20772077
JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val(jl_binding_t *b JL_ROOTING_ARGUMENT, jl_module_t *mod, jl_sym_t *var, jl_value_t *val JL_ROOTED_ARGUMENT JL_MAYBE_UNROOTED);
20782078
JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val2(jl_binding_t *b JL_ROOTING_ARGUMENT, jl_module_t *mod, jl_sym_t *var, jl_value_t *val JL_ROOTED_ARGUMENT JL_MAYBE_UNROOTED, enum jl_partition_kind);
20792079
JL_DLLEXPORT void jl_module_using(jl_module_t *to, jl_module_t *from);
2080-
JL_DLLEXPORT void jl_module_use(jl_module_t *to, jl_module_t *from, jl_sym_t *s);
2081-
JL_DLLEXPORT void jl_module_use_as(jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname);
2082-
JL_DLLEXPORT void jl_module_import(jl_module_t *to, jl_module_t *from, jl_sym_t *s);
2083-
JL_DLLEXPORT void jl_module_import_as(jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname);
2080+
JL_DLLEXPORT void jl_module_use(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s);
2081+
JL_DLLEXPORT void jl_module_use_as(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname);
2082+
JL_DLLEXPORT void jl_module_import(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s);
2083+
JL_DLLEXPORT void jl_module_import_as(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname);
20842084
JL_DLLEXPORT void jl_module_public(jl_module_t *from, jl_sym_t *s, int exported);
20852085
JL_DLLEXPORT int jl_is_imported(jl_module_t *m, jl_sym_t *s);
20862086
JL_DLLEXPORT int jl_module_exports_p(jl_module_t *m, jl_sym_t *var);

‎src/module.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -887,11 +887,11 @@ JL_DLLEXPORT void check_safe_import_from(jl_module_t *m)
887887
}
888888

889889
// NOTE: we use explici since explicit is a C++ keyword
890-
static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *asname, jl_sym_t *s, int explici)
890+
static void module_import_(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *asname, jl_sym_t *s, int explici)
891891
{
892892
check_safe_import_from(from);
893893
jl_binding_t *b = jl_get_binding(from, s);
894-
jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age);
894+
jl_binding_partition_t *bpart = jl_get_binding_partition(b, ct->world_age);
895895
if (b->deprecated) {
896896
if (jl_get_binding_value(b) == jl_nothing) {
897897
// silently skip importing deprecated values assigned to nothing (to allow later mutation)
@@ -914,7 +914,7 @@ static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *asname,
914914

915915
jl_binding_t *ownerb = b;
916916
jl_binding_partition_t *ownerbpart = bpart;
917-
jl_walk_binding_inplace(&ownerb, &ownerbpart, jl_current_task->world_age);
917+
jl_walk_binding_inplace(&ownerb, &ownerbpart, ct->world_age);
918918

919919
if (jl_bkind_is_some_guard(jl_binding_kind(ownerbpart))) {
920920
jl_printf(JL_STDERR,
@@ -964,24 +964,24 @@ static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *asname,
964964
JL_UNLOCK(&world_counter_lock);
965965
}
966966

967-
JL_DLLEXPORT void jl_module_import(jl_module_t *to, jl_module_t *from, jl_sym_t *s)
967+
JL_DLLEXPORT void jl_module_import(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s)
968968
{
969-
module_import_(to, from, s, s, 1);
969+
module_import_(ct, to, from, s, s, 1);
970970
}
971971

972-
JL_DLLEXPORT void jl_module_import_as(jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname)
972+
JL_DLLEXPORT void jl_module_import_as(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname)
973973
{
974-
module_import_(to, from, asname, s, 1);
974+
module_import_(ct, to, from, asname, s, 1);
975975
}
976976

977-
JL_DLLEXPORT void jl_module_use(jl_module_t *to, jl_module_t *from, jl_sym_t *s)
977+
JL_DLLEXPORT void jl_module_use(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s)
978978
{
979-
module_import_(to, from, s, s, 0);
979+
module_import_(ct, to, from, s, s, 0);
980980
}
981981

982-
JL_DLLEXPORT void jl_module_use_as(jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname)
982+
JL_DLLEXPORT void jl_module_use_as(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname)
983983
{
984-
module_import_(to, from, asname, s, 0);
984+
module_import_(ct, to, from, asname, s, 0);
985985
}
986986

987987
void jl_add_usings_backedge(jl_module_t *from, jl_module_t *to)

‎src/toplevel.c

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -488,20 +488,18 @@ static void body_attributes(jl_array_t *body, int *has_ccall, int *has_defs, int
488488
}
489489

490490
extern size_t jl_require_world;
491-
static jl_module_t *call_require(jl_module_t *mod, jl_sym_t *var) JL_GLOBALLY_ROOTED
491+
static jl_module_t *call_require(jl_task_t *ct, jl_module_t *mod, jl_sym_t *var) JL_GLOBALLY_ROOTED
492492
{
493493
JL_TIMING(LOAD_IMAGE, LOAD_Require);
494494
jl_timing_printf(JL_TIMING_DEFAULT_BLOCK, "%s", jl_symbol_name(var));
495495

496496
int build_mode = jl_options.incremental && jl_generating_output();
497497
jl_module_t *m = NULL;
498-
jl_task_t *ct = jl_current_task;
499498
static jl_value_t *require_func = NULL;
500499
if (require_func == NULL && jl_base_module != NULL) {
501500
require_func = jl_get_global(jl_base_module, jl_symbol("require"));
502501
}
503502
if (require_func != NULL) {
504-
size_t last_age = ct->world_age;
505503
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
506504
if (build_mode && jl_require_world < ct->world_age)
507505
ct->world_age = jl_require_world;
@@ -510,18 +508,19 @@ static jl_module_t *call_require(jl_module_t *mod, jl_sym_t *var) JL_GLOBALLY_RO
510508
reqargs[1] = (jl_value_t*)mod;
511509
reqargs[2] = (jl_value_t*)var;
512510
m = (jl_module_t*)jl_apply(reqargs, 3);
513-
ct->world_age = last_age;
514511
}
515512
if (m == NULL || !jl_is_module(m)) {
516513
jl_errorf("failed to load module %s", jl_symbol_name(var));
517514
}
515+
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
518516
return m;
519517
}
520518

521519
// either:
522520
// - sets *name and returns the module to import *name from
523521
// - sets *name to NULL and returns a module to import
524-
static jl_module_t *eval_import_path(jl_module_t *where, jl_module_t *from JL_PROPAGATES_ROOT,
522+
// also updates world_age
523+
static jl_module_t *eval_import_path(jl_task_t *ct, jl_module_t *where, jl_module_t *from JL_PROPAGATES_ROOT,
525524
jl_array_t *args, jl_sym_t **name, const char *keyword) JL_GLOBALLY_ROOTED
526525
{
527526
if (jl_array_nrows(args) == 0)
@@ -546,7 +545,7 @@ static jl_module_t *eval_import_path(jl_module_t *where, jl_module_t *from JL_PR
546545
m = jl_base_module;
547546
}
548547
else {
549-
m = call_require(where, var);
548+
m = call_require(ct, where, var);
550549
}
551550
if (i == jl_array_nrows(args))
552551
return m;
@@ -566,6 +565,8 @@ static jl_module_t *eval_import_path(jl_module_t *where, jl_module_t *from JL_PR
566565
}
567566
}
568567

568+
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
569+
569570
while (1) {
570571
var = (jl_sym_t*)jl_array_ptr_ref(args, i);
571572
if (!jl_is_symbol(var))
@@ -650,17 +651,17 @@ JL_DLLEXPORT jl_method_instance_t *jl_method_instance_for_thunk(jl_code_info_t *
650651
return mi;
651652
}
652653

653-
static void import_module(jl_module_t *JL_NONNULL m, jl_module_t *import, jl_sym_t *asname)
654+
static void import_module(jl_task_t *ct, jl_module_t *JL_NONNULL m, jl_module_t *import, jl_sym_t *asname)
654655
{
655656
assert(m);
656657
jl_sym_t *name = asname ? asname : import->name;
657658
// TODO: this is a bit race-y with what error message we might print
658659
jl_binding_t *b = jl_get_module_binding(m, name, 1);
659-
jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age);
660+
jl_binding_partition_t *bpart = jl_get_binding_partition(b, ct->world_age);
660661
enum jl_partition_kind kind = jl_binding_kind(bpart);
661662
if (kind != BINDING_KIND_GUARD && kind != BINDING_KIND_FAILED && kind != BINDING_KIND_DECLARED && kind != BINDING_KIND_IMPLICIT) {
662663
// Unlike regular constant declaration, we allow this as long as we eventually end up at a constant.
663-
jl_walk_binding_inplace(&b, &bpart, jl_current_task->world_age);
664+
jl_walk_binding_inplace(&b, &bpart, ct->world_age);
664665
if (jl_binding_kind(bpart) == BINDING_KIND_CONST || jl_binding_kind(bpart) == BINDING_KIND_BACKDATED_CONST || jl_binding_kind(bpart) == BINDING_KIND_CONST_IMPORT) {
665666
// Already declared (e.g. on another thread) or imported.
666667
if (bpart->restriction == (jl_value_t*)import)
@@ -673,7 +674,7 @@ static void import_module(jl_module_t *JL_NONNULL m, jl_module_t *import, jl_sym
673674
}
674675

675676
// in `import A.B: x, y, ...`, evaluate the `A.B` part if it exists
676-
static jl_module_t *eval_import_from(jl_module_t *m JL_PROPAGATES_ROOT, jl_expr_t *ex, const char *keyword)
677+
static jl_module_t *eval_import_from(jl_task_t *ct, jl_module_t *m JL_PROPAGATES_ROOT, jl_expr_t *ex, const char *keyword)
677678
{
678679
if (jl_expr_nargs(ex) == 1 && jl_is_expr(jl_exprarg(ex, 0))) {
679680
jl_expr_t *fr = (jl_expr_t*)jl_exprarg(ex, 0);
@@ -682,7 +683,7 @@ static jl_module_t *eval_import_from(jl_module_t *m JL_PROPAGATES_ROOT, jl_expr_
682683
jl_expr_t *path = (jl_expr_t*)jl_exprarg(fr, 0);
683684
if (((jl_expr_t*)path)->head == jl_dot_sym) {
684685
jl_sym_t *name = NULL;
685-
jl_module_t *from = eval_import_path(m, NULL, path->args, &name, keyword);
686+
jl_module_t *from = eval_import_path(ct, m, NULL, path->args, &name, keyword);
686687
if (name != NULL) {
687688
from = (jl_module_t*)jl_eval_global_var(from, name);
688689
if (!from || !jl_is_module(from))
@@ -828,8 +829,7 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val
828829
}
829830
else if (head == jl_using_sym) {
830831
jl_sym_t *name = NULL;
831-
jl_module_t *from = eval_import_from(m, ex, "using");
832-
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
832+
jl_module_t *from = eval_import_from(ct, m, ex, "using");
833833
size_t i = 0;
834834
if (from) {
835835
i = 1;
@@ -839,10 +839,10 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val
839839
jl_value_t *a = jl_exprarg(ex, i);
840840
if (jl_is_expr(a) && ((jl_expr_t*)a)->head == jl_dot_sym) {
841841
name = NULL;
842-
jl_module_t *import = eval_import_path(m, from, ((jl_expr_t*)a)->args, &name, "using");
842+
jl_module_t *import = eval_import_path(ct, m, from, ((jl_expr_t*)a)->args, &name, "using");
843843
if (from) {
844844
// `using A: B` and `using A: B.c` syntax
845-
jl_module_use(m, import, name);
845+
jl_module_use(ct, m, import, name);
846846
}
847847
else {
848848
jl_module_t *u = import;
@@ -857,7 +857,7 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val
857857
if (m == jl_main_module && name == NULL) {
858858
// TODO: for now, `using A` in Main also creates an explicit binding for `A`
859859
// This will possibly be extended to all modules.
860-
import_module(m, u, NULL);
860+
import_module(ct, m, u, NULL);
861861
}
862862
}
863863
continue;
@@ -868,12 +868,11 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val
868868
if (jl_is_symbol(asname)) {
869869
jl_expr_t *path = (jl_expr_t*)jl_exprarg(a, 0);
870870
name = NULL;
871-
jl_module_t *import = eval_import_path(m, from, ((jl_expr_t*)path)->args, &name, "using");
872-
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
871+
jl_module_t *import = eval_import_path(ct, m, from, ((jl_expr_t*)path)->args, &name, "using");
873872
assert(name);
874873
check_macro_rename(name, asname, "using");
875874
// `using A: B as C` syntax
876-
jl_module_use_as(m, import, name, asname);
875+
jl_module_use_as(ct, m, import, name, asname);
877876
continue;
878877
}
879878
}
@@ -886,8 +885,7 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val
886885
}
887886
else if (head == jl_import_sym) {
888887
jl_sym_t *name = NULL;
889-
jl_module_t *from = eval_import_from(m, ex, "import");
890-
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
888+
jl_module_t *from = eval_import_from(ct, m, ex, "import");
891889
size_t i = 0;
892890
if (from) {
893891
i = 1;
@@ -897,14 +895,14 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val
897895
jl_value_t *a = jl_exprarg(ex, i);
898896
if (jl_is_expr(a) && ((jl_expr_t*)a)->head == jl_dot_sym) {
899897
name = NULL;
900-
jl_module_t *import = eval_import_path(m, from, ((jl_expr_t*)a)->args, &name, "import");
898+
jl_module_t *import = eval_import_path(ct, m, from, ((jl_expr_t*)a)->args, &name, "import");
901899
if (name == NULL) {
902900
// `import A` syntax
903-
import_module(m, import, NULL);
901+
import_module(ct, m, import, NULL);
904902
}
905903
else {
906904
// `import A.B` or `import A: B` syntax
907-
jl_module_import(m, import, name);
905+
jl_module_import(ct, m, import, name);
908906
}
909907
continue;
910908
}
@@ -914,15 +912,15 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val
914912
if (jl_is_symbol(asname)) {
915913
jl_expr_t *path = (jl_expr_t*)jl_exprarg(a, 0);
916914
name = NULL;
917-
jl_module_t *import = eval_import_path(m, from, ((jl_expr_t*)path)->args, &name, "import");
915+
jl_module_t *import = eval_import_path(ct, m, from, ((jl_expr_t*)path)->args, &name, "import");
918916
if (name == NULL) {
919917
// `import A as B` syntax
920-
import_module(m, import, asname);
918+
import_module(ct, m, import, asname);
921919
}
922920
else {
923921
check_macro_rename(name, asname, "import");
924922
// `import A.B as C` syntax
925-
jl_module_import_as(m, import, name, asname);
923+
jl_module_import_as(ct, m, import, name, asname);
926924
}
927925
continue;
928926
}

‎test/worlds.jl

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,3 +524,13 @@ module AmbigWorldTest
524524
convert(Core.Binding, GlobalRef(M2, :x)).partitions.min_world
525525
)
526526
end
527+
528+
module X57316; module Y57316; end; end
529+
module A57316; using ..X57316.Y57316, .Y57316.Y57316; end
530+
module B57316; import ..X57316.Y57316, .Y57316.Y57316; end
531+
module C57316; import ..X57316.Y57316 as Z, .Z.Y57316 as W; end
532+
@test X57316.Y57316 === A57316.Y57316 === B57316.Y57316 === C57316.Z === C57316.W
533+
@test !isdefined(A57316, :X57316)
534+
@test !isdefined(B57316, :X57316)
535+
@test !isdefined(C57316, :X57316)
536+
@test !isdefined(C57316, :Y57316)

0 commit comments

Comments
 (0)