Skip to content

Commit 726c816

Browse files
authored
Prohibit importing or using Main during incremental compilation (#57426)
An upcoming optimization will skip most binding validation if no binding replacement has taken place in (sysimage, pkgimage) modules. However, as a special case, we would like to treat `Main` as a non-sysimage module because the addition of new bindings in `Main` is common and we would like this to not ruin the optimization. To make this legal, we have to prohibit `import`ing or `using` any `Main` bindings in pkgimages. I don't think anybody actually does this, particularly, since `Main` is not considered loading during precompile (so you have to use the main binding via (Core|Base|).Main), and I can't think of any good semantic reason to want to do this, but regardless, it does add additional restrictions to `using`/`import`, so I wanted to break it out into its own PR.
1 parent 56aed62 commit 726c816

File tree

2 files changed

+58
-18
lines changed

2 files changed

+58
-18
lines changed

src/module.c

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -509,28 +509,31 @@ static jl_binding_t *new_binding(jl_module_t *mod, jl_sym_t *name)
509509

510510
extern jl_mutex_t jl_modules_mutex;
511511

512+
static int is_module_open(jl_module_t *m)
513+
{
514+
JL_LOCK(&jl_modules_mutex);
515+
int open = ptrhash_has(&jl_current_modules, (void*)m);
516+
if (!open && jl_module_init_order != NULL) {
517+
size_t i, l = jl_array_len(jl_module_init_order);
518+
for (i = 0; i < l; i++) {
519+
if (m == (jl_module_t*)jl_array_ptr_ref(jl_module_init_order, i)) {
520+
open = 1;
521+
break;
522+
}
523+
}
524+
}
525+
JL_UNLOCK(&jl_modules_mutex);
526+
return open;
527+
}
528+
512529
extern void check_safe_newbinding(jl_module_t *m, jl_sym_t *var)
513530
{
514531
if (jl_current_task->ptls->in_pure_callback)
515532
jl_errorf("new strong globals cannot be created in a generated function. Declare them outside using `global x::Any`.");
516-
if (jl_options.incremental && jl_generating_output()) {
517-
JL_LOCK(&jl_modules_mutex);
518-
int open = ptrhash_has(&jl_current_modules, (void*)m);
519-
if (!open && jl_module_init_order != NULL) {
520-
size_t i, l = jl_array_len(jl_module_init_order);
521-
for (i = 0; i < l; i++) {
522-
if (m == (jl_module_t*)jl_array_ptr_ref(jl_module_init_order, i)) {
523-
open = 1;
524-
break;
525-
}
526-
}
527-
}
528-
JL_UNLOCK(&jl_modules_mutex);
529-
if (!open) {
530-
jl_errorf("Creating a new global in closed module `%s` (`%s`) breaks incremental compilation "
531-
"because the side effects will not be permanent.",
532-
jl_symbol_name(m->name), jl_symbol_name(var));
533-
}
533+
if (jl_options.incremental && jl_generating_output() && !is_module_open(m)) {
534+
jl_errorf("Creating a new global in closed module `%s` (`%s`) breaks incremental compilation "
535+
"because the side effects will not be permanent.",
536+
jl_symbol_name(m->name), jl_symbol_name(var));
534537
}
535538
}
536539

@@ -876,9 +879,17 @@ static void jl_binding_dep_message(jl_module_t *m, jl_sym_t *name, jl_binding_t
876879
JL_GC_POP();
877880
}
878881

882+
JL_DLLEXPORT void check_safe_import_from(jl_module_t *m)
883+
{
884+
if (jl_options.incremental && jl_generating_output() && m == jl_main_module) {
885+
jl_errorf("Any `import` or `using` from `Main` is prohibited during incremental compilation.");
886+
}
887+
}
888+
879889
// NOTE: we use explici since explicit is a C++ keyword
880890
static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *asname, jl_sym_t *s, int explici)
881891
{
892+
check_safe_import_from(from);
882893
jl_binding_t *b = jl_get_binding(from, s);
883894
jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age);
884895
if (b->deprecated) {
@@ -988,6 +999,7 @@ JL_DLLEXPORT void jl_module_using(jl_module_t *to, jl_module_t *from)
988999
{
9891000
if (to == from)
9901001
return;
1002+
check_safe_import_from(from);
9911003
JL_LOCK(&world_counter_lock);
9921004
JL_LOCK(&to->lock);
9931005
for (size_t i = 0; i < module_usings_length(to); i++) {

test/precompile.jl

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2332,4 +2332,32 @@ precompile_test_harness("BindingReplaceDisallow") do load_path
23322332
end
23332333
end
23342334

2335+
precompile_test_harness("MainImportDisallow") do load_path
2336+
write(joinpath(load_path, "MainImportDisallow.jl"),
2337+
"""
2338+
module MainImportDisallow
2339+
const importvar = try
2340+
import Base.Main: cant_get_at_me
2341+
catch ex
2342+
ex isa ErrorException || rethrow()
2343+
ex
2344+
end
2345+
const usingmain = try
2346+
using Base.Main
2347+
catch ex
2348+
ex isa ErrorException || rethrow()
2349+
ex
2350+
end
2351+
# Import `Main` is permitted, because it does not look at bindings inside `Main`
2352+
import Base.Main
2353+
end
2354+
""")
2355+
ji, ofile = Base.compilecache(Base.PkgId("MainImportDisallow"))
2356+
@eval using MainImportDisallow
2357+
invokelatest() do
2358+
@test MainImportDisallow.importvar.msg == "Any `import` or `using` from `Main` is prohibited during incremental compilation."
2359+
@test MainImportDisallow.usingmain.msg == "Any `import` or `using` from `Main` is prohibited during incremental compilation."
2360+
end
2361+
end
2362+
23352363
finish_precompile_test!()

0 commit comments

Comments
 (0)