From dcc0696971a100dbc63f90b8789632631dd1bfef Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Thu, 2 Jul 2020 23:43:54 -0400 Subject: [PATCH] attempt some code quality improvements remove unused type parameters from some functions, add some useful ones to others, and remove the fallback `==` method in Core.Compiler. --- base/Base.jl | 1 + base/array.jl | 2 +- base/compiler/abstractinterpretation.jl | 2 +- base/compiler/compiler.jl | 1 + base/compiler/inferencestate.jl | 5 +++- base/compiler/ssair/inlining.jl | 4 +-- base/compiler/ssair/passes.jl | 2 +- base/compiler/ssair/slot2ssa.jl | 6 ++--- base/compiler/tfuncs.jl | 4 +-- base/compiler/typelattice.jl | 10 +++++--- base/compiler/typelimits.jl | 5 ++-- base/namedtuple.jl | 33 +++++++++++-------------- base/operators.jl | 2 +- base/reducedim.jl | 2 +- base/reflection.jl | 4 +-- base/tuple.jl | 17 +++++++++++++ 16 files changed, 61 insertions(+), 39 deletions(-) diff --git a/base/Base.jl b/base/Base.jl index 750df94ec650f..5cb0d558c228d 100644 --- a/base/Base.jl +++ b/base/Base.jl @@ -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") diff --git a/base/array.jl b/base/array.jl index 1035cd38b15f3..f8de513cc60e8 100644 --- a/base/array.jl +++ b/base/array.jl @@ -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}) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 248a55baf370b..5ae29b2ce7be4 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -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) diff --git a/base/compiler/compiler.jl b/base/compiler/compiler.jl index 48a2d1e30adf3..3d3110ee4939b 100644 --- a/base/compiler/compiler.jl +++ b/base/compiler/compiler.jl @@ -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") diff --git a/base/compiler/inferencestate.jl b/base/compiler/inferencestate.jl index 2a699572778f5..0f39f13aa6fb5 100644 --- a/base/compiler/inferencestate.jl +++ b/base/compiler/inferencestate.jl @@ -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] diff --git a/base/compiler/ssair/inlining.jl b/base/compiler/ssair/inlining.jl index 018bf31f69b26..d9865e13b43cf 100644 --- a/base/compiler/ssair/inlining.jl +++ b/base/compiler/ssair/inlining.jl @@ -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 diff --git a/base/compiler/ssair/passes.jl b/base/compiler/ssair/passes.jl index 3895e220eed3a..4b444aa504715 100644 --- a/base/compiler/ssair/passes.jl +++ b/base/compiler/ssair/passes.jl @@ -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. diff --git a/base/compiler/ssair/slot2ssa.jl b/base/compiler/ssair/slot2ssa.jl index aa1a6d7a0c063..a8238f1c0fcb6 100644 --- a/base/compiler/ssair/slot2ssa.jl +++ b/base/compiler/ssair/slot2ssa.jl @@ -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) @@ -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) diff --git a/base/compiler/tfuncs.jl b/base/compiler/tfuncs.jl index 7e187b63094b2..24f1be5db8191 100644 --- a/base/compiler/tfuncs.jl +++ b/base/compiler/tfuncs.jl @@ -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 @@ -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) diff --git a/base/compiler/typelattice.jl b/base/compiler/typelattice.jl index a202f0f313203..2ea65bb749bf0 100644 --- a/base/compiler/typelattice.jl +++ b/base/compiler/typelattice.jl @@ -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 # ################# @@ -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) @@ -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 diff --git a/base/compiler/typelimits.jl b/base/compiler/typelimits.jl index c6adb5aafb1c0..2a20cd3212500 100644 --- a/base/compiler/typelimits.jl +++ b/base/compiler/typelimits.jl @@ -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} diff --git a/base/namedtuple.jl b/base/namedtuple.jl index c154ae9de889a..f8d41d0b21825 100644 --- a/base/namedtuple.jl +++ b/base/namedtuple.jl @@ -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 @@ -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 @@ -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) @@ -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 """ @@ -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) @@ -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) @@ -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 diff --git a/base/operators.jl b/base/operators.jl index c8e44e7c816eb..5402f068ed7e4 100644 --- a/base/operators.jl +++ b/base/operators.jl @@ -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) diff --git a/base/reducedim.jl b/base/reducedim.jl index 8980bd22c9c5e..c41d5fbb62844 100644 --- a/base/reducedim.jl +++ b/base/reducedim.jl @@ -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...)) diff --git a/base/reflection.jl b/base/reflection.jl index 89d88d2f99145..cd548de1d38fd 100644 --- a/base/reflection.jl +++ b/base/reflection.jl @@ -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 @@ -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) diff --git a/base/tuple.jl b/base/tuple.jl index 3a5806dc098c3..72fb42aabb33a 100644 --- a/base/tuple.jl +++ b/base/tuple.jl @@ -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)) t´ = unwrap_unionall(t) r = Union{} @@ -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)