-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Add basic infrastructure for binding replacement #56224
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -218,16 +218,19 @@ function _fieldnames(@nospecialize t) | |
return t.name.names | ||
end | ||
|
||
const BINDING_KIND_GLOBAL = 0x0 | ||
const BINDING_KIND_CONST = 0x1 | ||
const BINDING_KIND_CONST_IMPORT = 0x2 | ||
# N.B.: Needs to be synced with julia.h | ||
const BINDING_KIND_CONST = 0x0 | ||
const BINDING_KIND_CONST_IMPORT = 0x1 | ||
const BINDING_KIND_GLOBAL = 0x2 | ||
const BINDING_KIND_IMPLICIT = 0x3 | ||
const BINDING_KIND_EXPLICIT = 0x4 | ||
const BINDING_KIND_IMPORTED = 0x5 | ||
const BINDING_KIND_FAILED = 0x6 | ||
const BINDING_KIND_DECLARED = 0x7 | ||
const BINDING_KIND_GUARD = 0x8 | ||
|
||
is_some_const_binding(kind::UInt8) = (kind == BINDING_KIND_CONST || kind == BINDING_KIND_CONST_IMPORT) | ||
|
||
function lookup_binding_partition(world::UInt, b::Core.Binding) | ||
ccall(:jl_get_binding_partition, Ref{Core.BindingPartition}, (Any, UInt), b, world) | ||
end | ||
|
@@ -236,9 +239,27 @@ function lookup_binding_partition(world::UInt, gr::Core.GlobalRef) | |
ccall(:jl_get_globalref_partition, Ref{Core.BindingPartition}, (Any, UInt), gr, world) | ||
end | ||
|
||
partition_restriction(bpart::Core.BindingPartition) = ccall(:jl_bpart_get_restriction_value, Any, (Any,), bpart) | ||
|
||
binding_kind(bpart::Core.BindingPartition) = ccall(:jl_bpart_get_kind, UInt8, (Any,), bpart) | ||
binding_kind(m::Module, s::Symbol) = binding_kind(lookup_binding_partition(tls_world_age(), GlobalRef(m, s))) | ||
|
||
""" | ||
delete_binding(mod::Module, sym::Symbol) | ||
|
||
Force the binding `mod.sym` to be undefined again, allowing it be redefined. | ||
Note that this operation is very expensive, requirinig a full scan of all code in the system, | ||
Keno marked this conversation as resolved.
Show resolved
Hide resolved
|
||
as well as potential recompilation of any methods that (may) have used binding | ||
information. | ||
|
||
!!! warning | ||
The implementation of this functionality is currently incomplete. Do not use | ||
this method on versions that contain this disclaimer except for testing. | ||
Comment on lines
+255
to
+257
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we do add the |
||
""" | ||
function delete_binding(mod::Module, sym::Symbol) | ||
ccall(:jl_disable_binding, Cvoid, (Any,), GlobalRef(mod, sym)) | ||
end | ||
|
||
""" | ||
fieldname(x::DataType, i::Integer) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -620,6 +620,7 @@ typedef struct _jl_weakref_t { | |
jl_value_t *value; | ||
} jl_weakref_t; | ||
|
||
// N.B: Needs to be synced with runtime_internals.jl | ||
enum jl_partition_kind { | ||
// Constant: This binding partition is a constant declared using `const` | ||
// ->restriction holds the constant value | ||
|
@@ -684,7 +685,7 @@ typedef struct __attribute__((aligned(8))) _jl_binding_partition_t { | |
_Atomic(jl_ptr_kind_union_t) restriction; | ||
size_t min_world; | ||
_Atomic(size_t) max_world; | ||
_Atomic(struct _jl_binding_partition_t*) next; | ||
_Atomic(struct _jl_binding_partition_t *) next; | ||
size_t reserved; // Reserved for ->kind. Currently this holds the low bits of ->restriction during serialization | ||
} jl_binding_partition_t; | ||
|
||
|
@@ -1839,8 +1840,8 @@ JL_DLLEXPORT jl_sym_t *jl_symbol_n(const char *str, size_t len) JL_NOTSAFEPOINT; | |
JL_DLLEXPORT jl_sym_t *jl_gensym(void); | ||
JL_DLLEXPORT jl_sym_t *jl_tagged_gensym(const char *str, size_t len); | ||
JL_DLLEXPORT jl_sym_t *jl_get_root_symbol(void); | ||
JL_DLLEXPORT jl_value_t *jl_get_binding_value(jl_binding_t *b JL_PROPAGATES_ROOT) JL_NOTSAFEPOINT; | ||
JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_const(jl_binding_t *b JL_PROPAGATES_ROOT) JL_NOTSAFEPOINT; | ||
JL_DLLEXPORT jl_value_t *jl_get_binding_value(jl_binding_t *b JL_PROPAGATES_ROOT); | ||
JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_const(jl_binding_t *b JL_PROPAGATES_ROOT); | ||
Comment on lines
+1843
to
+1844
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both of these changes violate JL_NOTSAFEPOINT requirements in rtutils and codegen (the two files we aren't able to verify). I think you partly fixed one of the issues, but not the others There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I fixed rtutils. I don't think codegen assumes JL_NOTSAFEPOINT, but regardless these will be removed from codegen anyway as it gets adjusted to partition. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I saw the one place was fixed, but looks like |
||
JL_DLLEXPORT jl_value_t *jl_declare_const_gf(jl_binding_t *b, jl_module_t *mod, jl_sym_t *name); | ||
JL_DLLEXPORT jl_method_t *jl_method_def(jl_svec_t *argdata, jl_methtable_t *mt, jl_code_info_t *f, jl_module_t *module); | ||
JL_DLLEXPORT jl_code_info_t *jl_code_for_staged(jl_method_instance_t *linfo, size_t world, jl_code_instance_t **cache); | ||
|
@@ -2002,8 +2003,8 @@ 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_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_DLLEXPORT jl_value_t *jl_checked_assignonce(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs JL_MAYBE_UNROOTED); | ||
JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val(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) JL_NOTSAFEPOINT; | ||
JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val2(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) JL_NOTSAFEPOINT; | ||
JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val(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); | ||
JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val2(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); | ||
Comment on lines
+2006
to
+2007
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Curious this also didn't fail the verifier, since |
||
JL_DLLEXPORT void jl_module_using(jl_module_t *to, jl_module_t *from); | ||
JL_DLLEXPORT void jl_module_use(jl_module_t *to, jl_module_t *from, jl_sym_t *s); | ||
JL_DLLEXPORT void jl_module_use_as(jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we attempt to merge this with
Base.@invokelatest world M.a
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IanButterworth suggested that above. The syntax is reserved, but I'm not convinced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not convinced either, just observing the symmetry that the two different macros return the same result, particularly if we allow
@world
to handle call syntax or@invokelatest
to handle bindings