Skip to content

Commit

Permalink
fix isa fast path for typevars with lower bounds (#32040)
Browse files Browse the repository at this point in the history
  • Loading branch information
JeffBezanson authored May 16, 2019
1 parent 8f9ace0 commit 7a75753
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
16 changes: 12 additions & 4 deletions src/subtype.c
Original file line number Diff line number Diff line change
Expand Up @@ -1506,10 +1506,18 @@ JL_DLLEXPORT int jl_isa(jl_value_t *x, jl_value_t *t)
if (((jl_datatype_t*)t2)->name == jl_type_typename) {
jl_value_t *tp = jl_tparam0(t2);
if (jl_is_typevar(tp)) {
while (jl_is_typevar(tp))
tp = ((jl_tvar_t*)tp)->ub;
if (!jl_has_free_typevars(tp))
return jl_subtype(x, tp);
if (((jl_tvar_t*)tp)->lb == jl_bottom_type) {
while (jl_is_typevar(tp))
tp = ((jl_tvar_t*)tp)->ub;
if (!jl_has_free_typevars(tp))
return jl_subtype(x, tp);
}
else if (((jl_tvar_t*)tp)->ub == (jl_value_t*)jl_any_type) {
while (jl_is_typevar(tp))
tp = ((jl_tvar_t*)tp)->lb;
if (!jl_has_free_typevars(tp))
return jl_subtype(tp, x);
}
}
}
else {
Expand Down
4 changes: 4 additions & 0 deletions test/subtype.jl
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,10 @@ function test_Type()
@test !(Tuple{Int,} <: (@UnionAll T<:Tuple Type{T}))
@test isa(Tuple{Int}, (@UnionAll T<:Tuple Type{T}))

@test !isa(Int, Type{>:String})
@test isa(Union{Int,String}, Type{>:String})
@test isa(Any, Type{>:String})

# this matches with T==DataType, since DataType is concrete
@test issub(Tuple{Type{Int},Type{Int8}}, Tuple{T,T} where T)
@test !issub(Tuple{Type{Int},Type{Union{}}}, Tuple{T,T} where T)
Expand Down

2 comments on commit 7a75753

@vtjnash
Copy link
Member

Choose a reason for hiding this comment

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

Oddly seems to have failed on CI, although doesn't seem clear how it would be related: https://api.travis-ci.org/v3/job/533135114/log.txt

@vtjnash
Copy link
Member

Choose a reason for hiding this comment

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

In particular, that was fixed by #31934 over a week ago

Please sign in to comment.