fix isa tfunc for type kinds #22779
fix isa tfunc for type kinds #22779
Conversation
|
Add some version of the original problematic code to the test? |
4cf212d
to
cd959a2
Compare
previously, isa_tfunc(DataType, Type{...}) might return Const(false)
|
It's in there; I just reduced it to the precise MWE and unit-tested that. |
|
You're whitebox testing tfuncs though. I'd much prefer a test along the lines of the original code. |
| const n_ifunc = reinterpret(Int32, arraylen) + 1 | ||
| const t_ifunc = Array{Tuple{Int, Int, Any}, 1}(n_ifunc) | ||
| const t_ifunc_cost = Array{Int, 1}(n_ifunc) | ||
| const t_ffunc_key = Array{Any, 1}(0) |
There was a problem hiding this comment.
Are we putting any non-Functions in here?
There was a problem hiding this comment.
Nah – actually we only put Builtin subtypes in here. This should just be more likely to trigger our anti-specialization heuristics and avoid trying to put convert and typeassert into the code path.
I think you mean unit-testing, and that you would like to also see an integration test? Adding a test for the original failure would also be a whitebox test. However, I'm not sure that the integration test is necessary in this case. We already have a couple tests in this file to show that integration works, so I think we were just missing some of the cases for unit testing. |
|
@vtjnash I suspect this might not backport cleanly, and you might need to rebase it for 0.6 (and almost certainly for 0.5) manually. |
|
@vtjnash I'm not sure how to backport this properly to 0.6. Could you submit this change as a PR to the release-0.6 branch? |
previously, isa_tfunc(DataType, Type{...}) might return Const(false)