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.

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

ensure user-structs get the correct method table #33424

Closed
wants to merge 1 commit into from
Closed

ensure user-structs get the correct method table #33424

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
8 changes: 4 additions & 4 deletions src/datatype.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,16 +487,16 @@ JL_DLLEXPORT jl_datatype_t *jl_new_datatype(
}
else {
tn = jl_new_typename_in((jl_sym_t*)name, module);
if (super == jl_function_type || super == jl_builtin_type || jl_symbol_name(name)[0] == '#') {
// Callable objects (including compiler-generated closures) get independent method tables
// as an optimization
if (super == jl_function_type || super == jl_builtin_type) {
// Anything Julia detects as declared as a Callable objects (including compiler-generated closures)
// should get independent method tables, as a performance optimization
tn->mt = jl_new_method_table(name, module);
jl_gc_wb(tn, tn->mt);
if (jl_svec_len(parameters) > 0)
tn->mt->offs = 0;
}
else {
// Everything else, gets to use the unified table
// Everything else, must use the unified table
tn->mt = jl_nonfunction_mt;
}
}
Expand Down
51 changes: 36 additions & 15 deletions src/interpreter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,20 +109,17 @@ SECT_INTERP static void check_can_assign_type(jl_binding_t *b, jl_value_t *rhs)
void jl_reinstantiate_inner_types(jl_datatype_t *t);
void jl_reset_instantiate_inner_types(jl_datatype_t *t);

SECT_INTERP void jl_set_datatype_super(jl_datatype_t *tt, jl_value_t *super)
SECT_INTERP void check_datatype_super(jl_value_t *super, jl_typename_t *tt_name, jl_sym_t *name)
{
if (!jl_is_datatype(super) || !jl_is_abstracttype(super) ||
tt->name == ((jl_datatype_t*)super)->name ||
tt_name == ((jl_datatype_t*)super)->name ||
jl_subtype(super, (jl_value_t*)jl_vararg_type) ||
jl_is_tuple_type(super) ||
jl_is_namedtuple_type(super) ||
jl_subtype(super, (jl_value_t*)jl_type_type) ||
jl_subtype(super, (jl_value_t*)jl_builtin_type)) {
jl_errorf("invalid subtyping in definition of %s",
jl_symbol_name(tt->name->name));
jl_errorf("invalid subtyping in definition of %s", jl_symbol_name(name));
}
tt->super = (jl_datatype_t*)super;
jl_gc_wb(tt, tt->super);
}

static void eval_abstracttype(jl_expr_t *ex, interpreter_state *s)
Expand All @@ -144,7 +141,11 @@ static void eval_abstracttype(jl_expr_t *ex, interpreter_state *s)
name = (jl_value_t*)jl_globalref_name(name);
}
assert(jl_is_symbol(name));
dt = jl_new_abstracttype(name, modu, NULL, (jl_svec_t*)para);
if (jl_is_globalref(args[2])) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a little too unpredictable, since it can change e.g. based on whether the definition came from a macro.

super = jl_eval_global_var(jl_globalref_mod(args[2]), jl_globalref_name(args[2]));
check_datatype_super(super, NULL, (jl_sym_t*)name);
}
dt = jl_new_abstracttype(name, modu, (jl_datatype_t*)super, (jl_svec_t*)para);
w = dt->name->wrapper;
jl_binding_t *b = jl_get_binding_wr(modu, (jl_sym_t*)name, 1);
temp = b->value;
Expand All @@ -153,8 +154,12 @@ static void eval_abstracttype(jl_expr_t *ex, interpreter_state *s)
jl_gc_wb_binding(b, w);
JL_TRY {
inside_typedef = 1;
super = eval_value(args[2], s);
jl_set_datatype_super(dt, super);
if (super == NULL) {
super = eval_value(args[2], s);
check_datatype_super(super, dt->name, (jl_sym_t*)name);
dt->super = (jl_datatype_t*)super;
jl_gc_wb(dt, super);
}
jl_reinstantiate_inner_types(dt);
}
JL_CATCH {
Expand Down Expand Up @@ -195,7 +200,11 @@ static void eval_primitivetype(jl_expr_t *ex, interpreter_state *s)
if (nb < 1 || nb >= (1 << 23) || (nb & 7) != 0)
jl_errorf("invalid number of bits in primitive type %s",
jl_symbol_name((jl_sym_t*)name));
dt = jl_new_primitivetype(name, modu, NULL, (jl_svec_t*)para, nb);
if (jl_is_globalref(args[3])) {
super = jl_eval_global_var(jl_globalref_mod(args[3]), jl_globalref_name(args[3]));
check_datatype_super(super, NULL, (jl_sym_t*)name);
}
dt = jl_new_primitivetype(name, modu, (jl_datatype_t*)super, (jl_svec_t*)para, nb);
w = dt->name->wrapper;
jl_binding_t *b = jl_get_binding_wr(modu, (jl_sym_t*)name, 1);
temp = b->value;
Expand All @@ -204,8 +213,12 @@ static void eval_primitivetype(jl_expr_t *ex, interpreter_state *s)
jl_gc_wb_binding(b, w);
JL_TRY {
inside_typedef = 1;
super = eval_value(args[3], s);
jl_set_datatype_super(dt, super);
if (super == NULL) {
super = eval_value(args[3], s);
check_datatype_super(super, dt->name, (jl_sym_t*)name);
dt->super = (jl_datatype_t*)super;
jl_gc_wb(dt, super);
}
jl_reinstantiate_inner_types(dt);
}
JL_CATCH {
Expand Down Expand Up @@ -240,7 +253,11 @@ static void eval_structtype(jl_expr_t *ex, interpreter_state *s)
assert(jl_is_symbol(name));
assert(jl_is_svec(para));
temp = eval_value(args[2], s); // field names
dt = jl_new_datatype((jl_sym_t*)name, modu, NULL, (jl_svec_t*)para,
if (jl_is_globalref(args[3])) {
super = jl_eval_global_var(jl_globalref_mod(args[3]), jl_globalref_name(args[3]));
check_datatype_super(super, NULL, (jl_sym_t*)name);
}
dt = jl_new_datatype((jl_sym_t*)name, modu, (jl_datatype_t*)super, (jl_svec_t*)para,
(jl_svec_t*)temp, NULL,
0, args[5]==jl_true ? 1 : 0, jl_unbox_long(args[6]));
w = dt->name->wrapper;
Expand All @@ -255,8 +272,12 @@ static void eval_structtype(jl_expr_t *ex, interpreter_state *s)
JL_TRY {
inside_typedef = 1;
// operations that can fail
super = eval_value(args[3], s);
jl_set_datatype_super(dt, super);
if (super == NULL) {
super = eval_value(args[3], s);
check_datatype_super(super, dt->name, (jl_sym_t*)name);
dt->super = (jl_datatype_t*)super;
jl_gc_wb(dt, super);
}
dt->types = (jl_svec_t*)eval_value(args[4], s);
jl_gc_wb(dt, dt->types);
for (size_t i = 0; i < jl_svec_len(dt->types); i++) {
Expand Down
6 changes: 6 additions & 0 deletions test/core.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2404,6 +2404,12 @@ for f in (:(Core.arrayref), :((::typeof(Core.arrayref))), :((::Core.IntrinsicFun
@test_throws ErrorException("cannot add methods to a builtin function") @eval $f() = 1
end

# issue #33370: make sure user types get the right method table
let gs = gensym()
@eval struct $gs <: Core.Number; end
@test eval(gs).name.mt === Symbol.name.mt
end

# issue #8798
let
npy_typestrs = Dict("b1"=>Bool,
Expand Down