Skip to content

Commit 2b3fde4

Browse files
committed
Refine errors for incremental compilation
* Make incremental compilation failures an error. * Remove loophole which allowed Base.include to skip the incremental compilation warning * Add special case so that `Base.__toplevel__` is never considered closed, allowing normal use of include()
1 parent a6a2d26 commit 2b3fde4

File tree

4 files changed

+39
-11
lines changed

4 files changed

+39
-11
lines changed

src/ast.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -836,6 +836,7 @@ jl_value_t *jl_parse_eval_all(const char *fname,
836836
jl_ptls_t ptls = jl_get_ptls_states();
837837
if (ptls->in_pure_callback)
838838
jl_error("cannot use include inside a generated function");
839+
jl_check_open_for(inmodule, "include");
839840
jl_ast_context_t *ctx = jl_ast_ctx_enter();
840841
fl_context_t *fl_ctx = &ctx->fl;
841842
value_t f, ast, expression;

src/julia_internal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,7 @@ void jl_init_main_module(void);
470470
int jl_is_submodule(jl_module_t *child, jl_module_t *parent);
471471
jl_array_t *jl_get_loaded_modules(void);
472472

473+
void jl_check_open_for(jl_module_t *m, const char* funcname);
473474
jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int expanded);
474475

475476
jl_value_t *jl_eval_global_var(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *e);

src/toplevel.c

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,12 @@ jl_array_t *jl_get_loaded_modules(void)
107107
return NULL;
108108
}
109109

110+
static int jl_is__toplevel__mod(jl_module_t *mod)
111+
{
112+
return jl_base_module &&
113+
(jl_value_t*)mod == jl_get_global(jl_base_module, jl_symbol("__toplevel__"));
114+
}
115+
110116
// TODO: add locks around global state mutation operations
111117
jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex)
112118
{
@@ -128,8 +134,7 @@ jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex)
128134

129135
// copy parent environment info into submodule
130136
newm->uuid = parent_module->uuid;
131-
if (jl_base_module &&
132-
(jl_value_t*)parent_module == jl_get_global(jl_base_module, jl_symbol("__toplevel__"))) {
137+
if (jl_is__toplevel__mod(parent_module)) {
133138
newm->parent = newm;
134139
jl_register_root_module(newm);
135140
}
@@ -822,25 +827,33 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval(jl_module_t *m, jl_value_t *v)
822827
return jl_toplevel_eval_flex(m, v, 1, 0);
823828
}
824829

830+
// Check module `m` is open for `eval/include`, or throw an error.
831+
void jl_check_open_for(jl_module_t *m, const char* funcname)
832+
{
833+
if (jl_options.incremental && jl_generating_output()) {
834+
if (!ptrhash_has(&jl_current_modules, (void*)m) && !jl_is__toplevel__mod(m)) {
835+
if (m != jl_main_module) { // TODO: this was grand-fathered in
836+
const char* name = jl_symbol_name(m->name);
837+
jl_errorf("Evaluation into the closed module `%s` breaks incremental compilation "
838+
"because the side effects will not be permanent. "
839+
"This is likely due to some other module mutating `%s` with `%s` during "
840+
"precompilation - don't do this.", name, name, funcname);
841+
}
842+
}
843+
}
844+
}
845+
825846
JL_DLLEXPORT jl_value_t *jl_toplevel_eval_in(jl_module_t *m, jl_value_t *ex)
826847
{
827848
jl_ptls_t ptls = jl_get_ptls_states();
828849
if (ptls->in_pure_callback)
829850
jl_error("eval cannot be used in a generated function");
851+
jl_check_open_for(m, "eval");
830852
jl_value_t *v = NULL;
831853
int last_lineno = jl_lineno;
832854
const char *last_filename = jl_filename;
833855
jl_lineno = 1;
834856
jl_filename = "none";
835-
if (jl_options.incremental && jl_generating_output()) {
836-
if (!ptrhash_has(&jl_current_modules, (void*)m)) {
837-
if (m != jl_main_module) { // TODO: this was grand-fathered in
838-
jl_printf(JL_STDERR, "WARNING: eval into closed module %s:\n", jl_symbol_name(m->name));
839-
jl_static_show(JL_STDERR, ex);
840-
jl_printf(JL_STDERR, "\n ** incremental compilation may be fatally broken for this module **\n\n");
841-
}
842-
}
843-
}
844857
JL_TRY {
845858
v = jl_toplevel_eval(m, ex);
846859
}

test/precompile.jl

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,19 @@ try
397397
occursin("ERROR: LoadError: break me", exc.msg) && rethrow()
398398
end
399399

400+
# Test that trying to eval into closed modules during precompilation is an error
401+
FooBar3_file = joinpath(dir, "FooBar3.jl")
402+
FooBar3_inc = joinpath(dir, "FooBar3_inc.jl")
403+
write(FooBar3_inc, "x=1\n")
404+
for code in ["Core.eval(Base, :(x=1))", "Base.include(Base, \"FooBar3_inc.jl\")"]
405+
write(FooBar3_file, code)
406+
@test_warn "Evaluation into the closed module `Base` breaks incremental compilation" try
407+
Base.require(Main, :FooBar3)
408+
catch exc
409+
isa(exc, ErrorException) || rethrow()
410+
end
411+
end
412+
400413
# Test transitive dependency for #21266
401414
FooBarT_file = joinpath(dir, "FooBarT.jl")
402415
write(FooBarT_file,

0 commit comments

Comments
 (0)