Add global tests and fixes

TODO: codegen for typed_store/typed_load of a global to correctly throw UndefVarError
TODO: optimization for modifyglobal and memoryrefmodify

Author: vtjnash

base/compiler/tfuncs.jl

@@ -1987,23 +1987,19 @@ end
     return memoryrefget_tfunc(𝕃, mem, order, boundscheck)
 end
 @nospecs function memoryrefmodify!_tfunc(𝕃::AbstractLattice, mem, op, v, success_order, failure_order, boundscheck)
-    T = get_binding_type_tfunc(𝕃, M, s)
+    T = _memoryref_elemtype(mem)
     T === Bottom && return Bottom
-    T isa Const || return Pair
-    T = T.value
-    return Pair{T, T}
+    PT = Const(Pair)
+    return instanceof_tfunc(apply_type_tfunc(𝕃, PT, T, T), true)[1]
 end
-@nospecs function memoryrefreplace!_tfunc(𝕃𝕃::AbstractLattice, mem, x, v, success_order, failure_order, boundscheck)
-    v = memoryrefset!_tfunc(𝕃, o, f, v)
-    v === Bottom && return Bottom
-    T = get_binding_type_tfunc(𝕃, M, s)
+@nospecs function memoryrefreplace!_tfunc(𝕃::AbstractLattice, mem, x, v, success_order, failure_order, boundscheck)
+    T = _memoryref_elemtype(mem)
     T === Bottom && return Bottom
-    T isa Const || return ccall(:jl_apply_cmpswap_type, Any, (Any,), T) where T
-    T = T.value
-    return ccall(:jl_apply_cmpswap_type, Any, (Any,), T)
+    PT = Const(ccall(:jl_apply_cmpswap_type, Any, (Any,), T) where T)
+    return instanceof_tfunc(apply_type_tfunc(𝕃, PT, T), true)[1]
 end
 @nospecs function memoryrefsetonce!_tfunc(𝕃::AbstractLattice, mem, v, success_order, failure_order, boundscheck)
-    memoryrefsetonce!_tfunc(𝕃, o, f, v) === Bottom && return Bottom
+    memoryrefsetonce!_tfunc(𝕃, mem, v) === Bottom && return Bottom
     return Bool
 end
 
@@ -2064,7 +2060,7 @@ add_tfunc(memoryrefoffset, 1, 1, memoryrefoffset_tfunc, 5)
     return true
 end
 
-function memoryref_elemtype(@nospecialize mem)
+@nospecs function memoryref_elemtype(@nospecialize mem)
     m = widenconst(mem)
     if !has_free_typevars(m) && m <: GenericMemoryRef
         m0 = m
@@ -2080,6 +2076,23 @@ function memoryref_elemtype(@nospecialize mem)
     return Any
 end
 
+@nospecs function _memoryref_elemtype(@nospecialize mem)
+    m = widenconst(mem)
+    if !has_free_typevars(m) && m <: GenericMemoryRef
+        m0 = m
+        if isa(m, UnionAll)
+            m = unwrap_unionall(m0)
+        end
+        if isa(m, DataType)
+            T = m.parameters[2]
+            valid_as_lattice(T, true) || return Bottom
+            has_free_typevars(T) || return Const(T)
+            return rewrap_unionall(Type{T}, m0)
+        end
+    end
+    return Type
+end
+
 @nospecs function opaque_closure_tfunc(𝕃::AbstractLattice, arg, lb, ub, source, env::Vector{Any}, linfo::MethodInstance)
     argt, argt_exact = instanceof_tfunc(arg)
     lbt, lb_exact = instanceof_tfunc(lb)
@@ -3085,7 +3098,7 @@ end
         return Bottom
     end
     T = get_binding_type_tfunc(𝕃, M, s)
-    T isa Const && return T.value
+    T isa Const && return T.val
     return Any
 end
 @nospecs function setglobal!_tfunc(𝕃::AbstractLattice, M, s, v, order=Symbol)
@@ -3095,27 +3108,27 @@ end
     return v
 end
 @nospecs function swapglobal!_tfunc(𝕃::AbstractLattice, M, s, v, order=Symbol)
-    setglobal_tfunc(𝕃, o, f, v) === Bottom && return Bottom
-    return getglobal_tfunc(𝕃, o, f)
+    setglobal!_tfunc(𝕃, M, s, v) === Bottom && return Bottom
+    return getglobal_tfunc(𝕃, M, s)
 end
 @nospecs function modifyglobal!_tfunc(𝕃::AbstractLattice, M, s, op, v, order=Symbol)
     T = get_binding_type_tfunc(𝕃, M, s)
     T === Bottom && return Bottom
     T isa Const || return Pair
-    T = T.value
+    T = T.val
     return Pair{T, T}
 end
-@nospecs function replaceglobal!_tfunc(𝕃𝕃::AbstractLattice, M, s, x, v, success_order=Symbol, failure_order=Symbol)
-    v = setglobal!_tfunc(𝕃, o, f, v)
+@nospecs function replaceglobal!_tfunc(𝕃::AbstractLattice, M, s, x, v, success_order=Symbol, failure_order=Symbol)
+    v = setglobal!_tfunc(𝕃, M, s, v)
     v === Bottom && return Bottom
     T = get_binding_type_tfunc(𝕃, M, s)
     T === Bottom && return Bottom
     T isa Const || return ccall(:jl_apply_cmpswap_type, Any, (Any,), T) where T
-    T = T.value
+    T = T.val
     return ccall(:jl_apply_cmpswap_type, Any, (Any,), T)
 end
 @nospecs function setglobalonce!_tfunc(𝕃::AbstractLattice, M, s, v, success_order=Symbol, failure_order=Symbol)
-    setglobal!_tfunc(𝕃, o, f, v) === Bottom && return Bottom
+    setglobal!_tfunc(𝕃, M, s, v) === Bottom && return Bottom
     return Bool
 end
 

src/builtins.c

@@ -1272,6 +1272,10 @@ JL_CALLABLE(jl_f_isdefined)
         JL_TYPECHK(isdefined, symbol, args[1]);
         m = (jl_module_t*)args[0];
         s = (jl_sym_t*)args[1];
+        if (order == jl_memory_order_unspecified)
+            order = jl_memory_order_unordered;
+        if (order < jl_memory_order_unordered)
+            jl_atomic_error("isdefined: module binding cannot be accessed non-atomically");
         int bound = jl_boundp(m, s); // seq_cst always
         return bound ? jl_true : jl_false;
     }
@@ -1389,7 +1393,7 @@ JL_CALLABLE(jl_f_set_binding_type)
     JL_TYPECHK(set_binding_type!, type, ty);
     jl_binding_t *b = jl_get_binding_wr(m, s);
     jl_value_t *old_ty = NULL;
