Skip to content

Commit

Permalink
normalize unions made in subtyping (#49276)
Browse files Browse the repository at this point in the history
We observed a case where simple_tmeet made a Union of egal things, which
is undesirable. There also was no sorting of the result, as it normally
done, and theoretically, simplification with an omit_bad_union to remove
`S` could similar result in a Union that should be further simplified to
remove redundancies.
```
Union{Union{Val{T}, S} where T<:AbstractString, Union{Val{T}, Int64} where T<:AbstractString} where S
```
(In principle, that simplification might also be possible to do during
the original jl_type_union call when flattening it: see #49279)
  • Loading branch information
vtjnash authored Apr 7, 2023
1 parent 0c24073 commit 02704d9
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 8 deletions.
46 changes: 46 additions & 0 deletions src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,52 @@ JL_DLLEXPORT jl_value_t *jl_type_union(jl_value_t **ts, size_t n)
return tu;
}

jl_value_t *simple_union(jl_value_t *a, jl_value_t *b)
{
size_t nt = count_union_components(&a, 1);
nt += count_union_components(&b, 1);
jl_value_t **temp;
JL_GC_PUSHARGS(temp, nt+1);
size_t count = 0;
flatten_type_union(&a, 1, temp, &count);
flatten_type_union(&b, 1, temp, &count);
assert(count == nt);
size_t i, j;
for (i = 0; i < nt; i++) {
int has_free = temp[i] != NULL && jl_has_free_typevars(temp[i]);
for (j = 0; j < nt; j++) {
if (j != i && temp[i] && temp[j]) {
if (temp[i] == jl_bottom_type ||
temp[j] == (jl_value_t*)jl_any_type ||
jl_egal(temp[i], temp[j]) ||
(!has_free && !jl_has_free_typevars(temp[j]) &&
// issue #24521: don't merge Type{T} where typeof(T) varies
!(jl_is_type_type(temp[i]) && jl_is_type_type(temp[j]) && jl_typeof(jl_tparam0(temp[i])) != jl_typeof(jl_tparam0(temp[j]))) &&
jl_subtype(temp[i], temp[j]))) {
temp[i] = NULL;
}
}
}
}
isort_union(temp, nt);
jl_value_t **ptu = &temp[nt];
*ptu = jl_bottom_type;
int k;
for (k = (int)nt-1; k >= 0; --k) {
if (temp[k] != NULL) {
if (*ptu == jl_bottom_type)
*ptu = temp[k];
else
*ptu = jl_new_struct(jl_uniontype_type, temp[k], *ptu);
}
}
assert(*ptu != NULL);
jl_value_t *tu = *ptu;
JL_GC_POP();
return tu;
}


// unionall types -------------------------------------------------------------

JL_DLLEXPORT jl_value_t *jl_type_unionall(jl_tvar_t *v, jl_value_t *body)
Expand Down
11 changes: 3 additions & 8 deletions src/subtype.c
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,7 @@ static int is_any_like(jl_value_t* x, jl_typeenv_t *env) JL_NOTSAFEPOINT
return 0;
}

jl_value_t *simple_union(jl_value_t *a, jl_value_t *b);
// compute a least upper bound of `a` and `b`
static jl_value_t *simple_join(jl_value_t *a, jl_value_t *b)
{
Expand All @@ -589,13 +590,7 @@ static jl_value_t *simple_join(jl_value_t *a, jl_value_t *b)
return a;
if (jl_is_typevar(b) && obviously_egal(a, ((jl_tvar_t*)b)->lb))
return b;
if (!jl_has_free_typevars(a) && !jl_has_free_typevars(b) &&
// issue #24521: don't merge Type{T} where typeof(T) varies
!(jl_is_type_type(a) && jl_is_type_type(b) && jl_typeof(jl_tparam0(a)) != jl_typeof(jl_tparam0(b)))) {
if (jl_subtype(a, b)) return b;
if (jl_subtype(b, a)) return a;
}
return jl_new_struct(jl_uniontype_type, a, b);
return simple_union(a, b);
}

// Compute a greatest lower bound of `a` and `b`
Expand Down Expand Up @@ -2759,7 +2754,7 @@ static jl_value_t *omit_bad_union(jl_value_t *u, jl_tvar_t *t)
b = omit_bad_union(b, t);
res = a == NULL ? b :
b == NULL ? a :
jl_new_struct(jl_uniontype_type, a, b);
simple_join(a, b);
JL_GC_POP();
}
return res;
Expand Down

0 comments on commit 02704d9

Please sign in to comment.