Skip to content

Commit e2e8a59

Browse files
committed
Prohibit importing or using Main during incremental compilation
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 3d54065 commit e2e8a59

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) {
@@ -991,6 +1002,7 @@ JL_DLLEXPORT void jl_module_using(jl_module_t *to, jl_module_t *from)
9911002
{
9921003
if (to == from)
9931004
return;
1005+
check_safe_import_from(from);
9941006
JL_LOCK(&world_counter_lock);
9951007
JL_LOCK(&to->lock);
9961008
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)