fix #36104, assign global name during type definitions (#36121)

also fixes #21816

(cherry picked from commit 095e92d)

base/compiler/tfuncs.jl

@@ -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

src/builtins.c

@@ -1262,6 +1262,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);
@@ -1270,16 +1291,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 {
@@ -1306,15 +1334,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);
@@ -1340,21 +1366,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:

src/jltypes.c

@@ -1631,6 +1631,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);

src/julia-syntax.scm

@@ -878,7 +878,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)
@@ -892,16 +893,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
@@ -3345,7 +3359,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)))

test/core.jl

@@ -7216,3 +7216,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)