Skip to content

Commit 0e96f04

Browse files
authored
Refine errors for incremental compilation (JuliaLang#35410)
* 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 11379eb commit 0e96f04

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
{
@@ -133,8 +139,7 @@ jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex)
133139

134140
// copy parent environment info into submodule
135141
newm->uuid = parent_module->uuid;
136-
if (jl_base_module &&
137-
(jl_value_t*)parent_module == jl_get_global(jl_base_module, jl_symbol("__toplevel__"))) {
142+
if (jl_is__toplevel__mod(parent_module)) {
138143
newm->parent = newm;
139144
jl_register_root_module(newm);
140145
}
@@ -827,25 +832,33 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval(jl_module_t *m, jl_value_t *v)
827832
return jl_toplevel_eval_flex(m, v, 1, 0);
828833
}
829834

835+
// Check module `m` is open for `eval/include`, or throw an error.
836+
void jl_check_open_for(jl_module_t *m, const char* funcname)
837+
{
838+
if (jl_options.incremental && jl_generating_output()) {
839+
if (!ptrhash_has(&jl_current_modules, (void*)m) && !jl_is__toplevel__mod(m)) {
840+
if (m != jl_main_module) { // TODO: this was grand-fathered in
841+
const char* name = jl_symbol_name(m->name);
842+
jl_errorf("Evaluation into the closed module `%s` breaks incremental compilation "
843+
"because the side effects will not be permanent. "
844+
"This is likely due to some other module mutating `%s` with `%s` during "
845+
"precompilation - don't do this.", name, name, funcname);
846+
}
847+
}
848+
}
849+
}
850+
830851
JL_DLLEXPORT jl_value_t *jl_toplevel_eval_in(jl_module_t *m, jl_value_t *ex)
831852
{
832853
jl_ptls_t ptls = jl_get_ptls_states();
833854
if (ptls->in_pure_callback)
834855
jl_error("eval cannot be used in a generated function");
856+
jl_check_open_for(m, "eval");
835857
jl_value_t *v = NULL;
836858
int last_lineno = jl_lineno;
837859
const char *last_filename = jl_filename;
838860
jl_lineno = 1;
839861
jl_filename = "none";
840-
if (jl_options.incremental && jl_generating_output()) {
841-
if (!ptrhash_has(&jl_current_modules, (void*)m)) {
842-
if (m != jl_main_module) { // TODO: this was grand-fathered in
843-
jl_printf(JL_STDERR, "WARNING: eval into closed module %s:\n", jl_symbol_name(m->name));
844-
jl_static_show(JL_STDERR, ex);
845-
jl_printf(JL_STDERR, "\n ** incremental compilation may be fatally broken for this module **\n\n");
846-
}
847-
}
848-
}
849862
JL_TRY {
850863
v = jl_toplevel_eval(m, ex);
851864
}

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)