From 725c385a698a21df7ce714863648447674c1cd27 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Mon, 15 May 2023 11:47:46 -0400 Subject: [PATCH] jltypes: always run parameter normalization This simplifies the types, which may help subtyping other other similar lookup code any time this is later used as a parameter, so it is probably worthwhile to do. This is a followup to #49820, where we reorganized the code to make this more straightforward. --- src/builtins.c | 6 ++-- src/gf.c | 4 +-- src/jltypes.c | 65 ++++++++++++++++++++++++-------------------- src/julia_internal.h | 2 +- src/subtype.c | 6 ++-- 5 files changed, 45 insertions(+), 38 deletions(-) diff --git a/src/builtins.c b/src/builtins.c index a6c904c851c95..b664b8d73710f 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -1363,11 +1363,11 @@ JL_CALLABLE(jl_f_apply_type) jl_vararg_t *vm = (jl_vararg_t*)args[0]; if (!vm->T) { JL_NARGS(apply_type, 2, 3); - return (jl_value_t*)jl_wrap_vararg(args[1], nargs == 3 ? args[2] : NULL); + return (jl_value_t*)jl_wrap_vararg(args[1], nargs == 3 ? args[2] : NULL, 1); } else if (!vm->N) { JL_NARGS(apply_type, 2, 2); - return (jl_value_t*)jl_wrap_vararg(vm->T, args[1]); + return (jl_value_t*)jl_wrap_vararg(vm->T, args[1], 1); } } else if (jl_is_unionall(args[0])) { @@ -2060,7 +2060,7 @@ void jl_init_primitives(void) JL_GC_DISABLED add_builtin("Tuple", (jl_value_t*)jl_anytuple_type); add_builtin("TypeofVararg", (jl_value_t*)jl_vararg_type); add_builtin("SimpleVector", (jl_value_t*)jl_simplevector_type); - add_builtin("Vararg", (jl_value_t*)jl_wrap_vararg(NULL, NULL)); + add_builtin("Vararg", (jl_value_t*)jl_wrap_vararg(NULL, NULL, 0)); add_builtin("Module", (jl_value_t*)jl_module_type); add_builtin("MethodTable", (jl_value_t*)jl_methtable_type); diff --git a/src/gf.c b/src/gf.c index 8bfbad4b0f7ca..b49fc32bf4e0b 100644 --- a/src/gf.c +++ b/src/gf.c @@ -735,7 +735,7 @@ static jl_value_t *inst_varargp_in_env(jl_value_t *decl, jl_svec_t *sparams) vm = T_has_tv ? jl_type_unionall(v, T) : T; if (N_has_tv) N = NULL; - vm = (jl_value_t*)jl_wrap_vararg(vm, N); // this cannot throw for these inputs + vm = (jl_value_t*)jl_wrap_vararg(vm, N, 1); // this cannot throw for these inputs } sp++; decl = ((jl_unionall_t*)decl)->body; @@ -984,7 +984,7 @@ static void jl_compilation_sig( // avoid Vararg{Type{Type{...}}} if (jl_is_type_type(type_i) && jl_is_type_type(jl_tparam0(type_i))) type_i = (jl_value_t*)jl_type_type; - type_i = (jl_value_t*)jl_wrap_vararg(type_i, (jl_value_t*)NULL); // this cannot throw for these inputs + type_i = (jl_value_t*)jl_wrap_vararg(type_i, (jl_value_t*)NULL, 1); // this cannot throw for these inputs } else { type_i = inst_varargp_in_env(decl, sparams); diff --git a/src/jltypes.c b/src/jltypes.c index 5fc98194775b5..59889c9b5a740 100644 --- a/src/jltypes.c +++ b/src/jltypes.c @@ -847,14 +847,14 @@ JL_DLLEXPORT jl_value_t *jl_type_unionall(jl_tvar_t *v, jl_value_t *body) if (T_has_tv) { jl_value_t *wrapped = jl_type_unionall(v, vm->T); JL_GC_PUSH1(&wrapped); - wrapped = (jl_value_t*)jl_wrap_vararg(wrapped, vm->N); + wrapped = (jl_value_t*)jl_wrap_vararg(wrapped, vm->N, 1); JL_GC_POP(); return wrapped; } else { assert(N_has_tv); assert(vm->N == (jl_value_t*)v); - return (jl_value_t*)jl_wrap_vararg(vm->T, NULL); + return (jl_value_t*)jl_wrap_vararg(vm->T, NULL, 1); } } if (!jl_is_type(body) && !jl_is_typevar(body)) @@ -1889,7 +1889,7 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value } // if some normalization might be needed, do that now // it is probably okay to mutate iparams, and we only store globally rooted objects here - if (check && cacheable) { + if (check) { size_t i; for (i = 0; i < ntp; i++) { jl_value_t *pi = iparams[i]; @@ -1898,8 +1898,7 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value if (jl_is_datatype(pi)) continue; if (jl_is_vararg(pi)) - // This would require some special handling, but is not needed - // at the moment (and might be better handled in jl_wrap_vararg instead). + // This is already handled in jl_wrap_vararg instead continue; if (!cacheable && jl_has_free_typevars(pi)) continue; @@ -2327,7 +2326,7 @@ static jl_value_t *inst_type_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_t N = inst_type_w_(v->N, env, stack, check); } if (T != v->T || N != v->N) { - t = (jl_value_t*)jl_wrap_vararg(T, N); + t = (jl_value_t*)jl_wrap_vararg(T, N, check); } JL_GC_POP(); return t; @@ -2400,36 +2399,44 @@ jl_datatype_t *jl_wrap_Type(jl_value_t *t) return (jl_datatype_t*)jl_instantiate_unionall(jl_type_type, t); } -jl_vararg_t *jl_wrap_vararg(jl_value_t *t, jl_value_t *n) +jl_vararg_t *jl_wrap_vararg(jl_value_t *t, jl_value_t *n, int check) { - if (n) { - if (jl_is_typevar(n) || jl_is_uniontype(jl_unwrap_unionall(n))) { - // TODO: this is disabled due to #39698; it is also inconsistent - // with other similar checks, where we usually only check substituted - // values and not the bounds of variables. - /* - jl_tvar_t *N = (jl_tvar_t*)n; - if (!(N->lb == jl_bottom_type && N->ub == (jl_value_t*)jl_any_type)) - jl_error("TypeVar in Vararg length must have bounds Union{} and Any"); - */ - } - else if (!jl_is_long(n)) { - jl_type_error_rt("Vararg", "count", (jl_value_t*)jl_long_type, n); - } - else if (jl_unbox_long(n) < 0) { - jl_errorf("Vararg length is negative: %zd", jl_unbox_long(n)); + jl_task_t *ct = jl_current_task; + JL_GC_PUSH1(&t); + if (check) { + if (n) { + if (jl_is_typevar(n) || jl_is_uniontype(jl_unwrap_unionall(n))) { + // TODO: this is disabled due to #39698; it is also inconsistent + // with other similar checks, where we usually only check substituted + // values and not the bounds of variables. + /* + jl_tvar_t *N = (jl_tvar_t*)n; + if (!(N->lb == jl_bottom_type && N->ub == (jl_value_t*)jl_any_type)) + jl_error("TypeVar in Vararg length must have bounds Union{} and Any"); + */ + } + else if (!jl_is_long(n)) { + jl_type_error_rt("Vararg", "count", (jl_value_t*)jl_long_type, n); + } + else if (jl_unbox_long(n) < 0) { + jl_errorf("Vararg length is negative: %zd", jl_unbox_long(n)); + } } - } - if (t) { - if (!jl_valid_type_param(t)) { - jl_type_error_rt("Vararg", "type", (jl_value_t*)jl_type_type, t); + if (t) { + if (!jl_valid_type_param(t)) { + jl_type_error_rt("Vararg", "type", (jl_value_t*)jl_type_type, t); + } + t = normalize_unionalls(t); + jl_value_t *tw = extract_wrapper(t); + if (tw && t != tw && jl_types_equal(t, tw)) + t = tw; } } - jl_task_t *ct = jl_current_task; jl_vararg_t *vm = (jl_vararg_t *)jl_gc_alloc(ct->ptls, sizeof(jl_vararg_t), jl_vararg_type); jl_set_typetagof(vm, jl_vararg_tag, 0); vm->T = t; vm->N = n; + JL_GC_POP(); return vm; } @@ -2712,7 +2719,7 @@ void jl_init_types(void) JL_GC_DISABLED // It seems like we probably usually end up needing the box for kinds (often used in an Any context), so force it to exist jl_vararg_type->name->mayinlinealloc = 0; - jl_svec_t *anytuple_params = jl_svec(1, jl_wrap_vararg((jl_value_t*)jl_any_type, (jl_value_t*)NULL)); + jl_svec_t *anytuple_params = jl_svec(1, jl_wrap_vararg((jl_value_t*)jl_any_type, (jl_value_t*)NULL, 0)); jl_anytuple_type = jl_new_datatype(jl_symbol("Tuple"), core, jl_any_type, anytuple_params, jl_emptysvec, anytuple_params, jl_emptysvec, 0, 0, 0); jl_tuple_typename = jl_anytuple_type->name; diff --git a/src/julia_internal.h b/src/julia_internal.h index 49f0b19ec4209..a221d856e5dac 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -712,7 +712,7 @@ jl_datatype_t *jl_new_abstracttype(jl_value_t *name, jl_module_t *module, jl_datatype_t *jl_new_uninitialized_datatype(void); void jl_precompute_memoized_dt(jl_datatype_t *dt, int cacheable); JL_DLLEXPORT jl_datatype_t *jl_wrap_Type(jl_value_t *t); // x -> Type{x} -jl_vararg_t *jl_wrap_vararg(jl_value_t *t, jl_value_t *n); +jl_vararg_t *jl_wrap_vararg(jl_value_t *t, jl_value_t *n, int check); void jl_reinstantiate_inner_types(jl_datatype_t *t); jl_datatype_t *jl_lookup_cache_type_(jl_datatype_t *type); void jl_cache_type_(jl_datatype_t *type); diff --git a/src/subtype.c b/src/subtype.c index fd9bd3e8be00f..4013666e4b32a 100644 --- a/src/subtype.c +++ b/src/subtype.c @@ -953,7 +953,7 @@ static int subtype_unionall(jl_value_t *t, jl_unionall_t *u, jl_stenv_t *e, int8 if (R && ans && e->envidx < e->envsz) { jl_value_t *val; if (vb.intvalued && vb.lb == (jl_value_t*)jl_any_type) - val = (jl_value_t*)jl_wrap_vararg(NULL, NULL); // special token result that represents N::Int in the envout + val = (jl_value_t*)jl_wrap_vararg(NULL, NULL, 0); // special token result that represents N::Int in the envout else if (!vb.occurs_inv && vb.lb != jl_bottom_type) val = is_leaf_bound(vb.lb) ? vb.lb : (jl_value_t*)jl_new_typevar(u->var->name, jl_bottom_type, vb.lb); else if (vb.lb == vb.ub) @@ -3089,7 +3089,7 @@ static jl_value_t *intersect_varargs(jl_vararg_t *vmx, jl_vararg_t *vmy, ssize_t ii = (jl_value_t*)vmy; else { JL_GC_PUSH1(&ii); - ii = (jl_value_t*)jl_wrap_vararg(ii, NULL); + ii = (jl_value_t*)jl_wrap_vararg(ii, NULL, 1); JL_GC_POP(); } return ii; @@ -3130,7 +3130,7 @@ static jl_value_t *intersect_varargs(jl_vararg_t *vmx, jl_vararg_t *vmy, ssize_t else if (yp2 && obviously_egal(yp1, ii) && obviously_egal(yp2, i2)) ii = (jl_value_t*)vmy; else - ii = (jl_value_t*)jl_wrap_vararg(ii, i2); + ii = (jl_value_t*)jl_wrap_vararg(ii, i2, 1); JL_GC_POP(); return ii; }