Skip to content

Commit

Permalink
Consolidate various isdefined functionality into a new builtin
Browse files Browse the repository at this point in the history
In #54999 I extended `:isdefined` with the ability to specify whether
or not to consider imported bindings defined. As a result, we now have
two mechanisms for querying `isdefined` on globals (the other being
`Core.isdefined`) with incompatible feature sets (`Core.isdefined`
supports an atomic ordering argument, but not `allow_import`).
Additionally, only one of them had proper codegen support.
I also don't like to have IR forms for things that could be
perfectly well handled by builtin calls (along the lines of #56713).
So this tries to clean that all up by:
1. Adding a new builtin `isdefinedglobal` that has the full feature set
2. Dropping `:isdefined` on globals as an IR form (the frontend form gets
   lowered to the intrinsic if called on globals)
3. Wiring up codegen and correcting inference for that new builtin

An additional motivation is that `isdefined` on globals needs support
for partition edges (like other builtins), and having to have a special
case for :isdefined was marginally annoying. Just using an intrinsic
for this is much cleaner.

Lastly, the reason for a new intrinsic over extending the existing
`isdefined`, is that over time we've moved away from conflating
fields and globals for Module (e.g. introducing `getglobal`/`setglobal!`),
so this is a natural extension of that. Of course, the existing
behavior is retained for ordinary `isdefined`.
  • Loading branch information
Keno committed Jan 8, 2025
1 parent ec2b509 commit 6d957c3
Show file tree
Hide file tree
Showing 13 changed files with 244 additions and 95 deletions.
4 changes: 2 additions & 2 deletions Compiler/src/Compiler.jl
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ using Core: ABIOverride, Builtin, CodeInstance, IntrinsicFunction, MethodInstanc

using Base
using Base: @_foldable_meta, @_gc_preserve_begin, @_gc_preserve_end, @nospecializeinfer,
BINDING_KIND_GLOBAL, Base, BitVector, Bottom, Callable, DataTypeFieldDesc,
BINDING_KIND_GLOBAL, BINDING_KIND_UNDEF_CONST, Base, BitVector, Bottom, Callable, DataTypeFieldDesc,
EffectsOverride, Filter, Generator, IteratorSize, JLOptions, NUM_EFFECTS_OVERRIDES,
OneTo, Ordering, RefValue, SizeUnknown, _NAMEDTUPLE_NAME,
_array_for, _bits_findnext, _methods_by_ftype, _uniontypes, all, allocatedinline, any,
Expand All @@ -58,7 +58,7 @@ using Base: @_foldable_meta, @_gc_preserve_begin, @_gc_preserve_end, @nospeciali
datatype_pointerfree, decode_effects_override, diff_names, fieldindex,
generating_output, get_nospecializeinfer_sig, get_world_counter, has_free_typevars,
hasgenerator, hasintersect, indexed_iterate, isType, is_file_tracked, is_function_def,
is_meta_expr, is_meta_expr_head, is_nospecialized, is_nospecializeinfer,
is_meta_expr, is_meta_expr_head, is_nospecialized, is_nospecializeinfer, is_defined_const_binding,
is_some_const_binding, is_some_guard, is_some_imported, is_valid_intrinsic_elptr,
isbitsunion, isconcretedispatch, isdispatchelem, isexpr, isfieldatomic, isidentityfree,
iskindtype, ismutabletypename, ismutationfree, issingletontype, isvarargtype, isvatuple,
Expand Down
107 changes: 82 additions & 25 deletions Compiler/src/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2637,21 +2637,14 @@ function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f),
elseif f === Core.getfield && argtypes_are_actually_getglobal(argtypes)
return Future(abstract_eval_getglobal(interp, sv, si.saw_latestworld, argtypes))
elseif f === Core.isdefined && argtypes_are_actually_getglobal(argtypes)
exct = Bottom
if length(argtypes) == 4
order = argtypes[4]
exct = global_order_exct(order, #=loading=#true, #=storing=#false)
if !(isa(order, Const) && get_atomic_order(order.val, #=loading=#true, #=storing=#false).x >= MEMORY_ORDER_UNORDERED.x)
exct = Union{exct, ConcurrencyViolationError}
end
end
return Future(merge_exct(CallMeta(abstract_eval_isdefined(
interp,
GlobalRef((argtypes[2]::Const).val::Module,
(argtypes[3]::Const).val::Symbol),
si.saw_latestworld,
sv),
NoCallInfo()), exct))
return Future(abstract_eval_isdefinedglobal(interp, argtypes[2], argtypes[3], Const(true),
length(argtypes) == 4 ? argtypes[4] : Const(:unordered),
si.saw_latestworld, sv))
elseif f === Core.isdefinedglobal && 3 <= length(argtypes) <= 5
return Future(abstract_eval_isdefinedglobal(interp, argtypes[2], argtypes[3],
length(argtypes) >= 4 ? argtypes[4] : Const(true),
length(argtypes) >= 5 ? argtypes[5] : Const(:unordered),
si.saw_latestworld, sv))
elseif f === Core.get_binding_type
return Future(abstract_eval_get_binding_type(interp, sv, argtypes))
end
Expand Down Expand Up @@ -3203,20 +3196,76 @@ function abstract_eval_isdefined_expr(interp::AbstractInterpreter, e::Expr, ssta
return abstract_eval_isdefined(interp, sym, sstate.saw_latestworld, sv)
end

function abstract_eval_isdefined(interp::AbstractInterpreter, @nospecialize(sym), saw_latestworld::Bool, sv::AbsIntState)
const generic_isdefinedglobal_effects = Effects(EFFECTS_TOTAL, consistent=ALWAYS_FALSE, nothrow=false)
function abstract_eval_isdefinedglobal(interp::AbstractInterpreter, mod::Module, sym::Symbol, allow_import::Union{Bool, Nothing}, saw_latestworld::Bool, sv::AbsIntState)
rt = Bool
if saw_latestworld
return RTEffects(rt, Union{}, Effects(generic_isdefinedglobal_effects, nothrow=true))
end

effects = EFFECTS_TOTAL
exct = Union{}
isa(sym, Symbol) && (sym = GlobalRef(frame_module(sv), sym))
if isa(sym, GlobalRef)
rte = abstract_eval_globalref(interp, sym, saw_latestworld, sv)
partition = lookup_binding_partition!(interp, GlobalRef(mod, sym), sv)
if allow_import !== true && is_some_imported(binding_kind(partition))
if allow_import === false
rt = Const(false)
else
effects = Effects(generic_isdefinedglobal_effects, nothrow=true)
end
else
partition = walk_binding_partition!(interp, partition, sv)
rte = abstract_eval_partition_load(interp, partition)
if rte.exct == Union{}
rt = Const(true)
elseif rte.rt === Union{} && rte.exct === UndefVarError
rt = Const(false)
else
effects = Effects(EFFECTS_TOTAL; consistent=ALWAYS_FALSE)
effects = Effects(generic_isdefinedglobal_effects, nothrow=true)
end
end
return RTEffects(rt, Union{}, effects)
end

function abstract_eval_isdefinedglobal(interp::AbstractInterpreter, @nospecialize(M), @nospecialize(s), @nospecialize(allow_import_arg), @nospecialize(order_arg), saw_latestworld::Bool, sv::AbsIntState)
exct = Bottom
allow_import = true
if allow_import_arg !== nothing
if !isa(allow_import_arg, Const)
allow_import = nothing
if widenconst(allow_import_arg) != Bool
exct = Union{exct, TypeError}
end
else
allow_import = allow_import_arg.val
end
end
if order_arg !== nothing
exct = global_order_exct(order_arg, #=loading=#true, #=storing=#false)
if !(isa(order_arg, Const) && get_atomic_order(order_arg.val, #=loading=#true, #=storing=#false).x >= MEMORY_ORDER_UNORDERED.x)
exct = Union{exct, ConcurrencyViolationError}
end
end
if M isa Const && s isa Const
M, s = M.val, s.val
if M isa Module && s isa Symbol
return merge_exct(CallMeta(abstract_eval_isdefinedglobal(interp, M, s, allow_import, saw_latestworld, sv), NoCallInfo()), exct)
end
return CallMeta(Union{}, TypeError, EFFECTS_THROWS, NoCallInfo())
elseif !hasintersect(widenconst(M), Module) || !hasintersect(widenconst(s), Symbol)
return CallMeta(Union{}, TypeError, EFFECTS_THROWS, NoCallInfo())
elseif M Module && s Symbol
return CallMeta(Bool, Union{exct, UndefVarError}, generic_isdefinedglobal_effects, NoCallInfo())
end
return CallMeta(Bool, Union{exct, TypeError, UndefVarError}, generic_isdefinedglobal_effects, NoCallInfo())
end

function abstract_eval_isdefined(interp::AbstractInterpreter, @nospecialize(sym), saw_latestworld::Bool, sv::AbsIntState)
rt = Bool
effects = EFFECTS_TOTAL
exct = Union{}
if isa(sym, Symbol)
return abstract_eval_isdefinedglobal(interp, frame_module(sv), sym, true, saw_latestworld, sv)
elseif isa(sym, GlobalRef)
return abstract_eval_isdefinedglobal(interp, sym.mod, sym.name, true, saw_latestworld, sv)
elseif isexpr(sym, :static_parameter)
n = sym.args[1]::Int
if 1 <= n <= length(sv.sptypes)
Expand Down Expand Up @@ -3443,22 +3492,31 @@ function abstract_eval_globalref_type(g::GlobalRef, src::Union{CodeInfo, IRCode,
return partition_restriction(partition)
end

function abstract_eval_binding_partition!(interp::AbstractInterpreter, g::GlobalRef, sv::AbsIntState)
function lookup_binding_partition!(interp::AbstractInterpreter, g::GlobalRef, sv::AbsIntState)
force_binding_resolution!(g)
partition = lookup_binding_partition(get_inference_world(interp), g)
update_valid_age!(sv, WorldRange(partition.min_world, partition.max_world))
partition
end

function walk_binding_partition!(interp::AbstractInterpreter, partition::Core.BindingPartition, sv::AbsIntState)
while is_some_imported(binding_kind(partition))
imported_binding = partition_restriction(partition)::Core.Binding
partition = lookup_binding_partition(get_inference_world(interp), imported_binding)
update_valid_age!(sv, WorldRange(partition.min_world, partition.max_world))
end
return partition
end

function abstract_eval_binding_partition!(interp::AbstractInterpreter, g::GlobalRef, sv::AbsIntState)
partition = lookup_binding_partition!(interp, g, sv)
partition = walk_binding_partition!(interp, partition, sv)
return partition
end

function abstract_eval_partition_load(interp::AbstractInterpreter, partition::Core.BindingPartition)
if is_some_guard(binding_kind(partition))
kind = binding_kind(partition)
if is_some_guard(kind) || kind == BINDING_KIND_UNDEF_CONST
if InferenceParams(interp).assume_bindings_static
return RTEffects(Union{}, UndefVarError, EFFECTS_THROWS)
else
Expand All @@ -3468,13 +3526,12 @@ function abstract_eval_partition_load(interp::AbstractInterpreter, partition::Co
end
end

if is_some_const_binding(binding_kind(partition))
if is_defined_const_binding(kind)
rt = Const(partition_restriction(partition))
return RTEffects(rt, Union{}, Effects(EFFECTS_TOTAL, inaccessiblememonly=is_mutation_free_argtype(rt) ? ALWAYS_TRUE : ALWAYS_FALSE))
end

rt = partition_restriction(partition)

return RTEffects(rt, UndefVarError, generic_getglobal_effects)
end

Expand Down
2 changes: 1 addition & 1 deletion base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ export
fieldtype, getfield, setfield!, swapfield!, modifyfield!, replacefield!, setfieldonce!,
nfields, throw, tuple, ===, isdefined, eval,
# access to globals
getglobal, setglobal!, swapglobal!, modifyglobal!, replaceglobal!, setglobalonce!,
getglobal, setglobal!, swapglobal!, modifyglobal!, replaceglobal!, setglobalonce!, isdefinedglobal,
# ifelse, sizeof # not exported, to avoid conflicting with Base
# type reflection
<:, typeof, isa, typeassert,
Expand Down
34 changes: 34 additions & 0 deletions base/docs/basedocs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2754,6 +2754,9 @@ compatible with the stores to that location. Otherwise, if not declared as
To test whether an array element is defined, use [`isassigned`](@ref) instead.
The global variable variant is supported for compatibility with older julia
releases. For new code, prefer [`isdefinedglobal`](@ref).
See also [`@isdefined`](@ref).
# Examples
Expand Down Expand Up @@ -2781,6 +2784,37 @@ false
"""
isdefined


"""
isdefinedglobal(m::Module, s::Symbol, [allow_import::Bool=true, [order::Symbol=:unordered]])
Tests whether a global variable `s` is defined in module `m` (in the current world age).
A variable is considered defined if and only if a value may be read from this global variable
and an access will not throw. This includes both constants and global variables that have
a value set.
If `allow_import` is `false`, the global variable must be defined inside `m`
and may not be imported from another module.
See also [`@isdefined`](@ref).
# Examples
```jldoctest
julia> isdefinedglobal(Base, :sum)
true
julia> isdefinedglobal(Base, :NonExistentMethod)
false
julia> isdefinedglobal(Base, :sum, false)
true
julia> isdefinedglobal(Main, :sum, false)
false
```
"""
isdefinedglobal

"""
Memory{T}(undef, n)
Expand Down
1 change: 1 addition & 0 deletions doc/src/base/base.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ Core.replacefield!
Core.swapfield!
Core.setfieldonce!
Core.isdefined
Core.isdefinedglobal
Base.@isdefined
Base.convert
Base.promote
Expand Down
1 change: 1 addition & 0 deletions src/builtin_proto.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ DECLARE_BUILTIN(invoke);
DECLARE_BUILTIN(is);
DECLARE_BUILTIN(isa);
DECLARE_BUILTIN(isdefined);
DECLARE_BUILTIN(isdefinedglobal);
DECLARE_BUILTIN(issubtype);
DECLARE_BUILTIN(memorynew);
DECLARE_BUILTIN(memoryref);
Expand Down
28 changes: 28 additions & 0 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -1352,6 +1352,33 @@ JL_CALLABLE(jl_f_getglobal)
return v;
}

JL_CALLABLE(jl_f_isdefinedglobal)
{
jl_module_t *m = NULL;
jl_sym_t *s = NULL;
JL_NARGS(isdefined, 2, 3);
int allow_import = 1;
enum jl_memory_order order = jl_memory_order_unspecified;
JL_TYPECHK(isdefined, module, args[0]);
JL_TYPECHK(isdefined, symbol, args[1]);
if (nargs == 3) {
JL_TYPECHK(isdefined, bool, args[2]);
allow_import = jl_unbox_bool(args[2]);
}
if (nargs == 4) {
JL_TYPECHK(isdefined, symbol, args[3]);
order = jl_get_atomic_order_checked((jl_sym_t*)args[2], 1, 0);
}
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, allow_import); // seq_cst always
return bound ? jl_true : jl_false;
}

JL_CALLABLE(jl_f_setglobal)
{
enum jl_memory_order order = jl_memory_order_release;
Expand Down Expand Up @@ -2451,6 +2478,7 @@ void jl_init_primitives(void) JL_GC_DISABLED
// module bindings
jl_builtin_getglobal = add_builtin_func("getglobal", jl_f_getglobal);
jl_builtin_setglobal = add_builtin_func("setglobal!", jl_f_setglobal);
jl_builtin_isdefinedglobal = add_builtin_func("isdefinedglobal", jl_f_isdefinedglobal);
add_builtin_func("get_binding_type", jl_f_get_binding_type);
jl_builtin_swapglobal = add_builtin_func("swapglobal!", jl_f_swapglobal);
jl_builtin_replaceglobal = add_builtin_func("replaceglobal!", jl_f_replaceglobal);
Expand Down
Loading

0 comments on commit 6d957c3

Please sign in to comment.