Skip to content

Commit

Permalink
attempt some code quality improvements
Browse files Browse the repository at this point in the history
remove unused type parameters from some functions, add some useful ones
to others, and remove the fallback `==` method in Core.Compiler.
  • Loading branch information
vtjnash committed Jul 10, 2020
1 parent 3fec075 commit dcc0696
Show file tree
Hide file tree
Showing 16 changed files with 61 additions and 39 deletions.
1 change: 1 addition & 0 deletions base/Base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ include("range.jl")
include("error.jl")

# core numeric operations & types
==(x, y) = x === y
include("bool.jl")
include("number.jl")
include("int.jl")
Expand Down
2 changes: 1 addition & 1 deletion base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ size(a::Array{<:Any,N}) where {N} = (@_inline_meta; ntuple(M -> size(a, M), Val(

asize_from(a::Array, n) = n > ndims(a) ? () : (arraysize(a,n), asize_from(a, n+1)...)

allocatedinline(::Type{T}) where {T} = (@_pure_meta; ccall(:jl_stored_inline, Cint, (Any,), T) != Cint(0))
allocatedinline(T::Type) = (@_pure_meta; ccall(:jl_stored_inline, Cint, (Any,), T) != Cint(0))

"""
Base.isbitsunion(::Type{T})
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1326,7 +1326,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
if isa(fname, Slot)
changes = StateUpdate(fname, VarState(Any, false), changes)
end
elseif hd === :inbounds || hd === :meta || hd === :loopinfo || hd == :code_coverage_effect
elseif hd === :inbounds || hd === :meta || hd === :loopinfo || hd === :code_coverage_effect
# these do not generate code
else
t = abstract_eval_statement(interp, stmt, changes, frame)
Expand Down
1 change: 1 addition & 0 deletions base/compiler/compiler.jl
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ include("expr.jl")
include("error.jl")

# core numeric operations & types
==(x::T, y::T) where {T} = x === y
include("bool.jl")
include("number.jl")
include("int.jl")
Expand Down
5 changes: 4 additions & 1 deletion base/compiler/inferencestate.jl
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,10 @@ update_valid_age!(edge::InferenceState, sv::InferenceState) = update_valid_age!(
function record_ssa_assign(ssa_id::Int, @nospecialize(new), frame::InferenceState)
old = frame.src.ssavaluetypes[ssa_id]
if old === NOT_FOUND || !(new old)
frame.src.ssavaluetypes[ssa_id] = tmerge(old, new)
# typically, we expect that old ⊑ new (that output information only
# gets less precise with worse input information), but to actually
# guarantee convergence we need to use tmerge here to ensure that is true
frame.src.ssavaluetypes[ssa_id] = old === NOT_FOUND ? new : tmerge(old, new)
W = frame.ip
s = frame.stmt_types
for r in frame.ssavalue_uses[ssa_id]
Expand Down
4 changes: 2 additions & 2 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -839,9 +839,9 @@ function inline_splatnew!(ir::IRCode, idx::Int)
tup = eargs[2]
tt = argextype(tup, ir, ir.sptypes)
tnf = nfields_tfunc(tt)
# TODO: hoisting this tnf.val == nf.val check into codegen
# TODO: hoisting this tnf.val === nf.val check into codegen
# would enable us to almost always do this transform
if tnf isa Const && tnf.val == nf.val
if tnf isa Const && tnf.val === nf.val
n = tnf.val
new_argexprs = Any[eargs[1]]
for j = 1:n
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/ssair/passes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ function walk_to_defs(compact::IncrementalCompact, @nospecialize(defssa), @nospe
end
val = new_def
end
if def == val
if def === val
# This shouldn't really ever happen, but
# patterns like this can occur in dead code,
# so bail out.
Expand Down
6 changes: 3 additions & 3 deletions base/compiler/ssair/slot2ssa.jl
Original file line number Diff line number Diff line change
Expand Up @@ -692,13 +692,13 @@ function construct_ssa!(ci::CodeInfo, ir::IRCode, domtree::DomTree, defuse, narg
for (idx, slot) in Iterators.enumerate(phi_slots[item])
ssaval, node = phi_nodes[item][idx]
incoming_val = incoming_vals[slot]
if incoming_val == SSAValue(-1)
if incoming_val === SSAValue(-1)
# Optimistically omit this path.
# Liveness analysis would probably have prevented us from inserting this phi node
continue
end
push!(node.edges, pred)
if incoming_val == undef_token
if incoming_val === undef_token
resize!(node.values, length(node.values)+1)
else
push!(node.values, incoming_val)
Expand All @@ -708,7 +708,7 @@ function construct_ssa!(ci::CodeInfo, ir::IRCode, domtree::DomTree, defuse, narg
if isa(incoming_val, NewSSAValue)
push!(type_refine_phi, ssaval.id)
end
typ = incoming_val == undef_token ? MaybeUndef(Union{}) : typ_for_val(incoming_val, ci, sptypes, -1, slottypes)
typ = incoming_val === undef_token ? MaybeUndef(Union{}) : typ_for_val(incoming_val, ci, sptypes, -1, slottypes)
old_entry = new_nodes.stmts[ssaval.id]
if isa(typ, DelayedTyp)
push!(type_refine_phi, ssaval.id)
Expand Down
4 changes: 2 additions & 2 deletions base/compiler/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,7 @@ function fieldtype_tfunc(@nospecialize(s0), @nospecialize(name))
if isa(s0, Const) && (!(isa(s0.val, DataType) || isa(s0.val, UnionAll) || isa(s0.val, Union)) || s0.val === Module)
return Bottom
end
if s0 == Type{Module} || s0 == Type{Union{}} || isa(s0, Conditional)
if (s0 isa Type && (s0 == Type{Module} || s0 == Type{Union{}})) || isa(s0, Conditional)
return Bottom
end

Expand Down Expand Up @@ -1458,7 +1458,7 @@ function intrinsic_nothrow(f::IntrinsicFunction, argtypes::Array{Any, 1})
end
# The remaining intrinsics are math/bits/comparison intrinsics. They work on all
# primitive types of the same type.
isshift = f == shl_int || f == lshr_int || f == ashr_int
isshift = f === shl_int || f === lshr_int || f === ashr_int
argtype1 = widenconst(argtypes[1])
isprimitivetype(argtype1) || return false
for i = 2:length(argtypes)
Expand Down
10 changes: 7 additions & 3 deletions base/compiler/typelattice.jl
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ struct NotFound end

const NOT_FOUND = NotFound()

const CompilerTypes = Union{Type, MaybeUndef, Const, Conditional, NotFound, PartialStruct}
==(x::CompilerTypes, y::CompilerTypes) = x === y

#################
# lattice logic #
#################
Expand Down Expand Up @@ -216,7 +219,8 @@ end
widenconst(m::MaybeUndef) = widenconst(m.typ)
widenconst(c::PartialTypeVar) = TypeVar
widenconst(t::PartialStruct) = t.typ
widenconst(@nospecialize(t)) = t
widenconst(t::Type) = t
widenconst(t::TypeVar) = t

issubstate(a::VarState, b::VarState) = (a.typ b.typ && a.undef <= b.undef)

Expand All @@ -234,9 +238,9 @@ end

widenconditional(@nospecialize typ) = typ
function widenconditional(typ::Conditional)
if typ.vtype == Union{}
if typ.vtype === Union{}
return Const(false)
elseif typ.elsetype == Union{}
elseif typ.elsetype === Union{}
return Const(true)
else
return Bool
Expand Down
5 changes: 2 additions & 3 deletions base/compiler/typelimits.jl
Original file line number Diff line number Diff line change
Expand Up @@ -350,12 +350,11 @@ function tmerge(@nospecialize(typea), @nospecialize(typeb))
end
# no special type-inference lattice, join the types
typea, typeb = widenconst(typea), widenconst(typeb)
typea == typeb && return typea
if !(isa(typea, Type) || isa(typea, TypeVar)) ||
!(isa(typeb, Type) || isa(typeb, TypeVar))
if !isa(typea, Type) || !isa(typeb, Type)
# XXX: this should never happen
return Any
end
typea == typeb && return typea
# it's always ok to form a Union of two concrete types
if (isconcretetype(typea) || isType(typea)) && (isconcretetype(typeb) || isType(typeb))
return Union{typea, typeb}
Expand Down
33 changes: 15 additions & 18 deletions base/namedtuple.jl
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,12 @@ end

function NamedTuple{names}(nt::NamedTuple) where {names}
if @generated
types = Tuple{(fieldtype(nt, n) for n in names)...}
Expr(:new, :(NamedTuple{names, $types}), Any[ :(getfield(nt, $(QuoteNode(n)))) for n in names ]...)
idx = Int[ fieldindex(nt, names[n]) for n in 1:length(names) ]
types = Tuple{(fieldtype(nt, idx[n]) for n in 1:length(idx))...}
Expr(:new, :(NamedTuple{names, $types}), Any[ :(getfield(nt, $(idx[n]))) for n in 1:length(idx) ]...)
else
types = Tuple{(fieldtype(typeof(nt), n) for n in names)...}
NamedTuple{names, types}(Tuple(getfield(nt, n) for n in names))
types = Tuple{(fieldtype(typeof(nt), names[n]) for n in 1:length(names))...}
NamedTuple{names, types}(Tuple(getfield(nt, n) for n in 1:length(names)))
end
end

Expand Down Expand Up @@ -160,7 +161,7 @@ function show(io::IO, t::NamedTuple)
end
end

eltype(::Type{NamedTuple{names,T}}) where {names,T} = eltype(T)
eltype(::Type{NamedTuple{names,T}} where names) where {T} = eltype(T)

==(a::NamedTuple{n}, b::NamedTuple{n}) where {n} = Tuple(a) == Tuple(b)
==(a::NamedTuple, b::NamedTuple) = false
Expand All @@ -187,15 +188,8 @@ function map(f, nt::NamedTuple{names}, nts::NamedTuple...) where names
NamedTuple{names}(map(f, map(Tuple, (nt, nts...))...))
end

# a version of `in` for the older world these generated functions run in
@pure function sym_in(x::Symbol, itr::Tuple{Vararg{Symbol}})
for y in itr
y === x && return true
end
return false
end

@pure function merge_names(an::Tuple{Vararg{Symbol}}, bn::Tuple{Vararg{Symbol}})
@nospecialize an bn
names = Symbol[an...]
for n in bn
if !sym_in(n, an)
Expand All @@ -206,8 +200,9 @@ end
end

@pure function merge_types(names::Tuple{Vararg{Symbol}}, a::Type{<:NamedTuple}, b::Type{<:NamedTuple})
@nospecialize names a b
bn = _nt_names(b)
Tuple{Any[ fieldtype(sym_in(n, bn) ? b : a, n) for n in names ]...}
return Tuple{Any[ fieldtype(sym_in(names[n], bn) ? b : a, names[n]) for n in 1:length(names) ]...}
end

"""
Expand Down Expand Up @@ -239,7 +234,7 @@ function merge(a::NamedTuple{an}, b::NamedTuple{bn}) where {an, bn}
if @generated
names = merge_names(an, bn)
types = merge_types(names, a, b)
vals = Any[ :(getfield($(sym_in(n, bn) ? :b : :a), $(QuoteNode(n)))) for n in names ]
vals = Any[ :(getfield($(sym_in(names[n], bn) ? :b : :a), $(QuoteNode(names[n])))) for n in 1:length(names) ]
:( NamedTuple{$names,$types}(($(vals...),)) )
else
names = merge_names(an, bn)
Expand Down Expand Up @@ -294,6 +289,7 @@ tail(t::NamedTuple{names}) where names = NamedTuple{tail(names)}(t)
front(t::NamedTuple{names}) where names = NamedTuple{front(names)}(t)

@pure function diff_names(an::Tuple{Vararg{Symbol}}, bn::Tuple{Vararg{Symbol}})
@nospecialize an bn
names = Symbol[]
for n in an
if !sym_in(n, bn)
Expand All @@ -312,12 +308,13 @@ Construct a copy of named tuple `a`, except with fields that exist in `b` remove
function structdiff(a::NamedTuple{an}, b::Union{NamedTuple{bn}, Type{NamedTuple{bn}}}) where {an, bn}
if @generated
names = diff_names(an, bn)
types = Tuple{Any[ fieldtype(a, n) for n in names ]...}
vals = Any[ :(getfield(a, $(QuoteNode(n)))) for n in names ]
idx = Int[ fieldindex(a, names[n]) for n in 1:length(names) ]
types = Tuple{Any[ fieldtype(a, idx[n]) for n in 1:length(idx) ]...}
vals = Any[ :(getfield(a, $(idx[n]))) for n in 1:length(idx) ]
:( NamedTuple{$names,$types}(($(vals...),)) )
else
names = diff_names(an, bn)
types = Tuple{Any[ fieldtype(typeof(a), n) for n in names ]...}
types = Tuple{Any[ fieldtype(typeof(a), names[n]) for n in 1:length(names) ]...}
NamedTuple{names,types}(map(n->getfield(a, n), names))
end
end
Expand Down
2 changes: 1 addition & 1 deletion base/operators.jl
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ handle comparison to other types via promotion rules where possible.
[`Dict`](@ref) type to compare keys. If your type will be used as a dictionary key, it
should therefore also implement [`hash`](@ref).
"""
==(x, y) = x === y
==

"""
isequal(x, y)
Expand Down
2 changes: 1 addition & 1 deletion base/reducedim.jl
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ Extract first entry of slices of array A into existing array R.
"""
copyfirst!(R::AbstractArray, A::AbstractArray) = mapfirst!(identity, R, A)

function mapfirst!(f, R::AbstractArray, A::AbstractArray{<:Any,N}) where {N}
function mapfirst!(f::F, R::AbstractArray, A::AbstractArray{<:Any,N}) where {N, F}
lsiz = check_reducedims(R, A)
t = _firstreducedslice(axes(R), axes(A))
map!(f, R, view(A, t...))
Expand Down
4 changes: 2 additions & 2 deletions base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ Return a boolean indicating whether `T` has `name` as one of its own fields.
!!! compat "Julia 1.2"
This function requires at least Julia 1.2.
"""
function hasfield(::Type{T}, name::Symbol) where T
function hasfield(T::Type, name::Symbol)
@_pure_meta
return fieldindex(T, name, false) > 0
end
Expand Down Expand Up @@ -339,7 +339,7 @@ function datatype_alignment(dt::DataType)
end

# amount of total space taken by T when stored in a container
function aligned_sizeof(T)
function aligned_sizeof(T::Type)
@_pure_meta
if isbitsunion(T)
sz = Ref{Csize_t}(0)
Expand Down
17 changes: 17 additions & 0 deletions base/tuple.jl
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ end
eltype(t::Type{<:Tuple}) = _compute_eltype(t)
function _compute_eltype(t::Type{<:Tuple})
@_pure_meta
@nospecialize t
t isa Union && return promote_typejoin(eltype(t.a), eltype(t.b))
= unwrap_unionall(t)
r = Union{}
Expand Down Expand Up @@ -420,6 +421,22 @@ function _tuple_any(f::Function, tf::Bool, a, b...)
end
_tuple_any(f::Function, tf::Bool) = tf


# a version of `in` esp. for NamedTuple, to make it pure, and not compiled for each tuple length
function sym_in(x::Symbol, itr::Tuple{Vararg{Symbol}})
@nospecialize itr
@_pure_meta
for y in itr
y === x && return true
end
return false
end
function in(x::Symbol, itr::Tuple{Vararg{Symbol}})
@nospecialize itr
return sym_in(x, itr)
end


"""
empty(x::Tuple)
Expand Down

0 comments on commit dcc0696

Please sign in to comment.