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

isa_tfunc not working around broken subtyping in the way I hoped it would #27078

Closed
jrevels opened this issue May 11, 2018 · 0 comments · Fixed by #27736
Closed

isa_tfunc not working around broken subtyping in the way I hoped it would #27078

jrevels opened this issue May 11, 2018 · 0 comments · Fixed by #27736

Comments

@jrevels
Copy link
Member

jrevels commented May 11, 2018

julia> bar(T::Type{S}) where {S} = isa(T, UnionAll) ? bar(T.body) : T
bar (generic function with 1 method)

julia> V = Vector{Vector{T}} where T
Array{Array{T,1},1} where T

julia> bar(V)

ERROR: type DataType has no field body
Stacktrace:
 [1] getproperty at ./sysimg.jl:15 [inlined]
 [2] bar(::Type{Array{Array{T,1},1} where T}) at ./REPL[1]:1 (repeats 2 times)
 [3] top-level scope

If I remove the Type{S} where S:

julia> bar(T) = isa(T, UnionAll) ? bar(T.body) : T
bar (generic function with 1 method)

julia> V = Vector{Vector{T}} where T
Array{Array{T,1},1} where T

julia> bar(V)
Array{Array{T,1},1}

Title might not be quite correct, but the sadness this bug causes me is very real

jrevels added a commit that referenced this issue Jun 22, 2018
jrevels added a commit that referenced this issue Jun 25, 2018
jrevels added a commit that referenced this issue Jun 25, 2018
jrevels added a commit that referenced this issue Jul 2, 2018
jrevels added a commit that referenced this issue Jul 2, 2018
jrevels added a commit that referenced this issue Jul 6, 2018
jrevels added a commit that referenced this issue Jul 6, 2018
jrevels added a commit that referenced this issue Jul 7, 2018
* small workaround checks against incorrect subtyping for kind types for isa_tfunc

* add test for #27078

* fix pretty crazy bug where Type{T}s were inferred as Ts

* work around broken subtyping in emit_isa codegen optimization

* a test that was checking for exact/optimal inference result is now broken

Justification for allowing this test to remain broken for now:
- benchmarking the expression (including downstream toy calculations on the output, e.g. broadcast sin) using BenchmarkTools reveals no actual performance difference
- inference returning an optimal result before was probably reliant on the broken subtyping behavior; correctness >>> performance
- inference is still returning a fairly tightly bounded, correct Union type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant