Track "permanently defined" status for bindings

base/compiler/abstractinterpretation.jl

@@ -2599,7 +2599,7 @@ function abstract_eval_isdefined(interp::AbstractInterpreter, e::Expr, vtypes::U
     elseif isa(sym, GlobalRef)
         if InferenceParams(interp).assume_bindings_static
             rt = Const(isdefined_globalref(sym))
-        elseif isdefinedconst_globalref(sym)
+        elseif permanently_isdefined_globalref(sym)
             rt = Const(true)
         else
             effects = Effects(EFFECTS_TOTAL; consistent=ALWAYS_FALSE)
@@ -2820,10 +2820,10 @@ function override_effects(effects::Effects, override::EffectsOverride)
 end
 
 isdefined_globalref(g::GlobalRef) = !iszero(ccall(:jl_globalref_boundp, Cint, (Any,), g))
-isdefinedconst_globalref(g::GlobalRef) = isconst(g) && isdefined_globalref(g)
+permanently_isdefined_globalref(g::GlobalRef) = !iszero(ccall(:jl_globalref_permboundp, Cint, (Any,), g))
 
 function abstract_eval_globalref_type(g::GlobalRef)
-    if isdefinedconst_globalref(g)
+    if isconst(g) && isdefined_globalref(g)
         return Const(ccall(:jl_get_globalref_value, Any, (Any,), g))
     end
     ty = ccall(:jl_get_binding_type, Any, (Any, Any), g.mod, g.name)
@@ -2849,7 +2849,7 @@ function abstract_eval_globalref(interp::AbstractInterpreter, g::GlobalRef, sv::
         else
             rt = Union{}
         end
-    elseif isdefinedconst_globalref(g)
+    elseif permanently_isdefined_globalref(g)
         nothrow = true
     end
     return RTEffects(rt, nothrow ? Union{} : UndefVarError, Effects(EFFECTS_TOTAL; consistent, nothrow, inaccessiblememonly))

base/compiler/optimize.jl

@@ -298,7 +298,8 @@ function stmt_effect_flags(𝕃ₒ::AbstractLattice, @nospecialize(stmt), @nospe
     isa(stmt, GotoNode) && return (true, false, true)
     isa(stmt, GotoIfNot) && return (true, false, ⊑(𝕃ₒ, argextype(stmt.cond, src), Bool))
     if isa(stmt, GlobalRef)
-        nothrow = consistent = isdefinedconst_globalref(stmt)
+        nothrow = permanently_isdefined_globalref(stmt)
+        consistent = nothrow && isconst(stmt.mod, stmt.name)
         return (consistent, nothrow, nothrow)
     elseif isa(stmt, Expr)
         (; head, args) = stmt

base/compiler/tfuncs.jl

@@ -398,7 +398,7 @@ end
         if a1 === Module
             hasintersect(widenconst(sym), Symbol) || return Bottom
             if isa(sym, Const) && isa(sym.val, Symbol) && isa(arg1, Const) &&
-               isdefinedconst_globalref(GlobalRef(arg1.val::Module, sym.val::Symbol))
+               permanently_isdefined_globalref(GlobalRef(arg1.val::Module, sym.val::Symbol))
                 return Const(true)
             end
         elseif isa(sym, Const)
@@ -3041,7 +3041,7 @@ end
     if M isa Const && s isa Const
         M, s = M.val, s.val
         if M isa Module && s isa Symbol
-            return isdefinedconst_globalref(GlobalRef(M, s))
+            return permanently_isdefined_globalref(GlobalRef(M, s))
         end
     end
     return false

src/codegen.cpp

@@ -1975,6 +1975,7 @@ class jl_codectx_t {
     int nargs = 0;
     int nvargs = -1;
     bool is_opaque_closure = false;
+    bool is_toplevel = false;
 
     Value *pgcstack = NULL;
     Instruction *topalloca = NULL;
@@ -3215,7 +3216,7 @@ static jl_cgval_t emit_globalop(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t *s
     Value *bp = global_binding_pointer(ctx, mod, sym, &bnd, true);
     if (bp == NULL)
         return jl_cgval_t();
-    if (bnd && !bnd->constp) {
+    if (bnd && !bnd->constp && !ctx.is_toplevel) {
         jl_value_t *ty = jl_atomic_load_relaxed(&bnd->ty);
         if (ty != nullptr) {
             const std::string fname = issetglobal ? "setglobal!" : isreplaceglobal ? "replaceglobal!" : isswapglobal ? "swapglobal!" : ismodifyglobal ? "modifyglobal!" : "setglobalonce!";
@@ -3227,7 +3228,7 @@ static jl_cgval_t emit_globalop(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t *s
                     return jl_cgval_t();
             }
             bool isboxed = true;
-            bool maybe_null = jl_atomic_load_relaxed(&bnd->value) == NULL;
+            bool maybe_null = (!bnd->isdefined && !bnd->constp) || (jl_atomic_load_relaxed(&bnd->value) == NULL);
             return typed_store(ctx,
                                julia_binding_pvalue(ctx, bp),
                                rval, cmp, ty,
@@ -5612,7 +5613,7 @@ static jl_cgval_t emit_isdefined(jl_codectx_t &ctx, jl_value_t *sym)
         }
         jl_binding_t *bnd = jl_get_binding(modu, name);
         if (bnd) {
-            if (jl_atomic_load_acquire(&bnd->value) != NULL && bnd->constp)
+            if (jl_atomic_load_acquire(&bnd->value) != NULL && (bnd->constp || bnd->isdefined))
                 return mark_julia_const(ctx, jl_true);
             Value *bp = julia_binding_gv(ctx, bnd);
             bp = julia_binding_pvalue(ctx, bp);
@@ -8101,7 +8102,6 @@ static jl_llvm_functions_t
     ctx.source = src;
 
     std::map<int, BasicBlock*> labels;
-    bool toplevel = false;
     ctx.module = jl_is_method(lam->def.method) ? lam->def.method->module : lam->def.module;
     ctx.linfo = lam;
     ctx.name = TSM.getModuleUnlocked()->getModuleIdentifier().data();
@@ -8121,7 +8121,7 @@ static jl_llvm_functions_t
         if (vn != jl_unused_sym)
             ctx.vaSlot = ctx.nargs - 1;
     }
-    toplevel = !jl_is_method(lam->def.method);
+    ctx.is_toplevel = !jl_is_method(lam->def.method);
     ctx.rettype = jlrettype;
     ctx.funcName = ctx.name;
     ctx.spvals_ptr = NULL;
@@ -8480,7 +8480,7 @@ static jl_llvm_functions_t
     allocate_gc_frame(ctx, b0);
     Value *last_age = NULL;
     Value *world_age_field = get_last_age_field(ctx);
-    if (toplevel || ctx.is_opaque_closure) {
+    if (ctx.is_toplevel || ctx.is_opaque_closure) {
         jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_gcframe);
         last_age = ai.decorateInst(ctx.builder.CreateAlignedLoad(
             ctx.types().T_size, world_age_field, ctx.types().alignof_ptr));
@@ -8996,7 +8996,7 @@ static jl_llvm_functions_t
     Instruction &prologue_end = ctx.builder.GetInsertBlock()->back();
 
     // step 11a. For top-level code, load the world age
-    if (toplevel && !ctx.is_opaque_closure) {
+    if (ctx.is_toplevel && !ctx.is_opaque_closure) {
         LoadInst *world = ctx.builder.CreateAlignedLoad(ctx.types().T_size,
             prepare_global_in(jl_Module, jlgetworld_global), ctx.types().alignof_ptr);
         world->setOrdering(AtomicOrdering::Acquire);
@@ -9298,7 +9298,7 @@ static jl_llvm_functions_t
             }
 
             mallocVisitStmt(sync_bytes, have_dbg_update);
-            if (toplevel || ctx.is_opaque_closure)
+            if (ctx.is_toplevel || ctx.is_opaque_closure)
                 ctx.builder.CreateStore(last_age, world_age_field);
             assert(type_is_ghost(retty) || returninfo.cc == jl_returninfo_t::SRet ||
                 retval->getType() == ctx.f->getReturnType());
@@ -9648,7 +9648,7 @@ static jl_llvm_functions_t
                         I.setDebugLoc(topdebugloc);
                     }
                 }
-                if (toplevel && !ctx.is_opaque_closure && !in_prologue) {
+                if (ctx.is_toplevel && !ctx.is_opaque_closure && !in_prologue) {
                     // we're at toplevel; insert an atomic barrier between every instruction
                     // TODO: inference is invalid if this has any effect (which it often does)
                     LoadInst *world = new LoadInst(ctx.types().T_size,

src/julia.h

@@ -639,7 +639,7 @@ typedef struct _jl_binding_t {
     uint8_t imported:1;
     uint8_t usingfailed:1;
     uint8_t deprecated:2; // 0=not deprecated, 1=renamed, 2=moved to another package
-    uint8_t padding:1;
+    uint8_t isdefined:1;  // 0=value may be null, 1=value is non-null (and always will be)
 } jl_binding_t;
 
 typedef struct {
@@ -1950,11 +1950,13 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_type(jl_module_t *m, jl_sym_t *var);
 JL_DLLEXPORT jl_binding_t *jl_get_binding_wr(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var);
 JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var);
 JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var);
+JL_DLLEXPORT int jl_permboundp(jl_module_t *m, jl_sym_t *var);
 JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var);
 JL_DLLEXPORT int jl_binding_resolved_p(jl_module_t *m, jl_sym_t *var);
 JL_DLLEXPORT int jl_is_const(jl_module_t *m, jl_sym_t *var);
 JL_DLLEXPORT int jl_globalref_is_const(jl_globalref_t *gr);
 JL_DLLEXPORT int jl_globalref_boundp(jl_globalref_t *gr);
+JL_DLLEXPORT int jl_globalref_permboundp(jl_globalref_t *gr);
 JL_DLLEXPORT jl_value_t *jl_get_globalref_value(jl_globalref_t *gr);
 JL_DLLEXPORT jl_value_t *jl_get_global(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var);
 JL_DLLEXPORT void jl_set_global(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *var, jl_value_t *val JL_ROOTED_ARGUMENT);

src/module.c

@@ -182,7 +182,7 @@ static jl_binding_t *new_binding(jl_module_t *mod, jl_sym_t *name)
     b->imported = 0;
     b->deprecated = 0;
     b->usingfailed = 0;
-    b->padding = 0;
+    b->isdefined = 0;
     JL_GC_PUSH1(&b);
     b->globalref = jl_new_globalref(mod, name, b);
     JL_GC_POP();
@@ -191,29 +191,65 @@ static jl_binding_t *new_binding(jl_module_t *mod, jl_sym_t *name)
 
 extern jl_mutex_t jl_modules_mutex;
 
+/**
+ * Return true if the provided module is "open".
+ *
+ * An open module is one that has not yet been serialized.
+ **/
+static int module_is_open(jl_module_t *m)
+{
+    JL_LOCK(&jl_modules_mutex);
+    int open = ptrhash_has(&jl_current_modules, (void*)m);
+    if (!open && jl_module_init_order != NULL) {
+        size_t i, l = jl_array_len(jl_module_init_order);
+        for (i = 0; i < l; i++) {
+            if (m == (jl_module_t*)jl_array_ptr_ref(jl_module_init_order, i)) {
+                open = 1;
+                break;
+            }
+        }
+    }
+    JL_UNLOCK(&jl_modules_mutex);
+    return open;
+}
+
+/**
+ * Return true if modifications of this module's bindings are guaranteed not to
+ * experience "mutation tearing".
+ *
+ * Mutation tearing happens when the modification of an object in a dependency's
+ * pkgimage ends up not recorded in the pkgimage being generated - instead the
+ * mutation is dropped.
+ *
+ * That behavior exists because there is no coherent way to resolve modifications
+ * across pkgimages. Objects must be serialized exactly once across all pkgimages,
+ * and once serialized, mutations to an object live only as long as the current
+ * Julia session.
+ **/
+static int module_mutation_is_persistent(jl_module_t *m)
+{
+    // No tearing if we're not generating a pkgimage - the semantic "timeline" dies
+    // with the current Julia session
+    if (!jl_generating_output())
+        return 1;
+
+    // No tearing if we're generating the sysimage - no objects have been serialized yet
+    if (!jl_options.incremental)
+        return 1;
+
+    // No tearing if the module/binding we're modifying have not been serialized yet
+    // (i.e. they are "open")
+    return module_is_open(m);
+}
+
 static void check_safe_newbinding(jl_module_t *m, jl_sym_t *var)
 {
     if (jl_current_task->ptls->in_pure_callback)
         jl_errorf("new globals cannot be created in a generated function");
-    if (jl_options.incremental && jl_generating_output()) {
-        JL_LOCK(&jl_modules_mutex);
-        int open = ptrhash_has(&jl_current_modules, (void*)m);
-        if (!open && jl_module_init_order != NULL) {
-            size_t i, l = jl_array_len(jl_module_init_order);
-            for (i = 0; i < l; i++) {
-                if (m == (jl_module_t*)jl_array_ptr_ref(jl_module_init_order, i)) {
-                    open = 1;
-                    break;
-                }
-            }
-        }
-        JL_UNLOCK(&jl_modules_mutex);
-        if (!open) {
-            jl_errorf("Creating a new global in closed module `%s` (`%s`) breaks incremental compilation "
-                      "because the side effects will not be permanent.",
-                      jl_symbol_name(m->name), jl_symbol_name(var));
-        }
-    }
+    if (!module_mutation_is_persistent(m))
+        jl_errorf("Creating a new global in closed module `%s` (`%s`) breaks incremental compilation "
+                  "because the side effects will not be permanent.",
+                  jl_symbol_name(m->name), jl_symbol_name(var));
 }
 
 static jl_module_t *jl_binding_dbgmodule(jl_binding_t *b, jl_module_t *m, jl_sym_t *var) JL_GLOBALLY_ROOTED;
@@ -684,6 +720,12 @@ JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var) // unlike most queries
     return b && (jl_atomic_load(&b->value) != NULL);
 }
 
+JL_DLLEXPORT int jl_permboundp(jl_module_t *m, jl_sym_t *var)
+{
+    jl_binding_t *b = jl_get_binding(m, var);
+    return b && (b->isdefined || b->constp) && (jl_atomic_load(&b->value) != NULL);
+}
+
 JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var)
 {
     jl_binding_t *b = jl_get_module_binding(m, var, 0);
@@ -834,6 +876,13 @@ JL_DLLEXPORT int jl_globalref_boundp(jl_globalref_t *gr)
     return b && jl_atomic_load_relaxed(&b->value) != NULL;
 }
 
+JL_DLLEXPORT int jl_globalref_permboundp(jl_globalref_t *gr)
+{
+    jl_binding_t *b = gr->binding;
+    b = jl_resolve_owner(b, gr->mod, gr->name, NULL);
+    return b && (b->isdefined || b->constp) && (jl_atomic_load_relaxed(&b->value) != NULL);
+}
+
 JL_DLLEXPORT int jl_is_const(jl_module_t *m, jl_sym_t *var)
 {
     jl_binding_t *b = jl_get_binding(m, var);
@@ -925,8 +974,11 @@ jl_value_t *jl_check_binding_wr(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var
 
 JL_DLLEXPORT void jl_checked_assignment(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs)
 {
+    assert(rhs != NULL);
     if (jl_check_binding_wr(b, mod, var, rhs, 1) != NULL) {
-        jl_atomic_store_release(&b->value, rhs);
+        jl_value_t *old = jl_atomic_exchange(&b->value, rhs);
+        if (__unlikely(old == NULL) && module_is_open(mod))
+            b->isdefined = 1;
         jl_gc_wb(b, rhs);
     }
 }