From 1ebc2ce7c099107142c47400e39e49afcf341a94 Mon Sep 17 00:00:00 2001 From: Milan Bouchet-Valat Date: Wed, 24 Jan 2018 10:17:55 +0100 Subject: [PATCH] Change fieldnames() and propertynames() to return a tuple rather than an array Using an immutable structure makes sense since the names cannot be modified, and it avoids an allocation. --- NEWS.md | 4 ++++ base/compiler/optimize.jl | 6 ++++-- base/reflection.jl | 14 ++++++-------- base/util.jl | 11 +---------- stdlib/LinearAlgebra/src/bunchkaufman.jl | 3 ++- stdlib/LinearAlgebra/src/cholesky.jl | 6 ++++-- stdlib/LinearAlgebra/src/hessenberg.jl | 3 ++- stdlib/LinearAlgebra/src/lq.jl | 3 ++- stdlib/LinearAlgebra/src/lu.jl | 3 ++- stdlib/LinearAlgebra/src/qr.jl | 7 +++++-- stdlib/LinearAlgebra/src/schur.jl | 6 ++++-- stdlib/LinearAlgebra/src/svd.jl | 6 ++++-- stdlib/LinearAlgebra/test/lu.jl | 4 ++-- test/reflection.jl | 4 ++-- 14 files changed, 44 insertions(+), 36 deletions(-) diff --git a/NEWS.md b/NEWS.md index fd68ecce6d5ed..f7e5670a19f60 100644 --- a/NEWS.md +++ b/NEWS.md @@ -420,6 +420,9 @@ 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` and `propertynames` functions now return a tuple rather than + an array ([#25725]). + Library improvements -------------------- @@ -1284,4 +1287,5 @@ Command-line option changes [#25634]: https://github.com/JuliaLang/julia/issues/25634 [#25654]: https://github.com/JuliaLang/julia/issues/25654 [#25655]: https://github.com/JuliaLang/julia/issues/25655 +[#25725]: https://github.com/JuliaLang/julia/issues/25725 [#25745]: https://github.com/JuliaLang/julia/issues/25745 diff --git a/base/compiler/optimize.jl b/base/compiler/optimize.jl index 160ec9b922340..fda39b098f6bd 100644 --- a/base/compiler/optimize.jl +++ b/base/compiler/optimize.jl @@ -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) @@ -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 diff --git a/base/reflection.jl b/base/reflection.jl index f3d71ae09970f..7384560e650ef 100644 --- a/base/reflection.jl +++ b/base/reflection.jl @@ -145,19 +145,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(Rational) -2-element Array{Symbol,1}: - :num - :den +(:num, :den) ``` """ -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 @@ -1062,8 +1060,8 @@ 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 -is typically the same as [`fieldnames(typeof(x))`](@ref), but types +Get a tuple or a vector 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. diff --git a/base/util.jl b/base/util.jl index feddea5e464a6..b4e7ed12f1807 100644 --- a/base/util.jl +++ b/base/util.jl @@ -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 diff --git a/stdlib/LinearAlgebra/src/bunchkaufman.jl b/stdlib/LinearAlgebra/src/bunchkaufman.jl index 9ac88bf8dc587..80db51dcb802c 100644 --- a/stdlib/LinearAlgebra/src/bunchkaufman.jl +++ b/stdlib/LinearAlgebra/src/bunchkaufman.jl @@ -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 diff --git a/stdlib/LinearAlgebra/src/cholesky.jl b/stdlib/LinearAlgebra/src/cholesky.jl index 5e1046c56ef16..1f390615da0e6 100644 --- a/stdlib/LinearAlgebra/src/cholesky.jl +++ b/stdlib/LinearAlgebra/src/cholesky.jl @@ -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) @@ -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 diff --git a/stdlib/LinearAlgebra/src/hessenberg.jl b/stdlib/LinearAlgebra/src/hessenberg.jl index 54be7719dcda0..d1014c2199bde 100644 --- a/stdlib/LinearAlgebra/src/hessenberg.jl +++ b/stdlib/LinearAlgebra/src/hessenberg.jl @@ -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)) diff --git a/stdlib/LinearAlgebra/src/lq.jl b/stdlib/LinearAlgebra/src/lq.jl index 4bea24b582250..1e875454e07f1 100644 --- a/stdlib/LinearAlgebra/src/lq.jl +++ b/stdlib/LinearAlgebra/src/lq.jl @@ -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) = lmul!(A, setindex!(zeros(eltype(A), size(A, 2)), 1, j))[i] diff --git a/stdlib/LinearAlgebra/src/lu.jl b/stdlib/LinearAlgebra/src/lu.jl index d83edc686efa1..17cab622dad5b 100644 --- a/stdlib/LinearAlgebra/src/lu.jl +++ b/stdlib/LinearAlgebra/src/lu.jl @@ -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 diff --git a/stdlib/LinearAlgebra/src/qr.jl b/stdlib/LinearAlgebra/src/qr.jl index f358bc9522d27..25a1a69e2527d 100644 --- a/stdlib/LinearAlgebra/src/qr.jl +++ b/stdlib/LinearAlgebra/src/qr.jl @@ -454,7 +454,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 @@ -475,7 +477,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 diff --git a/stdlib/LinearAlgebra/src/schur.jl b/stdlib/LinearAlgebra/src/schur.jl index 79ce8725fa289..410cce702409c 100644 --- a/stdlib/LinearAlgebra/src/schur.jl +++ b/stdlib/LinearAlgebra/src/schur.jl @@ -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, mime::MIME{Symbol("text/plain")}, F::Schur) println(io, summary(F)) @@ -280,7 +281,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 diff --git a/stdlib/LinearAlgebra/src/svd.jl b/stdlib/LinearAlgebra/src/svd.jl index 339c7ac03ef61..29a9b8da1c75d 100644 --- a/stdlib/LinearAlgebra/src/svd.jl +++ b/stdlib/LinearAlgebra/src/svd.jl @@ -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) @@ -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) diff --git a/stdlib/LinearAlgebra/test/lu.jl b/stdlib/LinearAlgebra/test/lu.jl index 0cbd8023fbcf6..bf6de1cb38127 100644 --- a/stdlib/LinearAlgebra/test/lu.jl +++ b/stdlib/LinearAlgebra/test/lu.jl @@ -266,9 +266,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 diff --git a/test/reflection.jl b/test/reflection.jl index 9993db250ecbd..348ef00bb8a94 100644 --- a/test/reflection.jl +++ b/test/reflection.jl @@ -218,7 +218,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) @@ -232,7 +232,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)