Skip to content

Commit

Permalink
Refine errors for incremental compilation (JuliaLang#35410)
Browse files Browse the repository at this point in the history
* 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()
  • Loading branch information
c42f authored and ztultrebor committed Apr 14, 2020
1 parent 15c81c8 commit 58a7c1b
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 11 deletions.
1 change: 1 addition & 0 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,7 @@ jl_value_t *jl_parse_eval_all(const char *fname,
jl_ptls_t ptls = jl_get_ptls_states();
if (ptls->in_pure_callback)
jl_error("cannot use include inside a generated function");
jl_check_open_for(inmodule, "include");
jl_ast_context_t *ctx = jl_ast_ctx_enter();
fl_context_t *fl_ctx = &ctx->fl;
value_t f, ast, expression;
Expand Down
1 change: 1 addition & 0 deletions src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,7 @@ void jl_init_main_module(void);
int jl_is_submodule(jl_module_t *child, jl_module_t *parent);
jl_array_t *jl_get_loaded_modules(void);

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

jl_value_t *jl_eval_global_var(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *e);
Expand Down
35 changes: 24 additions & 11 deletions src/toplevel.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ jl_array_t *jl_get_loaded_modules(void)
return NULL;
}

static int jl_is__toplevel__mod(jl_module_t *mod)
{
return jl_base_module &&
(jl_value_t*)mod == jl_get_global(jl_base_module, jl_symbol("__toplevel__"));
}

// TODO: add locks around global state mutation operations
jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex)
{
Expand All @@ -133,8 +139,7 @@ jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex)

// copy parent environment info into submodule
newm->uuid = parent_module->uuid;
if (jl_base_module &&
(jl_value_t*)parent_module == jl_get_global(jl_base_module, jl_symbol("__toplevel__"))) {
if (jl_is__toplevel__mod(parent_module)) {
newm->parent = newm;
jl_register_root_module(newm);
}
Expand Down Expand Up @@ -827,25 +832,33 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval(jl_module_t *m, jl_value_t *v)
return jl_toplevel_eval_flex(m, v, 1, 0);
}

// Check module `m` is open for `eval/include`, or throw an error.
void jl_check_open_for(jl_module_t *m, const char* funcname)
{
if (jl_options.incremental && jl_generating_output()) {
if (!ptrhash_has(&jl_current_modules, (void*)m) && !jl_is__toplevel__mod(m)) {
if (m != jl_main_module) { // TODO: this was grand-fathered in
const char* name = jl_symbol_name(m->name);
jl_errorf("Evaluation into the closed module `%s` breaks incremental compilation "
"because the side effects will not be permanent. "
"This is likely due to some other module mutating `%s` with `%s` during "
"precompilation - don't do this.", name, name, funcname);
}
}
}
}

JL_DLLEXPORT jl_value_t *jl_toplevel_eval_in(jl_module_t *m, jl_value_t *ex)
{
jl_ptls_t ptls = jl_get_ptls_states();
if (ptls->in_pure_callback)
jl_error("eval cannot be used in a generated function");
jl_check_open_for(m, "eval");
jl_value_t *v = NULL;
int last_lineno = jl_lineno;
const char *last_filename = jl_filename;
jl_lineno = 1;
jl_filename = "none";
if (jl_options.incremental && jl_generating_output()) {
if (!ptrhash_has(&jl_current_modules, (void*)m)) {
if (m != jl_main_module) { // TODO: this was grand-fathered in
jl_printf(JL_STDERR, "WARNING: eval into closed module %s:\n", jl_symbol_name(m->name));
jl_static_show(JL_STDERR, ex);
jl_printf(JL_STDERR, "\n ** incremental compilation may be fatally broken for this module **\n\n");
}
}
}
JL_TRY {
v = jl_toplevel_eval(m, ex);
}
Expand Down
13 changes: 13 additions & 0 deletions test/precompile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,19 @@ try
occursin("ERROR: LoadError: break me", exc.msg) && rethrow()
end

# Test that trying to eval into closed modules during precompilation is an error
FooBar3_file = joinpath(dir, "FooBar3.jl")
FooBar3_inc = joinpath(dir, "FooBar3_inc.jl")
write(FooBar3_inc, "x=1\n")
for code in ["Core.eval(Base, :(x=1))", "Base.include(Base, \"FooBar3_inc.jl\")"]
write(FooBar3_file, code)
@test_warn "Evaluation into the closed module `Base` breaks incremental compilation" try
Base.require(Main, :FooBar3)
catch exc
isa(exc, ErrorException) || rethrow()
end
end

# Test transitive dependency for #21266
FooBarT_file = joinpath(dir, "FooBarT.jl")
write(FooBarT_file,
Expand Down

0 comments on commit 58a7c1b

Please sign in to comment.