fix #36104, assign global name during type definitions

JuliaLang · Jun 2, 2020 · 5f4fb2a · 5f4fb2a
1 parent 8c8f7a6
commit 5f4fb2a
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 40 deletions.
diff --git a/src/builtins.c b/src/builtins.c
@@ -1238,7 +1238,6 @@ JL_CALLABLE(jl_f__primitivetype)
 void jl_set_datatype_super(jl_datatype_t *tt, jl_value_t *super)
 {
  if (!jl_is_datatype(super) || !jl_is_abstracttype(super) ||
- tt->super != NULL ||
  tt->name == ((jl_datatype_t*)super)->name ||
  jl_subtype(super, (jl_value_t*)jl_vararg_type) ||
  jl_is_tuple_type(super) ||
@@ -1248,8 +1247,20 @@ void jl_set_datatype_super(jl_datatype_t *tt, jl_value_t *super)
  jl_errorf("invalid subtyping in definition of %s",
  jl_symbol_name(tt->name->name));
  }
- tt->super = (jl_datatype_t*)super;
- jl_gc_wb(tt, tt->super);
+ if (tt->super != NULL) {
+ // type redefinition; check that the new supertype is compatible
+ jl_value_t *a=NULL, *b=NULL;
+ JL_GC_PUSH2(&a, &b);
+ a = jl_rewrap_unionall((jl_value_t*)tt->super, tt->name->wrapper);
+ b = jl_rewrap_unionall((jl_value_t*)super, tt->name->wrapper);
+ if (!jl_types_equal(a, b))
+ jl_errorf("invalid redefinition of type %s", jl_symbol_name(tt->name->name));
+ JL_GC_POP();
+ }
+ else {
+ tt->super = (jl_datatype_t*)super;
+ jl_gc_wb(tt, tt->super);
+ }
 }
 
 JL_CALLABLE(jl_f__setsuper)
@@ -1263,6 +1274,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);
@@ -1271,16 +1303,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 {
@@ -1307,20 +1346,14 @@ 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);
- if (!jl_types_equal(a, b))
- goto no;
  JL_TRY {
  a = jl_apply_type(dtb->name->wrapper, jl_svec_data(dta->parameters), jl_nparams(dta));
  }
@@ -1341,21 +1374,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:

diff --git a/src/jltypes.c b/src/jltypes.c
@@ -1773,7 +1773,7 @@ void jl_init_types(void) JL_GC_DISABLED
  jl_datatype_type->abstract = 0;
  // NOTE: types are not actually mutable, but we want to ensure they are heap-allocated with stable addresses
  jl_datatype_type->mutabl = 1;
- jl_datatype_type->ninitialized = 3;
+ jl_datatype_type->ninitialized = 1;
  jl_precompute_memoized_dt(jl_datatype_type, 1);
 
  jl_typename_type->name = jl_new_typename_in(jl_symbol("TypeName"), core);

diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm
@@ -884,7 +884,9 @@
  (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)))
+ (temp (make-ssavalue))
+ (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)
@@ -895,19 +897,30 @@
  (global ,name) (const ,name)
  (scope-block
  (block
- (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)
- (= ,name (call (core _structtype) (thismodule) (inert ,name) (call (core svec) ,@params)
+ (toplevel-only struct (outerref ,name))
+ (= ,temp (call (core _structtype) (thismodule) (inert ,name) (call (core svec) ,@params)
  (call (core svec) ,@(map quotify field-names))
  ,mut ,min-initialized))
+ (if (isdefined (outerref ,name))
+ (block
+ (= ,prev (outerref ,name))
+ (if (call (core _equiv_typedef) ,prev ,temp)
+ ;; if this is compatible with an old definition, use the existing type object
+ ;; and its parameters
+ (block ,@(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) ,temp)))
+ (= (outerref ,name) ,temp))
  (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)
- (= (outerref ,name) ,name))
  (null)))
  ;; "inner" constructors
  (scope-block
@@ -3351,7 +3364,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)))

diff --git a/test/core.jl b/test/core.jl
@@ -7221,3 +7221,17 @@ 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
+end
+@test fieldtypes(M36104.T36104) == (Vector{M36104.T36104},)
+@test_throws ErrorException @eval(struct X36104; x::error(); end)
+@test @isdefined(X36104)
+struct X36104; x::Int; end
+@test fieldtypes(X36104) == (Int,)
+primitive type P36104 8 end
+@test_throws ErrorException @eval(primitive type P36104 16 end)