Skip to content

Align interpreter and codegen error behavior of setglobal! and friends #59766

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 8, 2025
Merged

Align interpreter and codegen error behavior of setglobal! and friends #59766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions Compiler/src/abstractinterpretation.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2485,7 +2485,7 @@ function abstract_eval_setglobal!(interp::AbstractInterpreter, sv::AbsIntState,
(rt, exct) = global_assignment_rt_exct(interp, sv, saw_latestworld, gr, v)
return CallMeta(rt, exct, Effects(setglobal!_effects, nothrow=exct===Bottom), GlobalAccessInfo(convert(Core.Binding, gr)))
end
return CallMeta(Union{}, TypeError, EFFECTS_THROWS, NoCallInfo())
return CallMeta(Union{}, Union{TypeError, ErrorException}, EFFECTS_THROWS, NoCallInfo())
end
⊑ = partialorder(typeinf_lattice(interp))
if !(hasintersect(widenconst(M), Module) && hasintersect(widenconst(s), Symbol))
Expand Down Expand Up @@ -3751,7 +3751,7 @@ end

function global_assignment_rt_exct(interp::AbstractInterpreter, sv::AbsIntState, saw_latestworld::Bool, g::GlobalRef, @nospecialize(newty))
if saw_latestworld
return Pair{Any,Any}(newty, ErrorException)
return Pair{Any,Any}(newty, Union{TypeError, ErrorException})
end
newty′ = RefValue{Any}(newty)
(valid_worlds, ret) = scan_partitions(interp, g, sv.world) do interp::AbstractInterpreter, ::Core.Binding, partition::Core.BindingPartition
Expand All @@ -3766,15 +3766,16 @@ function global_assignment_binding_rt_exct(interp::AbstractInterpreter, partitio
if is_some_guard(kind)
return Pair{Any,Any}(newty, ErrorException)
elseif is_some_const_binding(kind) || is_some_imported(kind)
return Pair{Any,Any}(Bottom, ErrorException)
# N.B.: Backdating should not improve inference in an earlier world
return Pair{Any,Any}(kind == PARTITION_KIND_BACKDATED_CONST ? newty : Bottom, ErrorException)
end
ty = kind == PARTITION_KIND_DECLARED ? Any : partition_restriction(partition)
wnewty = widenconst(newty)
if !hasintersect(wnewty, ty)
return Pair{Any,Any}(Bottom, ErrorException)
return Pair{Any,Any}(Bottom, TypeError)
elseif !(wnewty <: ty)
retty = tmeet(typeinf_lattice(interp), newty, ty)
return Pair{Any,Any}(retty, ErrorException)
return Pair{Any,Any}(retty, TypeError)
end
return Pair{Any,Any}(newty, Bottom)
end
Expand Down
2 changes: 1 addition & 1 deletion Compiler/test/inference.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6458,7 +6458,7 @@ end
global invalid_setglobal!_exct_modeling::Int
@test Base.infer_exception_type((Float64,)) do x
setglobal!(@__MODULE__, :invalid_setglobal!_exct_modeling, x)
end == ErrorException
end == TypeError

# Issue #58257 - Hang in inference during BindingPartition resolution
module A58257
Expand Down
2 changes: 1 addition & 1 deletion base/boot.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ struct TypeError <: Exception
# `context` optionally adds extra detail, e.g. the name of the type parameter
# that got a bad value.
func::Symbol
context::Union{AbstractString,Symbol}
context::Union{AbstractString,GlobalRef,Symbol}
expected::Type
got
TypeError(func, context, @nospecialize(expected::Type), @nospecialize(got)) =
Expand Down
3 changes: 3 additions & 0 deletions base/errorshow.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ function showerror(io::IO, ex::TypeError)
end
if ex.context == ""
ctx = "in $(ex.func)"
elseif isa(ex.context, Core.GlobalRef)
gr = ex.context
ctx = "in $(ex.func) of global binding `$(gr.mod).$(gr.name)`"
elseif ex.func === :var"keyword argument"
ctx = "in keyword argument $(ex.context)"
else
Expand Down
3 changes: 1 addition & 2 deletions src/codegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3231,8 +3231,7 @@ static jl_cgval_t emit_globalop(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t *s
if (ty != nullptr) {
const std::string fname = issetglobal ? "setglobal!" : isreplaceglobal ? "replaceglobal!" : isswapglobal ? "swapglobal!" : ismodifyglobal ? "modifyglobal!" : "setglobalonce!";
if (!ismodifyglobal) {
// TODO: use typeassert in jl_check_binding_assign_value too
emit_typecheck(ctx, rval, ty, "typeassert");
emit_typecheck(ctx, rval, ty, fname.c_str());
rval = update_julia_type(ctx, rval, ty);
if (rval.typ == jl_bottom_type)
return jl_cgval_t();
Expand Down
13 changes: 6 additions & 7 deletions src/datatype.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1992,11 +1992,11 @@ jl_value_t *swap_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_
}
}

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)
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_binding_t *b, 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 (mod && name)
if (b)
jl_undefined_var_error(name, (jl_value_t*)mod);
jl_throw(jl_undefref_exception);
}
Expand All @@ -2007,11 +2007,10 @@ inline jl_value_t *modify_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_valu
args[1] = rhs;
jl_value_t *y = jl_apply_generic(op, args, 2);
args[1] = 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));
if (b)
jl_check_binding_assign_value(b, mod, name, y, "modifyglobal!");
else if (!jl_isa(y, ty))
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;
Expand Down Expand Up @@ -2125,7 +2124,7 @@ 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, NULL, NULL);
return modify_value(ty, (_Atomic(jl_value_t*)*)p, v, op, rhs, isatomic, NULL, NULL, NULL);
}
else {
uint8_t *psel = jl_is_uniontype(ty) ? (uint8_t*)&p[jl_field_size(st, i) - 1] : NULL;
Expand Down
2 changes: 1 addition & 1 deletion src/genericmemory.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,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, NULL, NULL);
return modify_value(eltype, (_Atomic(jl_value_t*)*)data, owner, op, rhs, isatomic, NULL, NULL, NULL);
}
size_t fsz = layout->size;
uint8_t *psel = NULL;
Expand Down
6 changes: 5 additions & 1 deletion src/julia.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2016,7 +2016,7 @@ JL_DLLEXPORT jl_value_t *jl_get_module_binding_or_nothing(jl_module_t *m, jl_sym

// get binding for reading
JL_DLLEXPORT jl_binding_t *jl_get_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var);
JL_DLLEXPORT jl_value_t *jl_module_globalref(jl_module_t *m, jl_sym_t *var);
JL_DLLEXPORT jl_value_t *jl_module_globalref(jl_module_t *m, jl_sym_t *var) JL_GLOBALLY_ROOTED;
JL_DLLEXPORT jl_value_t *jl_get_binding_type(jl_module_t *m, jl_sym_t *var);
// get binding for assignment
JL_DLLEXPORT void jl_check_binding_currently_writable(jl_binding_t *b, jl_module_t *m, jl_sym_t *s);
Expand Down Expand Up @@ -2086,6 +2086,10 @@ JL_DLLEXPORT void JL_NORETURN jl_type_error_rt(const char *fname,
const char *context,
jl_value_t *ty JL_MAYBE_UNROOTED,
jl_value_t *got JL_MAYBE_UNROOTED);
JL_DLLEXPORT void JL_NORETURN jl_type_error_global(const char *fname,
jl_module_t *mod, jl_sym_t *sym,
jl_value_t *ty JL_MAYBE_UNROOTED,
jl_value_t *got JL_MAYBE_UNROOTED);
JL_DLLEXPORT void JL_NORETURN jl_undefined_var_error(jl_sym_t *var, jl_value_t *scope JL_MAYBE_UNROOTED);
JL_DLLEXPORT void JL_NORETURN jl_has_no_field_error(jl_datatype_t *t, jl_sym_t *var);
JL_DLLEXPORT void JL_NORETURN jl_argument_error(char *str);
Expand Down
3 changes: 2 additions & 1 deletion src/julia_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -882,7 +882,7 @@ int set_nth_fieldonce(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *rh
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_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_module_t *mod, jl_sym_t *name);
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_binding_t *b, 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);
Expand All @@ -895,6 +895,7 @@ jl_array_t *jl_get_loaded_modules(void);
JL_DLLEXPORT int jl_datatype_isinlinealloc(jl_datatype_t *ty, int pointerfree);
int jl_type_equality_is_identity(jl_value_t *t1, jl_value_t *t2) JL_NOTSAFEPOINT;

