From abd3a9111c556e8eaeecf300cf3a285f9d9260e9 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Sat, 23 Jan 2016 23:58:11 -0500 Subject: [PATCH] replace `field_offset` and `fieldoffset` with `fieldoffset` `field_offset` was untested, undocumented, unexported, and incorrect (off-by-one error in implementation) `fieldoffset` just isn't particularly optimal, since it has to allocate a new array each time also make minor cleanup and test-coverage enhancements to related reflection methods --- base/deprecated.jl | 11 +++++++ base/docs/helpdb/Base.jl | 41 ------------------------ base/exports.jl | 3 +- base/reflection.jl | 64 +++++++++++++++++++++++++++++--------- base/sparse/cholmod.jl | 4 +-- doc/devdocs/reflection.rst | 4 +-- doc/stdlib/base.rst | 12 +++++-- src/sys.c | 14 ++------- test/reflection.jl | 43 +++++++++++++++++++++++++ 9 files changed, 121 insertions(+), 75 deletions(-) diff --git a/base/deprecated.jl b/base/deprecated.jl index 13cab4b5a14c5..5b9cbbf42fcce 100644 --- a/base/deprecated.jl +++ b/base/deprecated.jl @@ -967,3 +967,14 @@ end #https://github.com/JuliaLang/julia/issues/14608 @deprecate readall readstring @deprecate readbytes read + +@deprecate field_offset(x::DataType, idx) fieldoffset(x, idx+1) +@noinline function fieldoffsets(x::DataType) + depwarn("fieldoffsets is deprecated. use `map(idx->fieldoffset(x, idx), 1:nfields(x))` instead", :fieldoffsets) + nf = nfields(x) + offsets = Array(Int, nf) + for i = 1:nf + offsets[i] = fieldoffset(x, i) + end + return offsets +end diff --git a/base/docs/helpdb/Base.jl b/base/docs/helpdb/Base.jl index 6cae69720f377..add5299ce304a 100644 --- a/base/docs/helpdb/Base.jl +++ b/base/docs/helpdb/Base.jl @@ -313,13 +313,6 @@ Compute a "2d histogram" with respect to the bins delimited by the edges given i """ hist2d! -""" - fieldtype(T, name::Symbol | index::Int) - -Determine the declared type of a field (specified by name or index) in a composite DataType `T`. -""" -fieldtype - """ hypot(x, y) @@ -1968,13 +1961,6 @@ shift in each dimension. """ circshift -""" - fieldnames(x::DataType) - -Get an array of the fields of a `DataType`. -""" -fieldnames - """ yield() @@ -2086,33 +2072,6 @@ Right bit shift operator, preserving the sign of `x`. """ Base.(:(>>)) -""" - fieldoffsets(type) - -The byte offset of each field of a type relative to the data start. For example, we could -use it in the following manner to summarize information about a struct type: - -```jldoctest -julia> structinfo(T) = [zip(fieldoffsets(T),fieldnames(T),T.types)...]; - -julia> structinfo(StatStruct) -12-element Array{Tuple{Int64,Symbol,DataType},1}: - (0,:device,UInt64) - (8,:inode,UInt64) - (16,:mode,UInt64) - (24,:nlink,Int64) - (32,:uid,UInt64) - (40,:gid,UInt64) - (48,:rdev,UInt64) - (56,:size,Int64) - (64,:blksize,Int64) - (72,:blocks,Int64) - (80,:mtime,Float64) - (88,:ctime,Float64) -``` -""" -fieldoffsets - """ randn([rng], [dims...]) diff --git a/base/exports.jl b/base/exports.jl index 0ac0b168b6583..93a871d79d7b7 100644 --- a/base/exports.jl +++ b/base/exports.jl @@ -1048,7 +1048,8 @@ export # types convert, - fieldoffsets, + fieldoffset, + fieldname, fieldnames, isleaftype, oftype, diff --git a/base/reflection.jl b/base/reflection.jl index 9159e5c8493f8..c77d4162498b2 100644 --- a/base/reflection.jl +++ b/base/reflection.jl @@ -51,7 +51,18 @@ function resolve(g::GlobalRef; force::Bool=false) return g end -fieldnames(t::DataType) = Symbol[n for n in t.name.names ] +""" + fieldname(x::DataType, i) + +Get the name of field `i` of a `DataType`. +""" +fieldname(t::DataType, i::Integer) = t.name.names[i]::Symbol + +""" + fieldnames(x::DataType) + +Get an array of the fields of a `DataType`. +""" function fieldnames(v) t = typeof(v) if !isa(t,DataType) @@ -59,8 +70,7 @@ function fieldnames(v) end return fieldnames(t) end - -fieldname(t::DataType, i::Integer) = t.name.names[i]::Symbol +fieldnames(t::DataType) = Symbol[fieldname(t, n) for n in 1:nfields(t)] isconst(s::Symbol) = ccall(:jl_is_const, Int32, (Ptr{Void}, Any), C_NULL, s) != 0 @@ -82,14 +92,41 @@ isleaftype(t::ANY) = (@_pure_meta; ccall(:jl_is_leaf_type, Int32, (Any,), t) != typeintersect(a::ANY,b::ANY) = (@_pure_meta; ccall(:jl_type_intersection, Any, (Any,Any), a, b)) typeseq(a::ANY,b::ANY) = (@_pure_meta; a<:b && b<:a) -function fieldoffsets(x::DataType) - offsets = Array(Int, nfields(x)) - ccall(:jl_field_offsets, Void, (Any, Ptr{Int}), x, offsets) - offsets -end - -type_alignment(x::DataType) = (@_pure_meta; ccall(:jl_get_alignment,Csize_t,(Any,),x)) -field_offset(x::DataType,idx) = (@_pure_meta; ccall(:jl_get_field_offset,Csize_t,(Any,Int32),x,idx)) +""" + fieldoffset(type, i) + +The byte offset of field `i` of a type relative to the data start. For example, we could +use it in the following manner to summarize information about a struct type: + +```jldoctest +julia> structinfo(T) = [(fieldoffset(T,i), fieldname(T,i), fieldtype(T,i)) for i = 1:nfields(T)]; + +julia> structinfo(StatStruct) +12-element Array{Tuple{Int64,Symbol,DataType},1}: + (0,:device,UInt64) + (8,:inode,UInt64) + (16,:mode,UInt64) + (24,:nlink,Int64) + (32,:uid,UInt64) + (40,:gid,UInt64) + (48,:rdev,UInt64) + (56,:size,Int64) + (64,:blksize,Int64) + (72,:blocks,Int64) + (80,:mtime,Float64) + (88,:ctime,Float64) +``` +""" +fieldoffset(x::DataType, idx::Integer) = (@_pure_meta; ccall(:jl_get_field_offset, Csize_t, (Any, Cint), x, idx)) + +""" + fieldtype(T, name::Symbol | index::Int) + +Determine the declared type of a field (specified by name or index) in a composite DataType `T`. +""" +fieldtype + +type_alignment(x::DataType) = (@_pure_meta; ccall(:jl_get_alignment, Csize_t, (Any,), x)) # return all instances, for types that can be enumerated function instances end @@ -266,10 +303,9 @@ function code_typed(f::Function, types::ANY=Tuple; optimize=true) optimize=false) end if !isa(tree, Expr) - push!(asts, ccall(:jl_uncompress_ast, Any, (Any,Any), linfo, tree)) - else - push!(asts, tree) + tree = ccall(:jl_uncompress_ast, Any, (Any,Any), linfo, tree) end + push!(asts, tree) end asts end diff --git a/base/sparse/cholmod.jl b/base/sparse/cholmod.jl index 03ef1f62a86b1..103bcd326324b 100644 --- a/base/sparse/cholmod.jl +++ b/base/sparse/cholmod.jl @@ -1035,11 +1035,9 @@ sparse{Tv}(FC::FactorComponent{Tv,:LD}) = sparse(Sparse(Factor(FC))) # Calculate the offset into the stype field of the cholmod_sparse_struct and # change the value -let offidx=findfirst(fieldnames(C_Sparse) .== :stype) - +let offset = fieldoffset(C_Sparse{Float64}, findfirst(fieldnames(C_Sparse) .== :stype)) global change_stype! function change_stype!(A::Sparse, i::Integer) - offset = fieldoffsets(C_Sparse)[offidx] unsafe_store!(convert(Ptr{Cint}, A.p), i, div(offset, 4) + 1) return A end diff --git a/doc/devdocs/reflection.rst b/doc/devdocs/reflection.rst index e5356ada69dee..2f3ace38417db 100644 --- a/doc/devdocs/reflection.rst +++ b/doc/devdocs/reflection.rst @@ -74,8 +74,8 @@ The internal representation of a :obj:`DataType` is critically important when in C code and several functions are available to inspect these details. :func:`isbits(T::DataType) ` returns true if ``T`` is stored with C-compatible alignment. -:func:`fieldoffsets(T::DataType) ` returns the (byte) offset for each -field relative to the start of the type. +:func:`fieldoffset(T::DataType, i::Integer) ` returns the (byte) offset for +field `i` relative to the start of the type. .. rubric:: Function methods diff --git a/doc/stdlib/base.rst b/doc/stdlib/base.rst index ad10f24ad9511..c2eac358e24a8 100644 --- a/doc/stdlib/base.rst +++ b/doc/stdlib/base.rst @@ -520,15 +520,15 @@ Types Assign ``x`` to a named field in ``value`` of composite type. The syntax ``a.b = c`` calls ``setfield!(a, :b, c)``\ , and the syntax ``a.(b) = c`` calls ``setfield!(a, b, c)``\ . -.. function:: fieldoffsets(type) +.. function:: fieldoffset(type, i) .. Docstring generated from Julia source - The byte offset of each field of a type relative to the data start. For example, we could use it in the following manner to summarize information about a struct type: + The byte offset of field ``i`` of a type relative to the data start. For example, we could use it in the following manner to summarize information about a struct type: .. doctest:: - julia> structinfo(T) = [zip(fieldoffsets(T),fieldnames(T),T.types)...]; + julia> structinfo(T) = [(fieldoffset(T,i), fieldname(T,i), fieldtype(T,i)) for i = 1:nfields(T)]; julia> structinfo(StatStruct) 12-element Array{Tuple{Int64,Symbol,DataType},1}: @@ -1252,6 +1252,12 @@ Reflection Get an array of the fields of a ``DataType``\ . +.. function:: fieldname(x::DataType, i) + + .. Docstring generated from Julia source + + Get the name of field ``i`` of a ``DataType``\ . + .. function:: isconst([m::Module], s::Symbol) -> Bool .. Docstring generated from Julia source diff --git a/src/sys.c b/src/sys.c index 270c32858edce..f4fde2cc56eba 100644 --- a/src/sys.c +++ b/src/sys.c @@ -567,14 +567,6 @@ JL_DLLEXPORT jl_value_t *jl_is_char_signed(void) return ((char)255) < 0 ? jl_true : jl_false; } -JL_DLLEXPORT void jl_field_offsets(jl_datatype_t *dt, ssize_t *offsets) -{ - size_t i; - for(i=0; i < jl_datatype_nfields(dt); i++) { - offsets[i] = jl_field_offset(dt, i); - } -} - // -- misc sysconf info -- #ifdef _OS_WINDOWS_ @@ -624,9 +616,9 @@ JL_DLLEXPORT long jl_SC_CLK_TCK(void) JL_DLLEXPORT size_t jl_get_field_offset(jl_datatype_t *ty, int field) { - if (field > jl_datatype_nfields(ty)) - jl_error("This type does not have that many fields"); - return jl_field_offset(ty, field); + if (field > jl_datatype_nfields(ty) || field < 1) + jl_bounds_error_int((jl_value_t*)ty, field); + return jl_field_offset(ty, field - 1); } JL_DLLEXPORT size_t jl_get_alignment(jl_datatype_t *ty) diff --git a/test/reflection.jl b/test/reflection.jl index 608522b76e29b..018b082166f3a 100644 --- a/test/reflection.jl +++ b/test/reflection.jl @@ -201,6 +201,21 @@ end @test_throws ArgumentError which(is, Tuple{Int, Int}) +module TestingExported +using Base.Test +import Base.isexported +global this_is_not_defined +export this_is_not_defined +@test_throws ErrorException which(:this_is_not_defined) +@test_throws ErrorException @which this_is_not_defined +@test_throws ErrorException which(:this_is_not_exported) +@test isexported(current_module(), :this_is_not_defined) +@test !isexported(current_module(), :this_is_not_exported) +const a_value = 1 +@test which(:a_value) == current_module() +@test !isexported(current_module(), :a_value) +end + # issue #13264 @test isa((@which vcat(1...)), Method) @@ -219,3 +234,31 @@ let ex = :(a + b) ex.typ = Integer @test string(ex) == "(a + b)::Integer" end + +type TLayout + x::Int8 + y::Int16 + z::Int32 +end +tlayout = TLayout(5,7,11) +@test fieldnames(tlayout) == fieldnames(TLayout) == [:x, :y, :z] +@test [(fieldoffset(TLayout,i), fieldname(TLayout,i), fieldtype(TLayout,i)) for i = 1:nfields(TLayout)] == + [(0, :x, Int8), (2, :y, Int16), (4, :z, Int32)] +@test_throws BoundsError fieldtype(TLayout, 0) +@test_throws BoundsError fieldname(TLayout, 0) +@test_throws BoundsError fieldoffset(TLayout, 0) +@test_throws BoundsError fieldtype(TLayout, 4) +@test_throws BoundsError fieldname(TLayout, 4) +@test_throws BoundsError fieldoffset(TLayout, 4) + +import Base: isstructtype, type_alignment, return_types +@test !isstructtype(Union{}) +@test !isstructtype(Union{Int,Float64}) +@test !isstructtype(Int) +@test isstructtype(TLayout) +@test type_alignment(UInt16) == 2 +@test type_alignment(TLayout) == 4 +let rts = return_types(TLayout) + @test length(rts) >= 3 # general constructor, specific constructor, and call-to-convert adapter(s) + @test all(rts .== TLayout) +end