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

Revert "make last work on any reversible collection (#42991), and fix it #43106

Merged
merged 2 commits into from
Nov 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ Language changes
* `@time` and `@timev` now take an optional description to allow annotating the source of time reports.
i.e. `@time "Evaluating foo" foo()` ([#42431])
* New `@showtime` macro to show both the line being evaluated and the `@time` report ([#42431])
* `last(collection)` will now work on any collection that supports `Iterators.reverse` and `first`, rather than being
restricted to indexable collections.
* Unbalanced Unicode bidirectional formatting directives are now disallowed within strings and comments,
to mitigate the ["trojan source"](https://www.trojansource.codes) vulnerability ([#42918]).

Expand Down
11 changes: 3 additions & 8 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -462,15 +462,11 @@ end
last(coll)

Get the last element of an ordered collection, if it can be computed in O(1) time. This is
accomplished by calling [`Iterators.reverse`](@ref) and then [`first`](@ref) on that reversed iterator.
Return the end point of an [`AbstractRange`](@ref) even if it is empty.
accomplished by calling [`lastindex`](@ref) to get the last index. Return the end
point of an [`AbstractRange`](@ref) even if it is empty.

See also [`first`](@ref), [`endswith`](@ref).

!!! compat "Julia 1.8"
For versions of julia older than 1.8, `last(x)` will only work on collections that support indexing and
[`lastindex`](@ref).

# Examples
```jldoctest
julia> last(1:2:10)
Expand All @@ -480,8 +476,7 @@ julia> last([1; 2; 3; 4])
4
```
"""
last(a) = first(Iterators.reverse(a))
last(a::AbstractVector) = a[end]
last(a) = a[end]

"""
last(itr, n::Integer)
Expand Down
1 change: 1 addition & 0 deletions base/generator.jl
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ size(g::Generator) = size(g.iter)
axes(g::Generator) = axes(g.iter)
ndims(g::Generator) = ndims(g.iter)
keys(g::Generator) = keys(g.iter)
last(g::Generator) = g.f(last(g.iter))


## iterator traits
Expand Down
14 changes: 13 additions & 1 deletion base/iterators.jl
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ size(r::Reverse) = size(r.itr)
IteratorSize(::Type{Reverse{T}}) where {T} = IteratorSize(T)
IteratorEltype(::Type{Reverse{T}}) where {T} = IteratorEltype(T)
last(r::Reverse) = first(r.itr) # the first shall be last
first(r::Reverse) = last(r.itr) # and the last shall be first

# reverse-order array iterators: assumes more-specialized Reverse for eachindex
@propagate_inbounds function iterate(A::Reverse{<:AbstractArray}, state=(reverse(eachindex(A.itr)),))
Expand Down Expand Up @@ -159,6 +160,7 @@ size(e::Enumerate) = size(e.itr)
n === nothing && return n
(i, n[1]), (i+1, n[2])
end
last(e::Enumerate) = (length(e.itr), e.itr[end])

eltype(::Type{Enumerate{I}}) where {I} = Tuple{Int, eltype(I)}

Expand Down Expand Up @@ -250,6 +252,10 @@ IteratorSize(::Type{<:Pairs{<:Any, <:Any, I}}) where {I} = IteratorSize(I)
IteratorSize(::Type{<:Pairs{<:Any, <:Any, <:Base.AbstractUnitRange, <:Tuple}}) = HasLength()

reverse(v::Pairs) = Pairs(getfield(v, :data), reverse(getfield(v, :itr)))
function last(v::Pairs{K, V}) where {K, V}
idx = last(getfield(v, :itr))
return Pair{K, V}(idx, v[idx])
end

haskey(v::Pairs, key) = (key in getfield(v, :itr))
keys(v::Pairs) = getfield(v, :itr)
Expand Down Expand Up @@ -397,7 +403,8 @@ zip_iteratoreltype() = HasEltype()
zip_iteratoreltype(a) = a
zip_iteratoreltype(a, tail...) = and_iteratoreltype(a, zip_iteratoreltype(tail...))

reverse(z::Zip) = Zip(Base.map(reverse, z.is))
reverse(z::Zip) = Zip(Base.map(reverse, z.is)) # n.b. we assume all iterators are the same length
last(z::Zip) = getindex.(z.is, minimum(Base.map(lastindex, z.is)))

# filter

Expand Down Expand Up @@ -456,6 +463,7 @@ IteratorEltype(::Type{Filter{F,I}}) where {F,I} = IteratorEltype(I)
IteratorSize(::Type{<:Filter}) = SizeUnknown()

reverse(f::Filter) = Filter(f.flt, reverse(f.itr))
last(f::Filter) = first(reverse(f))

# Accumulate -- partial reductions of a function over an iterator

Expand Down Expand Up @@ -882,6 +890,7 @@ function iterate(it::Cycle, state)
end

reverse(it::Cycle) = Cycle(reverse(it.xs))
last(it::Cycle) = last(it.xs)
Copy link
Member

Choose a reason for hiding this comment

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

This (and the method on the previous line!) doesn't seem right. The sequence doesn't converge.

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 don't feel like I can delete them in this PR though


# Repeated - repeat an object infinitely many times

Expand Down Expand Up @@ -920,6 +929,7 @@ IteratorSize(::Type{<:Repeated}) = IsInfinite()
IteratorEltype(::Type{<:Repeated}) = HasEltype()

reverse(it::Union{Repeated,Take{<:Repeated}}) = it
last(it::Union{Repeated,Take{<:Repeated}}) = first(it)

# Product -- cartesian product of iterators
struct ProductIterator{T<:Tuple}
Expand Down Expand Up @@ -1051,6 +1061,7 @@ end
end

reverse(p::ProductIterator) = ProductIterator(Base.map(reverse, p.iterators))
last(p::ProductIterator) = Base.map(last, p.iterators)

# flatten an iterator of iterators

Expand Down Expand Up @@ -1130,6 +1141,7 @@ length(f::Flatten{Tuple{}}) = 0
end

reverse(f::Flatten) = Flatten(reverse(itr) for itr in reverse(f.it))
last(f::Flatten) = last(last(f.it))

"""
partition(collection, n)
Expand Down
9 changes: 8 additions & 1 deletion test/iterators.jl
Original file line number Diff line number Diff line change
Expand Up @@ -689,11 +689,18 @@ end
Iterators.Filter(isodd, 1:10), flatten((1:10, 50:60)), enumerate("foo"),
pairs(50:60), zip(1:10,21:30,51:60), product(1:3, 10:12), repeated(3.14159, 5),
(a=2, b=3, c=5, d=7, e=11))
@test squash(collect(Iterators.reverse(itr))) == reverse(squash(collect(itr)))
arr = reverse(squash(collect(itr)))
itr = Iterators.reverse(itr)
@test squash(collect(itr)) == arr
if !isempty(arr)
@test first(itr) == first(arr)
@test last(itr) == last(arr)
end
end
@test collect(take(Iterators.reverse(cycle(1:3)), 7)) == collect(take(cycle(3:-1:1), 7))
let r = repeated(3.14159)
@test Iterators.reverse(r) === r
@test last(r) === 3.14159
end
for t in [(1,), (2, 3, 5, 7, 11), (a=1,), (a=2, b=3, c=5, d=7, e=11)]
@test Iterators.reverse(Iterators.reverse(t)) === t
Expand Down