Refine errors for incremental compilation (#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()

Author: c42f

src/ast.c

@@ -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;

src/julia_internal.h

@@ -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);

src/toplevel.c

@@ -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)
 {
@@ -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);
     }
@@ -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);
     }

test/precompile.jl

@@ -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,