Skip to content

Commit

Permalink
fail early for FixedSizeArray subtypes we can't handle (#99)
Browse files Browse the repository at this point in the history
Before this PR `collect_as` had to use a type assert on the return
value to make sure the return value is of the type requested by the
user. This is necessary because of UnionAll types with non-default
constraints on their type parameters, such as
`FixedSizeVector{T} where {T <: Number}`, because accessing the
specific constraint is not currently possible in Julia in a supported
way.

This change removes the type assert, instead throwing early, as soon as
it is recognized that the requested return type is not among the
allowed types, which are known-good, in the sense that the return value
we produce ends up being of the requested type as expected.

Benefits:
* a type assert of nonconcrete type is not required any more (those can
  be expensive)
* the throw happens earlier than before

Future PRs will possibly apply the same mechanic to some constructors,
in addition to `collect_as`.
  • Loading branch information
nsajko authored Feb 11, 2025
1 parent b8c23e8 commit 30e318f
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 3 deletions.
37 changes: 37 additions & 0 deletions src/FixedSizeArray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,43 @@ function Base.propertynames(
()
end

const FixedSizeArrayAllowedConstructorType = let
function f(Mem::UnionAll)
Union{
# 0 fixed parameters
Type{FixedSizeArray{T, N, Mem{T}} where {T, N}},
# 1 fixed parameter
Type{FixedSizeArray{T, N, Mem{T}} where {N}} where {T},
Type{FixedSizeArray{T, N, Mem{T}} where {T}} where {N},
# 2 fixed parameters
Type{FixedSizeArray{T, N, Mem{T}}} where {T, N},
}
end
special_storage_types = (Vector, optional_memory...)
Union{
# 0 fixed parameters
Type{FixedSizeArray},
# 1 fixed parameter
Type{FixedSizeArray{T}} where {T},
Type{FixedSizeArray{T, N} where {T}} where {N},
# 2 fixed parameters
Type{FixedSizeArray{T, N}} where {T, N},
Type{FixedSizeArray{T, N, Mem} where {N}} where {T, Mem <: DenseVector{T}},
# 3 fixed parameters
Type{FixedSizeArray{T, N, Mem}} where {T, N, Mem <: DenseVector{T}},
# special cases depending on the underlying storage type
map(f, special_storage_types)...,
}
end

function check_constructor_is_allowed(::Type{T}) where {T <: FixedSizeArray}
if Type{T} <: FixedSizeArrayAllowedConstructorType
T
else
throw(ArgumentError("technical limitation: type is not among the allowed `FixedSizeArray` constructors (try deleting type parameter constraints)"))
end
end

const FixedSizeVector{T} = FixedSizeArray{T,1}
const FixedSizeMatrix{T} = FixedSizeArray{T,2}

Expand Down
5 changes: 3 additions & 2 deletions src/collect_as.jl
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,12 @@ end
Tries to construct a value of type `t` from the iterator `iterator`. The type `t`
must either be concrete, or a `UnionAll` without constraints.
"""
function collect_as(::Type{T}, iterator) where {T<:FixedSizeArray}
function collect_as(::Type{FSA}, iterator) where {FSA<:FixedSizeArray}
size_class = Base.IteratorSize(iterator)
if size_class == Base.IsInfinite()
throw(ArgumentError("iterator is infinite, can't fit infinitely many elements into a `FixedSizeArray`"))
end
T = check_constructor_is_allowed(FSA)
mem = parent_type_with_default(T)
output_dimension_count = checked_dimension_count_of(T, size_class)
fsv = if (
Expand All @@ -163,5 +164,5 @@ function collect_as(::Type{T}, iterator) where {T<:FixedSizeArray}
fsv # `size(iterator)` may throw in this branch
else
reshape(fsv, size(iterator))
end::T
end
end
2 changes: 1 addition & 1 deletion test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ end
collect_as(T, iter)
true
catch e
(e isa TypeError) || rethrow()
(e isa ArgumentError) || rethrow()
false
end
returns && @test collect_as(T, iter) isa T
Expand Down

0 comments on commit 30e318f

Please sign in to comment.