attempt some code quality improvements

vtjnash · vtjnash · commit d711a413032b · 2020-07-09T12:08:00.000-04:00
remove unused type parameters from some functions, add some useful ones
to others, and remove the fallback `==` method in Core.Compiler.
diff --git 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
@@ -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
@@ -1314,7 +1314,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
 