-    if (!jl_atomic_cmpswap_release(&b->ty, &old_ty, ty) && ty != old_ty) {
+    if (!jl_atomic_cmpswap_release(&b->ty, &old_ty, ty) && ty != old_ty) { // TODO: this should use jl_types_egal instead of !=, so that setting it to a non-concrete type doesn't error unnecessarily
         if (nargs == 2)
             return jl_nothing;
         jl_errorf("cannot set type for global %s.%s. It already has a value or is already set to a different type.",

src/codegen.cpp

@@ -3181,6 +3181,7 @@ static jl_cgval_t emit_globalop(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t *s
         jl_value_t *ty = jl_atomic_load_relaxed(&bnd->ty);
         if (ty && jl_subtype(rval.typ, ty)) {
             // TODO: use typeassert here instead, coordinated with jl_check_binding_wr
+            // TODO: this is wrong for ismodifyglobal as well, since its check is part of typed_store
             //emit_typecheck(ctx, rval, ty, "typeassert");
             //rval = update_julia_type(ctx, rval, ty);
             bool isboxed = true;

src/datatype.c

@@ -1789,7 +1789,7 @@ JL_DLLEXPORT jl_value_t *jl_get_nth_field_checked(jl_value_t *v, size_t i)
     return r;
 }
 
-void set_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *rhs, int isatomic) JL_NOTSAFEPOINT
+inline void set_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *rhs, int isatomic) JL_NOTSAFEPOINT
 {
     size_t offs = jl_field_offset(st, i);
     if (rhs == NULL) { // TODO: this should be invalid, but it happens frequently in ircode.c
@@ -1839,7 +1839,7 @@ void set_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *rhs,
     }
 }
 
-jl_value_t *swap_bits(jl_value_t *ty, char *v, uint8_t *psel, jl_value_t *parent, jl_value_t *rhs, enum atomic_kind isatomic)
+inline jl_value_t *swap_bits(jl_value_t *ty, char *v, uint8_t *psel, jl_value_t *parent, jl_value_t *rhs, enum atomic_kind isatomic)
 {
     jl_value_t *rty = jl_typeof(rhs);
     int hasptr;
@@ -1913,20 +1913,26 @@ jl_value_t *swap_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_
     }
 }
 
-jl_value_t *modify_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *parent, jl_value_t *op, jl_value_t *rhs, int isatomic)
+inline jl_value_t *modify_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *parent, jl_value_t *op, jl_value_t *rhs, int isatomic, jl_module_t *mod, jl_sym_t *name)
 {
     jl_value_t *r = isatomic ? jl_atomic_load(p) : jl_atomic_load_relaxed(p);
-    if (__unlikely(r == NULL))
+    if (__unlikely(r == NULL)) {
+        if (mod && name)
+            jl_undefined_var_error(name, (jl_value_t*)mod);
         jl_throw(jl_undefref_exception);
+    }
     jl_value_t **args;
     JL_GC_PUSHARGS(args, 2);
     args[0] = r;
     while (1) {
         args[1] = rhs;
         jl_value_t *y = jl_apply_generic(op, args, 2);
         args[1] = y;
-        if (!jl_isa(y, ty))
-            jl_type_error("modifyfield!", ty, y);
+        if (!jl_isa(y, ty)) {
+            if (mod && name)
+                jl_errorf("cannot assign an incompatible value to the global %s.%s.", jl_symbol_name(mod->name), jl_symbol_name(name));
+            jl_type_error(jl_is_genericmemory(parent) ? "memoryrefmodify!" : "modifyfield!", ty, y);
+        }
         if (isatomic ? jl_atomic_cmpswap(p, &r, y) : jl_atomic_cmpswap_release(p, &r, y)) {
             jl_gc_wb(parent, y);
             break;
@@ -1943,7 +1949,7 @@ jl_value_t *modify_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *pa
     return args[0];
 }
 
-jl_value_t *modify_bits(jl_value_t *ty, char *p, uint8_t *psel, jl_value_t *parent, jl_value_t *op, jl_value_t *rhs, enum atomic_kind isatomic)
+inline jl_value_t *modify_bits(jl_value_t *ty, char *p, uint8_t *psel, jl_value_t *parent, jl_value_t *op, jl_value_t *rhs, enum atomic_kind isatomic)
 {
     int hasptr;
     int isunion = psel != NULL;
@@ -1981,8 +1987,9 @@ jl_value_t *modify_bits(jl_value_t *ty, char *p, uint8_t *psel, jl_value_t *pare
         args[1] = rhs;
         jl_value_t *y = jl_apply_generic(op, args, 2);
         args[1] = y;
-        if (!jl_isa(y, ty))
-            jl_type_error("modifyfield!", ty, y);
+        if (!jl_isa(y, ty)) {
+            jl_type_error(jl_is_genericmemory(parent) ? "memoryrefmodify!" : "modifyfield!", ty, y);
+        }
         jl_value_t *yty = jl_typeof(y);
         if (isatomic && !needlock) {
             assert(yty == rty);
@@ -2039,15 +2046,15 @@ jl_value_t *modify_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_valu
     jl_value_t *ty = jl_field_type_concrete(st, i);
     char *p = (char*)v + offs;
     if (jl_field_isptr(st, i)) {
-        return modify_value(ty, (_Atomic(jl_value_t*)*)p, v, op, rhs, isatomic);
+        return modify_value(ty, (_Atomic(jl_value_t*)*)p, v, op, rhs, isatomic, NULL, NULL);
     }
     else {
         uint8_t *psel = jl_is_uniontype(ty) ? (uint8_t*)&p[jl_field_size(st, i) - 1] : NULL;
         return modify_bits(ty, p, psel, v, op, rhs, isatomic ? isatomic_object : isatomic_none);
     }
 }
 
-jl_value_t *replace_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *parent, jl_value_t *expected, jl_value_t *rhs, int isatomic)
+inline jl_value_t *replace_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *parent, jl_value_t *expected, jl_value_t *rhs, int isatomic, jl_module_t *mod, jl_sym_t *name)
 {
     jl_datatype_t *rettyp = jl_apply_cmpswap_type(ty);
     JL_GC_PROMISE_ROOTED(rettyp); // (JL_ALWAYS_LEAFTYPE)
@@ -2057,8 +2064,11 @@ jl_value_t *replace_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *p
         success = isatomic ? jl_atomic_cmpswap(p, &r, rhs) : jl_atomic_cmpswap_release(p, &r, rhs);
         if (success)
             jl_gc_wb(parent, rhs);
-        if (__unlikely(r == NULL))
+        if (__unlikely(r == NULL)) {
+            if (mod && name)
+                jl_undefined_var_error(name, (jl_value_t*)mod);
             jl_throw(jl_undefref_exception);
+        }
         if (success || !jl_egal(r, expected))
             break;
     }
@@ -2068,7 +2078,7 @@ jl_value_t *replace_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *p
     return r;
 }
 
-jl_value_t *replace_bits(jl_value_t *ty, char *p, uint8_t *psel, jl_value_t *parent, jl_value_t *expected, jl_value_t *rhs, enum atomic_kind isatomic)
+inline jl_value_t *replace_bits(jl_value_t *ty, char *p, uint8_t *psel, jl_value_t *parent, jl_value_t *expected, jl_value_t *rhs, enum atomic_kind isatomic)
 {
     jl_datatype_t *rettyp = jl_apply_cmpswap_type(ty);
     JL_GC_PROMISE_ROOTED(rettyp); // (JL_ALWAYS_LEAFTYPE)
@@ -2144,7 +2154,7 @@ jl_value_t *replace_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_val
     size_t offs = jl_field_offset(st, i);
     char *p = (char*)v + offs;
     if (jl_field_isptr(st, i)) {
-        return replace_value(ty, (_Atomic(jl_value_t*)*)p, v, expected, rhs, isatomic);
+        return replace_value(ty, (_Atomic(jl_value_t*)*)p, v, expected, rhs, isatomic, NULL, NULL);
     }
     else {
         size_t fsz = jl_field_size(st, i);
@@ -2154,7 +2164,7 @@ jl_value_t *replace_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_val
     }
 }
 
-int setonce_bits(jl_datatype_t *rty, char *p, jl_value_t *parent, jl_value_t *rhs, enum atomic_kind isatomic)
+inline int setonce_bits(jl_datatype_t *rty, char *p, jl_value_t *parent, jl_value_t *rhs, enum atomic_kind isatomic)
 {
     size_t fsz = jl_datatype_size((jl_datatype_t*)rty); // need to shrink-wrap the final copy
     assert(rty->layout->first_ptr >= 0);

src/genericmemory.c

@@ -570,7 +570,7 @@ JL_DLLEXPORT jl_value_t *jl_memoryrefmodify(jl_genericmemoryref_t m, jl_value_t
     char *data = (char*)m.ptr_or_offset;
     if (layout->flags.arrayelem_isboxed) {
         assert(data - (char*)m.mem->ptr < sizeof(jl_value_t*) * m.mem->length);
-        return modify_value(eltype, (_Atomic(jl_value_t*)*)data, owner, op, rhs, isatomic);
+        return modify_value(eltype, (_Atomic(jl_value_t*)*)data, owner, op, rhs, isatomic, NULL, NULL);
     }
     size_t fsz = layout->size;
     uint8_t *psel = NULL;
@@ -597,7 +597,7 @@ JL_DLLEXPORT jl_value_t *jl_memoryrefreplace(jl_genericmemoryref_t m, jl_value_t
     char *data = (char*)m.ptr_or_offset;
     if (layout->flags.arrayelem_isboxed) {
         assert(data - (char*)m.mem->ptr < sizeof(jl_value_t*) * m.mem->length);
-        return replace_value(eltype, (_Atomic(jl_value_t*)*)data, owner, expected, rhs, isatomic);
+        return replace_value(eltype, (_Atomic(jl_value_t*)*)data, owner, expected, rhs, isatomic, NULL, NULL);
     }
     uint8_t *psel = NULL;
     if (layout->flags.arrayelem_isunion) {

src/julia_internal.h

@@ -779,9 +779,9 @@ jl_value_t *modify_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_valu
 jl_value_t *replace_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *expected, jl_value_t *rhs, int isatomic);
 int set_nth_fieldonce(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *rhs, int isatomic);
 jl_value_t *swap_bits(jl_value_t *ty, char *v, uint8_t *psel, jl_value_t *parent, jl_value_t *rhs, enum atomic_kind isatomic);
-jl_value_t *replace_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *parent, jl_value_t *expected, jl_value_t *rhs, int isatomic);
+jl_value_t *replace_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *parent, jl_value_t *expected, jl_value_t *rhs, int isatomic, jl_module_t *mod, jl_sym_t *name);
 jl_value_t *replace_bits(jl_value_t *ty, char *p, uint8_t *psel, jl_value_t *parent, jl_value_t *expected, jl_value_t *rhs, enum atomic_kind isatomic);
-jl_value_t *modify_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *parent, jl_value_t *op, jl_value_t *rhs, int isatomic);
+jl_value_t *modify_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *parent, jl_value_t *op, jl_value_t *rhs, int isatomic, jl_module_t *mod, jl_sym_t *name);
 jl_value_t *modify_bits(jl_value_t *ty, char *p, uint8_t *psel, jl_value_t *parent, jl_value_t *op, jl_value_t *rhs, enum atomic_kind isatomic);
 int setonce_bits(jl_datatype_t *rty, char *p, jl_value_t *owner, jl_value_t *rhs, enum atomic_kind isatomic);
 jl_expr_t *jl_exprn(jl_sym_t *head, size_t n);

src/module.c

@@ -926,19 +926,26 @@ JL_DLLEXPORT jl_value_t *jl_checked_swap(jl_binding_t *b, jl_module_t *mod, jl_s
     jl_check_binding_wr(b, mod, var, rhs, 0);
     jl_value_t *old = jl_atomic_exchange(&b->value, rhs);
     jl_gc_wb(b, rhs);
+    if (__unlikely(old == NULL))
+        jl_undefined_var_error(var, (jl_value_t*)mod);
     return old;
 }
 
 JL_DLLEXPORT jl_value_t *jl_checked_replace(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *expected, jl_value_t *rhs)
 {
     jl_value_t *ty = jl_check_binding_wr(b, mod, var, rhs, 0);
-    return replace_value(ty, &b->value, (jl_value_t*)b, expected, rhs, 1);
+    return replace_value(ty, &b->value, (jl_value_t*)b, expected, rhs, 1, mod, var);
 }
 
 JL_DLLEXPORT jl_value_t *jl_checked_modify(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *op, jl_value_t *rhs)
 {
-    jl_value_t *ty = jl_check_binding_wr(b, mod, var, rhs, 0);
-    return modify_value(ty, &b->value, (jl_value_t*)b, op, rhs, 1);
+    jl_value_t *ty = NULL;
+    if (jl_atomic_cmpswap_relaxed(&b->ty, &ty, (jl_value_t*)jl_any_type))
+        ty = (jl_value_t*)jl_any_type;
+    if (b->constp)
+        jl_errorf("invalid redefinition of constant %s.%s",
+                  jl_symbol_name(mod->name), jl_symbol_name(var));
+    return modify_value(ty, &b->value, (jl_value_t*)b, op, rhs, 1, mod, var);
 }
 
 JL_DLLEXPORT jl_value_t *jl_checked_assignonce(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs )