Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add another tuple subtyping fast path #34065

Merged
merged 1 commit into from
Dec 16, 2019
Merged

Conversation

JeffBezanson
Copy link
Member

This handles cases like Type{Tuple{}} <: Type{Tuple{Vararg{_,N} where N}}, i.e. where a fixed-length tuple type cannot ever equal an indefinite-length tuple type.

This fixes an issue reported offline; see the added strings test for example code. In 1.2 the code worked fine, but it hangs in subtyping in 1.3. The change seems to be due to a combination of new * methods for Regex plus other internal changes (ml_matches_visitor being more aggressive at avoiding method matches shadowed by previous matches).

This handles cases like `Type{Tuple{}} <: Type{Tuple{Vararg{_,N} where N}}`,
i.e. where a fixed-length tuple type cannot ever equal an indefinite-length
tuple type.
@JeffBezanson JeffBezanson added types and dispatch Types, subtyping and method dispatch backport 1.3 labels Dec 9, 2019
if (!jl_is_tuple_type(x))
return 0;
size_t n = jl_nparams(x);
return n > 0 && jl_vararg_kind(jl_tparam(x, n-1)) == JL_VARARG_UNBOUND;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return n > 0 && jl_vararg_kind(jl_tparam(x, n-1)) == JL_VARARG_UNBOUND;
return n > 0 && jl_vararg_kind(jl_tparam(x, n-1)) == JL_VARARG_UNBOUND && !var_occurs_inside(x, jl_unwrap_vararg(jl_tparam(x, n-1)) , 0, 1);

(from #32742)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is needed, since we can make this conclusion based only on length.

@vtjnash
Copy link
Member

vtjnash commented Dec 16, 2019

Could/should we use the code from

julia/src/subtype.c

Lines 1645 to 1685 in 7794574

if (istuple) {
if (npx > 0) {
jl_value_t *xva = jl_tparam(x, npx - 1);
vx = jl_vararg_kind(xva);
if (vx != JL_VARARG_NONE) {
vxt = jl_unwrap_vararg(xva);
nparams_expanded_x -= 1;
if (vx == JL_VARARG_INT)
nparams_expanded_x += jl_vararg_length(xva);
}
}
if (npy > 0) {
jl_value_t *yva = jl_tparam(y, npy - 1);
vy = jl_vararg_kind(yva);
if (vy != JL_VARARG_NONE) {
nparams_expanded_y -= 1;
if (vy == JL_VARARG_INT)
nparams_expanded_y += jl_vararg_length(yva);
}
}
// if the nparams aren't equal, or at least one of them is a typevar (uncertain), they may be obviously disjoint
if (nparams_expanded_x != nparams_expanded_y || (vx != JL_VARARG_NONE && vx != JL_VARARG_INT) || (vy != JL_VARARG_NONE && vy != JL_VARARG_INT)) {
// we have a stronger bound on x if:
if (vy == JL_VARARG_NONE || vy == JL_VARARG_INT) { // the bound on y is certain
if (vx == JL_VARARG_NONE || vx == JL_VARARG_INT || vx == JL_VARARG_UNBOUND || // and the bound on x is also certain
nparams_expanded_x > nparams_expanded_y || npx > nparams_expanded_y) { // or x is unknown, but definitely longer than y
*subtype = 0;
return 1; // number of fixed parameters in x are more than declared in y
}
}
if (nparams_expanded_x < nparams_expanded_y) {
*subtype = 0;
return 1; // number of fixed parameters in x could be fewer than in y
}
uncertain = 1;
}
}
else if (npx != npy) {
*subtype = 0;
return 1;
}
pulled into a separate function in both places?

@JeffBezanson
Copy link
Member Author

Ah, it's tricky. The code in obvious_subtype is already extremely close to handling this, and the only reason it doesn't is the free variables check here:

if (!jl_has_free_typevars(b) && jl_obvious_subtype(b, a, subtype)) {

This is a lucky case where the fast path works despite free variables, so it's hard to incorporate in this structure. obvious_subtype also updates a few local variables in there, so it's hard to refactor.

@JeffBezanson JeffBezanson merged commit aa07c1c into master Dec 16, 2019
@JeffBezanson JeffBezanson deleted the jb/varargsubtypefastpath branch December 16, 2019 22:21
@KristofferC KristofferC mentioned this pull request Dec 17, 2019
18 tasks
KristofferC pushed a commit that referenced this pull request Dec 17, 2019
This handles cases like `Type{<:Tuple{A}} <: Type{Tuple{Vararg{_,N} where N}}`,
i.e. where a fixed-length tuple type cannot ever equal an indefinite-length
tuple type.
KristofferC pushed a commit that referenced this pull request Apr 11, 2020
This handles cases like `Type{<:Tuple{A}} <: Type{Tuple{Vararg{_,N} where N}}`,
i.e. where a fixed-length tuple type cannot ever equal an indefinite-length
tuple type.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants