Skip to content

Commit

Permalink
fix #36104, assign global name during type definitions (#36121)
Browse files Browse the repository at this point in the history
also fixes #21816
  • Loading branch information
JeffBezanson authored Jun 4, 2020
1 parent 186c3bb commit 095e92d
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 31 deletions.
2 changes: 1 addition & 1 deletion base/compiler/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ is_dt_const_field(fld::Int) = (
function const_datatype_getfield_tfunc(@nospecialize(sv), fld::Int)
if fld == DATATYPE_INSTANCE_FIELDINDEX
return isdefined(sv, fld) ? Const(getfield(sv, fld)) : Union{}
elseif is_dt_const_field(fld)
elseif is_dt_const_field(fld) && isdefined(sv, fld)
return Const(getfield(sv, fld))
end
return nothing
Expand Down
57 changes: 34 additions & 23 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -1250,6 +1250,27 @@ JL_CALLABLE(jl_f__setsuper)

void jl_reinstantiate_inner_types(jl_datatype_t *t);

static int equiv_field_types(jl_value_t *old, jl_value_t *ft)
{
size_t nf = jl_svec_len(ft);
if (jl_svec_len(old) != nf)
return 0;
size_t i;
for (i = 0; i < nf; i++) {
jl_value_t *ta = jl_svecref(old, i);
jl_value_t *tb = jl_svecref(ft, i);
if (jl_has_free_typevars(ta)) {
if (!jl_has_free_typevars(tb) || !jl_egal(ta, tb))
return 0;
}
else if (jl_has_free_typevars(tb) || jl_typeof(ta) != jl_typeof(tb) ||
!jl_types_equal(ta, tb)) {
return 0;
}
}
return 1;
}

JL_CALLABLE(jl_f__typebody)
{
JL_NARGS(_typebody!, 1, 2);
Expand All @@ -1258,16 +1279,23 @@ JL_CALLABLE(jl_f__typebody)
if (nargs == 2) {
jl_value_t *ft = args[1];
JL_TYPECHK(_typebody!, simplevector, ft);
dt->types = (jl_svec_t*)ft;
jl_gc_wb(dt, ft);
for (size_t i = 0; i < jl_svec_len(dt->types); i++) {
jl_value_t *elt = jl_svecref(dt->types, i);
size_t nf = jl_svec_len(ft);
for (size_t i = 0; i < nf; i++) {
jl_value_t *elt = jl_svecref(ft, i);
if ((!jl_is_type(elt) && !jl_is_typevar(elt)) || jl_is_vararg_type(elt)) {
jl_type_error_rt(jl_symbol_name(dt->name->name),
"type definition",
(jl_value_t*)jl_type_type, elt);
}
}
if (dt->types != NULL) {
if (!equiv_field_types((jl_value_t*)dt->types, ft))
jl_errorf("invalid redefinition of type %s", jl_symbol_name(dt->name->name));
}
else {
dt->types = (jl_svec_t*)ft;
jl_gc_wb(dt, ft);
}
}

JL_TRY {
Expand All @@ -1294,15 +1322,13 @@ static int equiv_type(jl_value_t *ta, jl_value_t *tb)
dta->name->name == dtb->name->name &&
dta->abstract == dtb->abstract &&
dta->mutabl == dtb->mutabl &&
dta->size == dtb->size &&
(jl_svec_len(jl_field_names(dta)) != 0 || dta->size == dtb->size) &&
dta->ninitialized == dtb->ninitialized &&
jl_egal((jl_value_t*)jl_field_names(dta), (jl_value_t*)jl_field_names(dtb)) &&
jl_nparams(dta) == jl_nparams(dtb) &&
jl_svec_len(dta->types) == jl_svec_len(dtb->types)))
jl_nparams(dta) == jl_nparams(dtb)))
return 0;
jl_value_t *a=NULL, *b=NULL;
int ok = 1;
size_t i, nf = jl_svec_len(dta->types);
JL_GC_PUSH2(&a, &b);
a = jl_rewrap_unionall((jl_value_t*)dta->super, dta->name->wrapper);
b = jl_rewrap_unionall((jl_value_t*)dtb->super, dtb->name->wrapper);
Expand All @@ -1328,21 +1354,6 @@ static int equiv_type(jl_value_t *ta, jl_value_t *tb)
a = jl_instantiate_unionall(ua, (jl_value_t*)ub->var);
b = ub->body;
}
assert(jl_is_datatype(a) && jl_is_datatype(b));
a = (jl_value_t*)jl_get_fieldtypes((jl_datatype_t*)a);
b = (jl_value_t*)jl_get_fieldtypes((jl_datatype_t*)b);
for (i = 0; i < nf; i++) {
jl_value_t *ta = jl_svecref(a, i);
jl_value_t *tb = jl_svecref(b, i);
if (jl_has_free_typevars(ta)) {
if (!jl_has_free_typevars(tb) || !jl_egal(ta, tb))
goto no;
}
else if (jl_has_free_typevars(tb) || jl_typeof(ta) != jl_typeof(tb) ||
!jl_types_equal(ta, tb)) {
goto no;
}
}
JL_GC_POP();
return 1;
no:
Expand Down
3 changes: 3 additions & 0 deletions src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -1773,6 +1773,9 @@ JL_DLLEXPORT jl_svec_t *jl_compute_fieldtypes(jl_datatype_t *st JL_PROPAGATES_RO
assert(n > 0 && "expected empty case to be handled during construction");
//if (n == 0)
// return ((st->types = jl_emptysvec));
if (wt->types == NULL)
jl_errorf("cannot determine field types of incomplete type %s",
jl_symbol_name(st->name->name));
jl_typeenv_t *env = (jl_typeenv_t*)alloca(n * sizeof(jl_typeenv_t));
for (i = 0; i < n; i++) {
env[i].var = (jl_tvar_t*)jl_svecref(wt->parameters, i);
Expand Down
33 changes: 26 additions & 7 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,8 @@
(defs2 (if (null? defs)
(default-inner-ctors name field-names field-types params bounds locs)
defs))
(min-initialized (min (ctors-min-initialized defs) (length fields))))
(min-initialized (min (ctors-min-initialized defs) (length fields)))
(prev (make-ssavalue)))
(let ((dups (has-dups field-names)))
(if dups (error (string "duplicate field name: \"" (car dups) "\" is not unique"))))
(for-each (lambda (v)
Expand All @@ -898,16 +899,29 @@
(local-def ,name)
,@(map (lambda (v) `(local ,v)) params)
,@(map (lambda (n v) (make-assignment n (bounds-to-TypeVar v #t))) params bounds)
(toplevel-only struct)
(toplevel-only struct (outerref ,name))
(= ,name (call (core _structtype) (thismodule) (inert ,name) (call (core svec) ,@params)
(call (core svec) ,@(map quotify field-names))
,mut ,min-initialized))
(call (core _setsuper!) ,name ,super)
(call (core _typebody!) ,name (call (core svec) ,@field-types))
(if (&& (isdefined (outerref ,name))
(call (core _equiv_typedef) (outerref ,name) ,name))
(null)
(if (isdefined (outerref ,name))
(block
(= ,prev (outerref ,name))
(if (call (core _equiv_typedef) ,prev ,name)
;; if this is compatible with an old definition, use the existing type object
;; and its parameters
(block (= ,name ,prev)
,@(if (pair? params)
`((= (tuple ,@params) (|.|
,(foldl (lambda (_ x) `(|.| ,x (quote body)))
prev
params)
(quote parameters))))
'()))
;; otherwise do an assignment to trigger an error
(= (outerref ,name) ,name)))
(= (outerref ,name) ,name))
(call (core _typebody!) ,name (call (core svec) ,@field-types))
(null)))
;; "inner" constructors
(scope-block
Expand Down Expand Up @@ -3351,7 +3365,12 @@ f(x) = yt(x)
((atom? e) e)
(else
(case (car e)
((quote top core globalref outerref thismodule toplevel-only line break inert module toplevel null true false meta) e)
((quote top core globalref outerref thismodule line break inert module toplevel null true false meta) e)
((toplevel-only)
;; hack to avoid generating a (method x) expr for struct types
(if (eq? (cadr e) 'struct)
(put! defined (caddr e) #t))
e)
((=)
(let ((var (cadr e))
(rhs (cl-convert (caddr e) fname lam namemap defined toplevel interp)))
Expand Down
17 changes: 17 additions & 0 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7221,3 +7221,20 @@ struct AVL35416{K,V}
avl:: Union{Nothing,Node35416{AVL35416{K,V},<:K,<:V}}
end
@test AVL35416(Node35416{AVL35416{Integer,AbstractString},Int,String}()) isa AVL35416{Integer,AbstractString}

# issue #36104
module M36104
struct T36104
v::Vector{M36104.T36104}
end
struct T36104 # check that redefining it works, issue #21816
v::Vector{T36104}
end
end
@test fieldtypes(M36104.T36104) == (Vector{M36104.T36104},)
@test_throws ErrorException("expected") @eval(struct X36104; x::error("expected"); end)
@test @isdefined(X36104)
struct X36104; x::Int; end
@test fieldtypes(X36104) == (Int,)
primitive type P36104 8 end
@test_throws ErrorException("invalid redefinition of constant P36104") @eval(primitive type P36104 16 end)

0 comments on commit 095e92d

Please sign in to comment.