Add recursive findfirst method for tuples.#42423
Conversation
|
Note #42263 |
aviatesk
left a comment
There was a problem hiding this comment.
You can use e.g.:
@test Base.return_types() do
findfirst(==(2), (1,2,3))
end == Any[Int]as a test case that is supposed to be improved by this PR.
Also, how about adding findlast also ? It might be enough to add:
function findlast(f::Function, x::Tuple)
r = findfirst(f, reverse(x))
return isnothing(r) ? r : length(x) - r + 1
end
base/tuple.jl
Outdated
| function findfirst(f::Function, x::Any32) | ||
| for i in 1:length(x) | ||
| f(x[i]) && return i | ||
| end | ||
| end |
There was a problem hiding this comment.
Let's use 4-spaces indents for Base:
| function findfirst(f::Function, x::Any32) | |
| for i in 1:length(x) | |
| f(x[i]) && return i | |
| end | |
| end | |
| function findfirst(f::Function, x::Any32) | |
| for i in 1:length(x) | |
| f(x[i]) && return i | |
| end | |
| end |
There was a problem hiding this comment.
Done, switched to 4 spaces and renamed to _findfirst_loop following #42263.
Also added the test case.
Co-authored-by: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com>
Co-authored-by: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com>
Co-authored-by: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com>
|
Let's see a benchmark result: @nanosoldier |
|
Something went wrong when running your job: Unfortunately, the logs could not be uploaded. |
|
@vtjnash could you please upload the result ? |
|
yep, looks unchanged to me |
|
Thanks. And yeah, the result looks reasonable and I think this is good to go. |
| end | ||
| return nothing | ||
| end | ||
| findfirst(f::Function, t::Tuple) = length(t) < 32 ? _findfirst_rec(f, 1, t) : _findfirst_loop(f, t) |
There was a problem hiding this comment.
I guess this should be <= if we want to include the 32 case?
There was a problem hiding this comment.
I went with < because Jeff did here: https://github.com/JuliaLang/julia/pull/42263/files
|
All CI failures are known issues. |
These recursive definitions will allow constant-folding for small tuple cases.
These recursive definitions will allow constant-folding for small tuple cases.
https://discourse.julialang.org/t/why-does-findfirst-t-on-a-tuple-of-typed-only-constant-fold-for-the-first/68893
With this PR: