Skip to content

Commit

Permalink
Change fieldnames() and propertynames() to return a tuple rather than…
Browse files Browse the repository at this point in the history
… an array

Using an immutable structure makes sense since the names cannot be modified,
and it avoids an allocation.
  • Loading branch information
nalimilan committed Jan 24, 2018
1 parent 0a42222 commit 40dbfd7
Show file tree
Hide file tree
Showing 14 changed files with 43 additions and 36 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,8 @@ This section lists changes that do not have deprecation warnings.
* The `tempname` function used to create a file on Windows but not on other
platforms. It now never creates a file ([#9053]).

* The `fieldnames` function now returns a tuple rather than an array ([#25725]).

Library improvements
--------------------

Expand Down Expand Up @@ -1249,3 +1251,4 @@ Command-line option changes
[#25622]: https://github.com/JuliaLang/julia/issues/25622
[#25634]: https://github.com/JuliaLang/julia/issues/25634
[#25654]: https://github.com/JuliaLang/julia/issues/25654
[#25725]: https://github.com/JuliaLang/julia/issues/25725
6 changes: 4 additions & 2 deletions base/compiler/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2950,7 +2950,8 @@ function structinfo_constant(ctx::AllocOptContext, @nospecialize(v), vt::DataTyp
if vt <: Tuple
si = StructInfo(Vector{Any}(uninitialized, nf), Symbol[], vt, false, false)
else
si = StructInfo(Vector{Any}(uninitialized, nf), fieldnames(vt), vt, false, false)
si = StructInfo(Vector{Any}(uninitialized, nf), collect(Symbol, fieldnames(vt)),
vt, false, false)
end
for i in 1:nf
if isdefined(v, i)
Expand All @@ -2966,7 +2967,8 @@ end
structinfo_tuple(ex::Expr) = StructInfo(ex.args[2:end], Symbol[], Tuple, false, false)
function structinfo_new(ctx::AllocOptContext, ex::Expr, vt::DataType)
nf = fieldcount(vt)
si = StructInfo(Vector{Any}(uninitialized, nf), fieldnames(vt), vt, vt.mutable, true)
si = StructInfo(Vector{Any}(uninitialized, nf), collect(Symbol, fieldnames(vt)),
vt, vt.mutable, true)
ninit = length(ex.args) - 1
for i in 1:nf
if i <= ninit
Expand Down
14 changes: 6 additions & 8 deletions base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -129,19 +129,17 @@ fieldname(t::Type{<:Tuple}, i::Integer) =
"""
fieldnames(x::DataType)
Get an array of the fields of a `DataType`.
Get a tuple with the names of the fields of a `DataType`.
# Examples
```jldoctest
julia> fieldnames(Hermitian)
2-element Array{Symbol,1}:
:data
:uplo
julia> fieldnames(Pair)
(:first, :second)
```
"""
fieldnames(t::DataType) = Symbol[fieldname(t, n) for n in 1:fieldcount(t)]
fieldnames(t::DataType) = ntuple(i -> fieldname(t, i), fieldcount(t))
fieldnames(t::UnionAll) = fieldnames(unwrap_unionall(t))
fieldnames(t::Type{<:Tuple}) = Int[n for n in 1:fieldcount(t)]
fieldnames(t::Type{<:Tuple}) = ntuple(identity, fieldcount(t))

"""
nameof(t::DataType) -> Symbol
Expand Down Expand Up @@ -1189,7 +1187,7 @@ max_world(m::Core.MethodInstance) = reinterpret(UInt, m.max_world)
"""
propertynames(x, private=false)
Get an array of the properties (`x.property`) of an object `x`. This
Get a tuple of the properties (`x.property`) of an object `x`. This
is typically the same as [`fieldnames(typeof(x))`](@ref), but types
that overload [`getproperty`](@ref) should generally overload `propertynames`
as well to get the properties of an instance of the type.
Expand Down
11 changes: 1 addition & 10 deletions base/util.jl
Original file line number Diff line number Diff line change
Expand Up @@ -288,16 +288,7 @@ julia> gctime
0.0055765
julia> fieldnames(typeof(memallocs))
9-element Array{Symbol,1}:
:allocd
:malloc
:realloc
:poolalloc
:bigalloc
:freecall
:total_time
:pause
:full_sweep
(:allocd, :malloc, :realloc, :poolalloc, :bigalloc, :freecall, :total_time, :pause, :full_sweep)
julia> memallocs.total_time
5576500
Expand Down
3 changes: 2 additions & 1 deletion stdlib/LinearAlgebra/src/bunchkaufman.jl
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,8 @@ function getproperty(B::BunchKaufman{T}, d::Symbol) where {T<:BlasFloat}
end
end

Base.propertynames(B::BunchKaufman, private::Bool=false) = append!([:p,:P,:L,:U,:D], private ? fieldnames(typeof(B)) : Symbol[])
Base.propertynames(B::BunchKaufman, private::Bool=false) =
(:p, :P, :L, :U, :D, (private ? fieldnames(typeof(B)) : ())...)

issuccess(B::BunchKaufman) = B.info == 0

Expand Down
6 changes: 4 additions & 2 deletions stdlib/LinearAlgebra/src/cholesky.jl
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,8 @@ function getproperty(C::Cholesky, d::Symbol)
return getfield(C, d)
end
end
Base.propertynames(F::Cholesky, private::Bool=false) = append!([:U,:L,:UL], private ? fieldnames(typeof(F)) : Symbol[])
Base.propertynames(F::Cholesky, private::Bool=false) =
(:U, :L, :UL, (private ? fieldnames(typeof(F)) : ())...)

function getproperty(C::CholeskyPivoted{T}, d::Symbol) where T<:BlasFloat
Cfactors = getfield(C, :factors)
Expand All @@ -415,7 +416,8 @@ function getproperty(C::CholeskyPivoted{T}, d::Symbol) where T<:BlasFloat
return getfield(C, d)
end
end
Base.propertynames(F::CholeskyPivoted, private::Bool=false) = append!([:U,:L,:p,:P], private ? fieldnames(typeof(F)) : Symbol[])
Base.propertynames(F::CholeskyPivoted, private::Bool=false) =
(:U, :L, :p, :P, (private ? fieldnames(typeof(F)) : ())...)

issuccess(C::Cholesky) = C.info == 0

Expand Down
3 changes: 2 additions & 1 deletion stdlib/LinearAlgebra/src/hessenberg.jl
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ function getproperty(F::Hessenberg, d::Symbol)
return getfield(F, d)
end

Base.propertynames(F::Hessenberg, private::Bool=false) = append!([:Q,:H], private ? fieldnames(typeof(F)) : Symbol[])
Base.propertynames(F::Hessenberg, private::Bool=false) =
(:Q, :H, (private ? fieldnames(typeof(F)) : ())...)

function getindex(A::HessenbergQ, i::Integer, j::Integer)
x = zeros(eltype(A), size(A, 1))
Expand Down
3 changes: 2 additions & 1 deletion stdlib/LinearAlgebra/src/lq.jl
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ function getproperty(F::LQ, d::Symbol)
end
end

Base.propertynames(F::LQ, private::Bool=false) = append!([:L,:Q], private ? fieldnames(typeof(F)) : Symbol[])
Base.propertynames(F::LQ, private::Bool=false) =
(:L, :Q, (private ? fieldnames(typeof(F)) : ())...)

getindex(A::LQPackedQ, i::Integer, j::Integer) =
mul2!(A, setindex!(zeros(eltype(A), size(A, 2)), 1, j))[i]
Expand Down
3 changes: 2 additions & 1 deletion stdlib/LinearAlgebra/src/lu.jl
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,8 @@ function getproperty(F::LU{T,<:StridedMatrix}, d::Symbol) where T
end
end

Base.propertynames(F::LU, private::Bool=false) = append!([:L,:U,:p,:P], private ? fieldnames(typeof(F)) : Symbol[])
Base.propertynames(F::LU, private::Bool=false) =
(:L, :U, :p, :P, (private ? fieldnames(typeof(F)) : ())...)

issuccess(F::LU) = F.info == 0

Expand Down
7 changes: 5 additions & 2 deletions stdlib/LinearAlgebra/src/qr.jl
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,9 @@ function getproperty(F::QRCompactWY, d::Symbol)
getfield(F, d)
end
end
Base.propertynames(F::Union{QR,QRCompactWY}, private::Bool=false) = append!([:R,:Q], private ? fieldnames(typeof(F)) : Symbol[])
Base.propertynames(F::Union{QR,QRCompactWY}, private::Bool=false) =
(:R, :Q, (private ? fieldnames(typeof(F)) : ())...)

function getproperty(F::QRPivoted{T}, d::Symbol) where T
m, n = size(F)
if d == :R
Expand All @@ -470,7 +472,8 @@ function getproperty(F::QRPivoted{T}, d::Symbol) where T
getfield(F, d)
end
end
Base.propertynames(F::QRPivoted, private::Bool=false) = append!([:R,:Q,:p,:P], private ? fieldnames(typeof(F)) : Symbol[])
Base.propertynames(F::QRPivoted, private::Bool=false) =
(:R, :Q, :p, :P, (private ? fieldnames(typeof(F)) : ())...)

abstract type AbstractQ{T} <: AbstractMatrix{T} end

Expand Down
6 changes: 4 additions & 2 deletions stdlib/LinearAlgebra/src/schur.jl
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ function getproperty(F::Schur, d::Symbol)
end
end

Base.propertynames(F::Schur) = append!([:Schur,:vectors], fieldnames(typeof(F)))
Base.propertynames(F::Schur) =
(:Schur, :vectors, fieldnames(typeof(F))...)

function show(io::IO, F::Schur)
println(io, "$(typeof(F)) with factors T and Z:")
Expand Down Expand Up @@ -276,7 +277,8 @@ function getproperty(F::GeneralizedSchur, d::Symbol)
end
end

Base.propertynames(F::GeneralizedSchur) = append!([:values,:left,:right], fieldnames(typeof(F)))
Base.propertynames(F::GeneralizedSchur) =
(:values, :left, :right, fieldnames(typeof(F))...)

"""
schur(A::StridedMatrix, B::StridedMatrix) -> S::StridedMatrix, T::StridedMatrix, Q::StridedMatrix, Z::StridedMatrix, α::Vector, β::Vector
Expand Down
6 changes: 4 additions & 2 deletions stdlib/LinearAlgebra/src/svd.jl
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,8 @@ function getproperty(F::SVD, d::Symbol)
end
end

Base.propertynames(F::SVD, private::Bool=false) = private ? append!([:V], fieldnames(typeof(F))) : [:U,:S,:V,:Vt]
Base.propertynames(F::SVD, private::Bool=false) =
private ? (:V, fieldnames(typeof(F))...) : (:U, :S, :V, :Vt)

"""
svdvals!(A)
Expand Down Expand Up @@ -463,7 +464,8 @@ svd(x::Number, y::Number) = first.(svd(fill(x, 1, 1), fill(y, 1, 1)))
end
end

Base.propertynames(F::GeneralizedSVD) = append!([:alpha,:beta,:vals,:S,:D1,:D2,:R0], fieldnames(typeof(F)))
Base.propertynames(F::GeneralizedSVD) =
(:alpha, :beta, :vals, :S, :D1, :D2, :R0, fieldnames(typeof(F))...)

"""
svdvals!(A, B)
Expand Down
4 changes: 2 additions & 2 deletions stdlib/LinearAlgebra/test/lu.jl
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,9 @@ U factor:
end

@testset "propertynames" begin
names = sort!(string.(Base.propertynames(lufact(rand(3,3)))))
names = sort!(collect(string.(Base.propertynames(lufact(rand(3,3))))))
@test names == ["L", "P", "U", "p"]
allnames = sort!(string.(Base.propertynames(lufact(rand(3,3)), true)))
allnames = sort!(collect(string.(Base.propertynames(lufact(rand(3,3)), true))))
@test allnames == ["L", "P", "U", "factors", "info", "ipiv", "p"]
end

Expand Down
4 changes: 2 additions & 2 deletions test/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ mutable struct TLayout
z::Int32
end
tlayout = TLayout(5,7,11)
@test fieldnames(TLayout) == [:x, :y, :z] == Base.propertynames(tlayout)
@test fieldnames(TLayout) == (:x, :y, :z) == Base.propertynames(tlayout)
@test [(fieldoffset(TLayout,i), fieldname(TLayout,i), fieldtype(TLayout,i)) for i = 1:fieldcount(TLayout)] ==
[(0, :x, Int8), (2, :y, Int16), (4, :z, Int32)]
@test_throws BoundsError fieldtype(TLayout, 0)
Expand All @@ -348,7 +348,7 @@ tlayout = TLayout(5,7,11)
@test fieldtype(Tuple{Vararg{Int8}}, 10) === Int8
@test_throws BoundsError fieldtype(Tuple{Vararg{Int8}}, 0)

@test fieldnames(NTuple{3, Int}) == [fieldname(NTuple{3, Int}, i) for i = 1:3] == [1, 2, 3]
@test fieldnames(NTuple{3, Int}) == ntuple(i -> fieldname(NTuple{3, Int}, i), 3) == (1, 2, 3)
@test_throws BoundsError fieldname(NTuple{3, Int}, 0)
@test_throws BoundsError fieldname(NTuple{3, Int}, 4)

Expand Down

0 comments on commit 40dbfd7

Please sign in to comment.