Skip to content

Commit

Permalink
Use at-complete macro in __foldl__
Browse files Browse the repository at this point in the history
Ideally, `complete` should be called outside `__foldl__`.  However,
doing so with current Julia (1.0--1.3) invokes type instability in
some cases.  For forward compatibility, let's use the macro version
inside `__foldl__` so that it can be safely removed without breaking
downstream projects.
  • Loading branch information
tkf committed Jul 10, 2019
1 parent d2cdc39 commit 6efb853
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 23 deletions.
1 change: 1 addition & 0 deletions docs/src/interface.md
Expand Up @@ -12,6 +12,7 @@ Transducers.start
Transducers.next
Transducers.@next
Transducers.complete
Transducers.@complete
Transducers.outtype
```

Expand Down
8 changes: 4 additions & 4 deletions examples/reducibles.jl
Expand Up @@ -8,11 +8,11 @@ struct VecOfVec{T}
vectors::Vector{Vector{T}}
end

# We need [`@next`](@ref Transducers.@next) and [`complete`](@ref
# Transducers.complete) to invoke the reducing function `rf`.
# We need [`@next`](@ref Transducers.@next) and [`@complete`](@ref
# Transducers.@complete) to invoke the reducing function `rf`.

using Transducers
using Transducers: @next, complete
using Transducers: @next, @complete

# Supporting [`mapfoldl`](@ref) and similar only requires
# [`Transducers.__foldl__`](@ref):
Expand All @@ -23,7 +23,7 @@ function Transducers.__foldl__(rf, val, vov::VecOfVec)
val = @next(rf, val, x)
end
end
return complete(rf, val)
return @complete(rf, val)
end

# Note that it's often a good idea to implement `Base.eltype`:
Expand Down
22 changes: 22 additions & 0 deletions src/core.jl
Expand Up @@ -185,6 +185,20 @@ macro next(rf, state, input)
end
end

"""
@complete(rf, result)
Currently, it is equivalent to [`complete(rf, result)`](@ref
complete). However, [`__foldl__`](@ref) implementers must use
`@complete` in ordered to be forward-compatible with future
Transducers.jl.
"""
macro complete(rf, result)
quote
complete($(esc(rf)), $(esc(result)))
end
end

struct NoType end
const NOTYPE = NoType()
const Typeish{T} = Union{Type{T}, NoType}
Expand Down Expand Up @@ -426,6 +440,14 @@ If `start(rf::R_{X}, state)` is defined, `complete` **must** unwarp
In Transducers.jl 0.2, `complete` had a fallback implementation
to automatically call `unwrap` when `wrap` is called in `start`.
Relying on this fallback implementation is now deprecated.
!!! warning
For forward compatibility, the macro [`@complete`](@ref) must be
used inside [`__foldl__`](@ref). Inside `next` and `complete`,
function `complete` must be used. It means that using macro and
non-macro forms (e.g., `@next` and `complete`) in a same function
is likely to be wrong.
"""
complete(f, result) = f(result)
complete(rf::AbstractReduction, result) =
Expand Down
8 changes: 4 additions & 4 deletions src/interop/lazyarrays.jl
@@ -1,9 +1,9 @@
@inline function _foldl_lazy_cat_vectors(rf, acc, vectors)
isempty(vectors) && return complete(rf, acc)
isempty(vectors) && return @complete(rf, acc)
result = @return_if_reduced foldlargs(acc, vectors...) do acc, arr
foldl_nocomplete(rf, acc, arr)
end
return complete(rf, result)
return @complete(rf, result)
end

"""
Expand All @@ -17,14 +17,14 @@ end
_foldl_lazy_vcat(rf, acc, coll::LazyArrays.Vcat)
"""
@inline function _foldl_lazy_vcat(rf, acc, coll)
isempty(coll.arrays) && return complete(rf, acc)
isempty(coll.arrays) && return @complete(rf, acc)
coll isa AbstractVector && return _foldl_lazy_cat_vectors(rf, acc, coll.arrays)
coll :: AbstractMatrix
for j in axes(coll, 2)
vectors = view.(coll.arrays, Ref(:), j)
acc = @return_if_reduced _foldl_lazy_cat_vectors(rf, acc, vectors)
end
return complete(rf, acc)
return @complete(rf, acc)
end
# Vcat currently always is an `AbstractVector` or `AbstractMatrix`

Expand Down
28 changes: 14 additions & 14 deletions src/processes.jl
Expand Up @@ -109,7 +109,7 @@ function __foldl__(rf, val, itr::MyType)
for x in itr
val = @next(rf, val, x)
end
return complete(rf, val)
return @complete(rf, val)
end
```
Expand All @@ -118,7 +118,7 @@ thus there is no need for defining it. In general, defining
`__foldl__` is useful only when there is a better way to go over items
in `reducible` than `Base.iterate`.
See also: [`@next`](@ref).
See also: [`@next`](@ref), [`@complete`](@ref).
"""
__foldl__

Expand All @@ -129,7 +129,7 @@ _dec(::Val{n}) where n = Val(n - 1)

function __foldl__(rf, init, coll)
ret = iterate(coll)
ret === nothing && return complete(rf, init)
ret === nothing && return @complete(rf, init)
x, state = ret
val = @next(rf, init, x)
return _foldl_iter(rf, val, coll, state, FOLDL_RECURSION_LIMIT)
Expand All @@ -143,19 +143,19 @@ end
return _foldl_iter(rf, y, iter, state, _dec(counter))
val = y
end
return complete(rf, val)
return @complete(rf, val)
end

# TODO: use IndexStyle
@inline function __foldl__(rf, init, arr::Union{AbstractArray, Broadcasted})
isempty(arr) && return complete(rf, init)
isempty(arr) && return @complete(rf, init)
idxs = eachindex(arr)
val = @next(rf, init, @inbounds arr[idxs[firstindex(idxs)]])
@simd_if rf for k in firstindex(idxs) + 1:lastindex(idxs)
i = @inbounds idxs[k]
val = @next(rf, val, @inbounds arr[i])
end
return complete(rf, val)
return @complete(rf, val)
end

@inline _getvalues(i) = ()
Expand All @@ -166,13 +166,13 @@ end
@inline function __foldl__(
rf, init,
zs::Iterators.Zip{<:Tuple{Vararg{AbstractArray}}})
isempty(zs) && return complete(rf, init)
isempty(zs) && return @complete(rf, init)
idxs = eachindex(zs.is...)
val = @next(rf, init, _getvalues(firstindex(idxs), zs.is...))
@simd_if rf for i in firstindex(idxs) + 1:lastindex(idxs)
val = @next(rf, val, _getvalues(i, zs.is...))
end
return complete(rf, val)
return @complete(rf, val)
end
end

Expand All @@ -181,7 +181,7 @@ end
prod::Iterators.ProductIterator{<:Tuple{Any,Any,Vararg{Any}}})
val = _foldl_product(rf, init, (), prod.iterators...)
val isa Reduced && return val
return complete(rf, val)
return @complete(rf, val)
end

@noinline _foldl_product(rf, val, ::Any) = error("Unreachable")
Expand Down Expand Up @@ -210,7 +210,7 @@ function __simple_foldl__(rf, val, itr)
for x in itr
val = @next(rf, val, x)
end
return complete(rf, val)
return @complete(rf, val)
end

"""
Expand Down Expand Up @@ -404,7 +404,7 @@ function __reduce__(rf, init, arr::AbstractArray;
c = foldl(results) do a, b
combine(rf, a, b)
end
return complete(rf, c)
return @complete(rf, c)
end
end

Expand Down Expand Up @@ -1000,7 +1000,7 @@ wrapper type.
# Examples
```jldoctest
julia> using Transducers
using Transducers: @next, complete
using Transducers: @next, @complete
using ArgCheck
julia> function uppertriangle(A::AbstractMatrix)
Expand All @@ -1009,7 +1009,7 @@ julia> function uppertriangle(A::AbstractMatrix)
for j in 1:size(A, 2), i in 1:min(j, size(A, 1))
acc = @next(rf, acc, @inbounds A[i, j])
end
return complete(rf, acc)
return @complete(rf, acc)
end
end;
Expand All @@ -1036,7 +1036,7 @@ julia> expressions(str::AbstractString; kwargs...) =
expr === nothing && break
val = @next(rf, val, expr)
end
return complete(rf, val)
return @complete(rf, val)
end;
julia> collect(Map(identity), expressions(\"\"\"
Expand Down
2 changes: 1 addition & 1 deletion src/show.jl
Expand Up @@ -13,7 +13,7 @@ function __foldl__(rf, val, xff::TransducerFolder)
xf = xf.inner
end
val = @next(rf, val, xf)
return complete(rf, val)
return @complete(rf, val)
end

function print_arrow(io, pre, post)
Expand Down

0 comments on commit 6efb853

Please sign in to comment.