jl_value_t *jl_check_binding_assign_value(jl_binding_t *b JL_PROPAGATES_ROOT, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs JL_MAYBE_UNROOTED, const char *msg);
void jl_binding_set_type(jl_binding_t *b, jl_module_t *mod, jl_sym_t *sym, jl_value_t *ty);
JL_DLLEXPORT void jl_declare_global(jl_module_t *m, jl_value_t *arg, jl_value_t *set_type, int strong);
JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val3(jl_binding_t *b JL_ROOTING_ARGUMENT, jl_module_t *mod, jl_sym_t *var, jl_value_t *val JL_ROOTED_ARGUMENT JL_MAYBE_UNROOTED, enum jl_partition_kind, size_t new_world) JL_GLOBALLY_ROOTED;
Expand Down
18 changes: 8 additions & 10 deletions src/module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1886,34 +1886,32 @@ void jl_binding_deprecation_warning(jl_binding_t *b)

// For a generally writable binding (checked using jl_check_binding_currently_writable in this world age), check whether
// we can actually write the value `rhs` to it.
jl_value_t *jl_check_binding_assign_value(jl_binding_t *b JL_PROPAGATES_ROOT, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs JL_MAYBE_UNROOTED)
jl_value_t *jl_check_binding_assign_value(jl_binding_t *b JL_PROPAGATES_ROOT, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs JL_MAYBE_UNROOTED, const char *msg)
{
JL_GC_PUSH1(&rhs); // callee-rooted
jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age);
enum jl_partition_kind kind = jl_binding_kind(bpart);
assert(kind == PARTITION_KIND_DECLARED || kind == PARTITION_KIND_GLOBAL);
jl_value_t *old_ty = kind == PARTITION_KIND_DECLARED ? (jl_value_t*)jl_any_type : bpart->restriction;
JL_GC_PROMISE_ROOTED(old_ty);
if (old_ty != (jl_value_t*)jl_any_type && jl_typeof(rhs) != old_ty) {
if (!jl_isa(rhs, old_ty))
jl_errorf("cannot assign an incompatible value to the global %s.%s.",
jl_symbol_name(mod->name), jl_symbol_name(var));
if (old_ty != (jl_value_t*)jl_any_type && jl_typeof(rhs) != old_ty && !jl_isa(rhs, old_ty)) {
jl_type_error_global(msg, mod, var, old_ty, rhs);
}
JL_GC_POP();
return old_ty;
}

JL_DLLEXPORT void jl_checked_assignment(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs)
{
if (jl_check_binding_assign_value(b, mod, var, rhs) != NULL) {
if (jl_check_binding_assign_value(b, mod, var, rhs, "setglobal!") != NULL) {
jl_atomic_store_release(&b->value, rhs);
jl_gc_wb(b, rhs);
}
}

JL_DLLEXPORT jl_value_t *jl_checked_swap(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs)
{
jl_check_binding_assign_value(b, mod, var, rhs);
jl_check_binding_assign_value(b, mod, var, rhs, "swapglobal!");
jl_value_t *old = jl_atomic_exchange(&b->value, rhs);
jl_gc_wb(b, rhs);
if (__unlikely(old == NULL))
Expand All @@ -1923,7 +1921,7 @@ JL_DLLEXPORT jl_value_t *jl_checked_swap(jl_binding_t *b, jl_module_t *mod, jl_s

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_assign_value(b, mod, var, rhs);
jl_value_t *ty = jl_check_binding_assign_value(b, mod, var, rhs, "replaceglobal!");
return replace_value(ty, &b->value, (jl_value_t*)b, expected, rhs, 1, mod, var);
}

Expand All @@ -1937,12 +1935,12 @@ JL_DLLEXPORT jl_value_t *jl_checked_modify(jl_binding_t *b, jl_module_t *mod, jl
jl_symbol_name(mod->name), jl_symbol_name(var));
jl_value_t *ty = bpart->restriction;
JL_GC_PROMISE_ROOTED(ty);
return modify_value(ty, &b->value, (jl_value_t*)b, op, rhs, 1, mod, var);
return modify_value(ty, &b->value, (jl_value_t*)b, op, rhs, 1, b, 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 )
{
jl_check_binding_assign_value(b, mod, var, rhs);
jl_check_binding_assign_value(b, mod, var, rhs, "setglobalonce!");
jl_value_t *old = NULL;
if (jl_atomic_cmpswap(&b->value, &old, rhs))
jl_gc_wb(b, rhs);
Expand Down
10 changes: 10 additions & 0 deletions src/rtutils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,16 @@ JL_DLLEXPORT void JL_NORETURN jl_type_error_rt(const char *fname, const char *co
jl_throw(ex);
}

JL_DLLEXPORT void JL_NORETURN jl_type_error_global(const char *fname, jl_module_t *mod, jl_sym_t *sym,
jl_value_t *expected JL_MAYBE_UNROOTED,
jl_value_t *got JL_MAYBE_UNROOTED)
{
jl_value_t *gr = jl_module_globalref(mod, sym);
jl_value_t *ex = jl_new_struct(jl_typeerror_type, jl_symbol(fname), gr, expected, got);
Comment on lines +128 to +129
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing GC pushes here (the correctness of your annotations is not checked in this file)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both the symbol and the globalref should be globally rooted here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both expected and got are marked as unrooted

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see what you're saying - will fix.

jl_throw(ex);
}


// with function name or description only
JL_DLLEXPORT void JL_NORETURN jl_type_error(const char *fname,
jl_value_t *expected JL_MAYBE_UNROOTED,
Expand Down
5 changes: 4 additions & 1 deletion test/core.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8153,7 +8153,10 @@ end
setglobal!(m, :x, 2, :release)
@test m.x === 2
@test_throws ConcurrencyViolationError setglobal!(m, :x, 3, :not_atomic)
@test_throws ErrorException setglobal!(m, :x, 4., :release)
@test_throws TypeError setglobal!(m, :x, 4., :release)

f_set_bad_type(m) = setglobal!(m, :x, 4., :release)
@test_throws TypeError f_set_bad_type(m)

m.x = 1
@test m.x === 1
Expand Down