Skip to content
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

External Method Tables: Fixup #40866

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Address some review comments.
  • Loading branch information
maleadt committed Jun 8, 2021
commit 8117e11a0fc986efd6c47056a433242162f59edf
3 changes: 1 addition & 2 deletions base/experimental.jl
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,7 @@ Create a new MethodTable in the current module, bound to `name`. This method tab
used with the [`Experimental.@overlay`](@ref) macro to define methods for a function without
adding them to the global method table.
"""
macro MethodTable(name)
isa(name, Symbol) || error("name must be a symbol")
macro MethodTable(name::Symbol)
esc(quote
const $name = ccall(:jl_new_method_table, Any, (Any, Any), $(quot(name)), $(__module__))
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Since we're bypassing the normal sanity checks in eval for pure and precompile-safe, we should add those to jl_new_method_table so that dump.c is safe

Copy link
Member

Choose a reason for hiding this comment

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

OK. Which sanity checks are those specifically? The !ptls->in_pure_callback?

end)
Expand Down
6 changes: 4 additions & 2 deletions src/dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,8 @@ static void jl_serialize_value_(jl_serializer_state *s, jl_value_t *v, int as_li
jl_methtable_t *mt = (jl_methtable_t*)m->external_mt;
jl_serialize_value(s, mt->module);
jl_serialize_value(s, mt->name);
} else {
}
else {
jl_serialize_value(s, (jl_value_t*)m->external_mt);
}
if (!(serialization_mode & METHOD_INTERNAL))
Expand Down Expand Up @@ -1498,7 +1499,8 @@ static jl_value_t *jl_deserialize_value_method(jl_serializer_state *s, jl_value_
m->external_mt = jl_get_global(mt_mod, mt_name);
jl_gc_wb(m, m->external_mt);
assert(jl_typeis(m->external_mt, jl_methtable_type));
} else {
}
else {
m->external_mt = jl_deserialize_value(s, &m->external_mt);
jl_gc_wb(m, m->external_mt);
}
Expand Down
2 changes: 0 additions & 2 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1888,8 +1888,6 @@ JL_DLLEXPORT jl_value_t *jl_matching_methods(jl_tupletype_t *types, jl_value_t *
return (jl_value_t*)jl_an_empty_vec_any;
if (mt == jl_nothing)
mt = (jl_value_t*)jl_method_table_for(unw);
else if (!jl_typeis(mt, jl_methtable_type))
jl_error("matching_method: `mt` is not a method table");
if ((jl_value_t*)mt == jl_nothing)
return jl_false; // indeterminate - ml_matches can't deal with this case
return ml_matches((jl_methtable_t*)mt, 0, types, lim, include_ambiguous, 1, world, 1, min_valid, max_valid, ambig);
Expand Down