Change fieldnames() and propertynames() to return a tuple rather than…

… an array (#25725) Using an immutable structure makes sense since the names cannot be modified, and it avoids an allocation.
JuliaLang · Feb 9, 2018 · 16620b6 · 16620b6
1 parent c174fd8
commit 16620b6
Show file tree

Hide file tree

Showing 14 changed files with 44 additions and 36 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -423,6 +423,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
 --------------------
 
@@ -1287,4 +1290,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)