WIP: deprecate iteration on bare Associative.

Users should use `pairs(dict)` or `values(dict)` (or `keys(dict)`) to
disambiguate iteration result.

Author: Andy Ferris

NEWS.md

@@ -738,6 +738,10 @@ Deprecated or removed
   * The `sum_kbn` and `cumsum_kbn` functions have been moved to the
     [KahanSummation](https://github.com/JuliaMath/KahanSummation.jl) package ([#24869]).
 
+  * Iteration (`next`, `eltype`, `in`, `map`, etc) on `Associative`s has been deprecated.
+    Explicitly nominate `pairs(dict)`, `values(dict)` or `keys(dict)` as appropriate
+    ([#25013]).
+
 Command-line option changes
 ---------------------------
 

base/associative.jl

@@ -16,20 +16,6 @@ const secret_table_token = :__c782dbf1cf4d6a2e5e3865d7e95634f2e09b5902__
 
 haskey(d::Associative, k) = in(k, keys(d))
 
-function in(p::Pair, a::Associative, valcmp=(==))
-    v = get(a,p[1],secret_table_token)
-    if v !== secret_table_token
-        valcmp(v, p[2]) && return true
-    end
-    return false
-end
-
-function in(p, a::Associative)
-    error("""Associative collections only contain Pairs;
-             Either look for e.g. A=>B instead, or use the `keys` or `values`
-             function if you are looking for a key or value respectively.""")
-end
-
 function summary(t::Associative)
     n = length(t)
     return string(typeof(t), " with ", n, (n==1 ? " entry" : " entries"))
@@ -44,26 +30,38 @@ struct ValueIterator{T<:Associative}
     dict::T
 end
 
-summary(iter::T) where {T<:Union{KeySet,ValueIterator}} =
+struct PairIterator{T<:Associative}
+    dict::T
+end
+
+function in(p::Pair, a::PairIterator, valcmp=(==))
+    v = get(a,p[1],secret_table_token)
+    if v !== secret_table_token
+        valcmp(v, p[2]) && return true
+    end
+    return false
+end
+
+summary(iter::T) where {T<:Union{KeySet,ValueIterator,PairIterator}} =
     string(T.name, " for a ", summary(iter.dict))
 
-show(io::IO, iter::Union{KeySet,ValueIterator}) = show(io, collect(iter))
+show(io::IO, iter::Union{KeySet,ValueIterator,PairIterator}) = show(io, collect(iter))
 
-length(v::Union{KeySet,ValueIterator}) = length(v.dict)
-isempty(v::Union{KeySet,ValueIterator}) = isempty(v.dict)
-_tt2(::Type{Pair{A,B}}) where {A,B} = B
-eltype(::Type{ValueIterator{D}}) where {D} = _tt2(eltype(D))
+length(v::Union{KeySet,ValueIterator,PairIterator}) = length(v.dict)
+isempty(v::Union{KeySet,ValueIterator,PairIterator}) = isempty(v.dict)
+eltype(::Type{ValueIterator{D}}) where {D} = valuetype(D)
+eltype(::Type{PairIterator{D}}) where {D} = pairtype(D)
 
-start(v::Union{KeySet,ValueIterator}) = start(v.dict)
-done(v::Union{KeySet,ValueIterator}, state) = done(v.dict, state)
+start(v::Union{KeySet,ValueIterator,PairIterator}) = start(v.dict)
+done(v::Union{KeySet,ValueIterator,PairIterator}, state) = done(v.dict, state)
 
 function next(v::KeySet, state)
-    n = next(v.dict, state)
+    n = next(PairIterator(v.dict), state)
     n[1][1], n[2]
 end
 
 function next(v::ValueIterator, state)
-    n = next(v.dict, state)
+    n = next(PairIterator(v.dict), state)
     n[1][2], n[2]
 end
 
@@ -136,7 +134,12 @@ This includes arrays, where the keys are the array indices.
 """
 pairs(collection) = Generator(=>, keys(collection), values(collection))
 
-pairs(a::Associative) = a
+pairs(a::Associative) = PairIterator(a)
+
+function next(a::Associative, i)
+    stacktrace()
+    next(PairIterator(a), i)
+end
 
 """
     empty(a::Associative, [index_type=keytype(a)], [value_type=valtype(a)])
@@ -155,7 +158,7 @@ empty(a::Associative, ::Type{V}) where {V} = empty(a, keytype(a), V) # Note: thi
 
 function copy(a::Associative)
     b = empty(a)
-    for (k,v) in a
+    for (k,v) in pairs(a)
         b[k] = v
     end
     return b
@@ -184,7 +187,7 @@ Dict{Int64,Int64} with 3 entries:
 """
 function merge!(d::Associative, others::Associative...)
     for other in others
-        for (k,v) in other
+        for (k,v) in pairs(other)
             d[k] = v
         end
     end
@@ -223,7 +226,7 @@ Dict{Int64,Int64} with 3 entries:
 """
 function merge!(combine::Function, d::Associative, others::Associative...)
     for other in others
-        for (k,v) in other
+        for (k,v) in pairs(other)
             d[k] = haskey(d, k) ? combine(d[k], v) : v
         end
     end
@@ -250,9 +253,9 @@ julia> keytype(Dict(Int32(1) => "foo"))
 Int32
 ```
 """
-keytype(::Type{Associative{K,V}}) where {K,V} = K
+keytype(::Type{Associative{K, V}}) where {K, V} = K
 keytype(a::Associative) = keytype(typeof(a))
-keytype(::Type{A}) where {A<:Associative} = keytype(supertype(A))
+keytype(::Type{A}) where {A <: Associative} = keytype(supertype(A))
 
 """
     valtype(type)
@@ -265,10 +268,25 @@ julia> valtype(Dict(Int32(1) => "foo"))
 String
 ```
 """
-valtype(::Type{Associative{K,V}}) where {K,V} = V
-valtype(::Type{A}) where {A<:Associative} = valtype(supertype(A))
+valtype(::Type{Associative{K, V}}) where {K, V} = V
+valtype(::Type{A}) where {A <: Associative} = valtype(supertype(A))
 valtype(a::Associative) = valtype(typeof(a))
 
+"""
+    pairtype(type)
+
+Get the `Pair` type of an associative collection type. Behaves similarly to [`eltype`](@ref).
+
+# Examples
+```jldoctest
+julia> pairtype(Dict(Int32(1) => "foo"))
+Pair{Int32,String}
+```
+"""
+pairtype(::Type{Associative{K, V}}) where {K, V} = Pair{K, V}
+pairtype(::Type{A}) where {A <: Associative} = pairtype(supertype(A))
+pairtype(a::Associative) = pairtype(typeof(a))
+
 """
     merge(d::Associative, others::Associative...)
 
@@ -368,7 +386,7 @@ Dict{Int64,String} with 2 entries:
 function filter!(f, d::Associative)
     badkeys = Vector{keytype(d)}()
     try
-        for pair in d
+        for pair in pairs(d)
             # don't delete!(d, k) here, since associative types
             # may not support mutation during iteration
             f(pair) || push!(badkeys, pair.first)
@@ -384,7 +402,7 @@ end
 
 function filter_in_one_pass!(f, d::Associative)
     try
-        for pair in d
+        for pair in pairs(d)
             if !f(pair)
                 delete!(d, pair.first)
             end
@@ -399,7 +417,7 @@ function filter!_dict_deprecation(e, f, d::Associative)
     if isa(e, MethodError) && e.f === f
         depwarn("In `filter!(f, dict)`, `f` is now passed a single pair instead of two arguments.", :filter!)
         badkeys = Vector{keytype(d)}()
-        for (k,v) in d
+        for (k,v) in pairs(d)
             # don't delete!(d, k) here, since associative types
             # may not support mutation during iteration
             f(k, v) || push!(badkeys, k)
@@ -435,15 +453,15 @@ function filter(f, d::Associative)
     # don't just do filter!(f, copy(d)): avoid making a whole copy of d
     df = empty(d)
     try
-        for pair in d
+        for pair in pairs(d)
             if f(pair)
                 df[pair.first] = pair.second
             end
         end
     catch e
         if isa(e, MethodError) && e.f === f
             depwarn("In `filter(f, dict)`, `f` is now passed a single pair instead of two arguments.", :filter)
-            for (k, v) in d
+            for (k, v) in pairs(d)
                 if f(k, v)
                     df[k] = v
                 end
@@ -455,16 +473,14 @@ function filter(f, d::Associative)
     return df
 end
 
-eltype(::Type{Associative{K,V}}) where {K,V} = Pair{K,V}
-
 function isequal(l::Associative, r::Associative)
     l === r && return true
     if isa(l,ObjectIdDict) != isa(r,ObjectIdDict)
         return false
     end
     if length(l) != length(r) return false end
-    for pair in l
-        if !in(pair, r, isequal)
+    for pair in pairs(l)
+        if !in(pair, pairs(r), isequal)
             return false
         end
     end
@@ -477,8 +493,8 @@ function ==(l::Associative, r::Associative)
         return false
     end
     if length(l) != length(r) return false end
-    for pair in l
-        if !in(pair, r, ==)
+    for pair in pairs(l)
+        if !in(pair, pairs(r), ==)
             return false
         end
     end
@@ -488,7 +504,7 @@ end
 const hasha_seed = UInt === UInt64 ? 0x6d35bb51952d5539 : 0x952d5539
 function hash(a::Associative, h::UInt)
     hv = hasha_seed
-    for (k,v) in a
+    for (k,v) in pairs(a)
         hv ⊻= hash(k, hash(v))
     end
     hash(hv, h)
@@ -599,11 +615,11 @@ _oidd_nextind(a, i) = reinterpret(Int,ccall(:jl_eqtable_nextind, Csize_t, (Any,
 
 start(t::ObjectIdDict) = _oidd_nextind(t.ht, 0)
 done(t::ObjectIdDict, i) = (i == -1)
-next(t::ObjectIdDict, i) = (Pair{Any,Any}(t.ht[i+1],t.ht[i+2]), _oidd_nextind(t.ht, i+2))
+next(t::PairIterator{<:ObjectIdDict}, i) = (Pair{Any,Any}(t.dict.ht[i+1],t.dict.ht[i+2]), _oidd_nextind(t.dict.ht, i+2))
 
 function length(d::ObjectIdDict)
     n = 0
-    for pair in d
+    for pair in pairs(d)
         n+=1
     end
     n

base/deprecated.jl

@@ -2186,6 +2186,10 @@ end
 @deprecate_moved sum_kbn "KahanSummation"
 @deprecate_moved cumsum_kbn "KahanSummation"
 
+# PR #25013
+@deprecate eltype(a::Type{Associative{K,V}}) where {K,V} pairtype(a)
+@deprecate in(p::Pair, a::Associative) in(p, PairIterator(a))
+
 # END 0.7 deprecations
 
 # BEGIN 1.0 deprecations

base/dict.jl

@@ -43,7 +43,7 @@ function show(io::IO, t::Associative{K,V}) where V where K
         if !show_circular(io, t)
             first = true
             n = 0
-            for pair in t
+            for pair in pairs(t)
                 first || print(io, ',')
                 first = false
                 show(recur_io, pair)
@@ -139,9 +139,12 @@ Dict(ps::Pair{K}...)             where {K}   = Dict{K,Any}(ps)
 Dict(ps::(Pair{K,V} where K)...) where {V}   = Dict{Any,V}(ps)
 Dict(ps::Pair...)                            = Dict{Any,Any}(ps)
 
+pair_or_eltype(x) = eltype(x)
+pair_or_eltype(x::Associative) = pairtype(x)
+
 function Dict(kv)
     try
-        associative_with_eltype((K, V) -> Dict{K, V}, kv, eltype(kv))
+        associative_with_eltype((K, V) -> Dict{K, V}, kv, pair_or_eltype(kv))
     catch e
         if !applicable(start, kv) || !all(x->isa(x,Union{Tuple,Pair}),kv)
             throw(ArgumentError("Dict(kv): kv needs to be an iterator of tuples or pairs"))
@@ -193,7 +196,7 @@ empty(a::Associative, ::Type{K}, ::Type{V}) where {K, V} = Dict{K, V}()
 # conversion between Dict types
 function convert(::Type{Dict{K,V}},d::Associative) where V where K
     h = Dict{K,V}()
-    for (k,v) in d
+    for (k,v) in pairs(d)
         ck = convert(K,k)
         if !haskey(h,ck)
             h[ck] = convert(V,v)
@@ -711,8 +714,8 @@ function start(t::Dict)
     return i
 end
 done(t::Dict, i) = i > length(t.vals)
-@propagate_inbounds function next(t::Dict{K,V}, i) where {K,V}
-    return (Pair{K,V}(t.keys[i],t.vals[i]), skip_deleted(t,i+1))
+@propagate_inbounds function next(t::PairIterator{Dict{K,V}}, i) where {K,V}
+    return (Pair{K,V}(t.dict.keys[i],t.dict.vals[i]), skip_deleted(t.dict,i+1))
 end
 
 isempty(t::Dict) = (t.count == 0)
@@ -756,11 +759,11 @@ ImmutableDict
 ImmutableDict(KV::Pair{K,V}) where {K,V} = ImmutableDict{K,V}(KV[1], KV[2])
 ImmutableDict(t::ImmutableDict{K,V}, KV::Pair) where {K,V} = ImmutableDict{K,V}(t, KV[1], KV[2])
 
-function in(key_value::Pair, dict::ImmutableDict, valcmp=(==))
+function in(key_value::Pair, dict::PairIterator{<:ImmutableDict}, valcmp=(==))
     key, value = key_value
     while isdefined(dict, :parent)
-        if dict.key == key
-            valcmp(value, dict.value) && return true
+        if dict.dict.key == key
+            valcmp(value, dict.dict.value) && return true
         end
         dict = dict.parent
     end
@@ -792,7 +795,7 @@ end
 
 # this actually defines reverse iteration (e.g. it should not be used for merge/copy/filter type operations)
 start(t::ImmutableDict) = t
-next(::ImmutableDict{K,V}, t) where {K,V} = (Pair{K,V}(t.key, t.value), t.parent)
+next(::PairIterator{ImmutableDict{K,V}}, t) where {K,V} = (Pair{K,V}(t.key, t.value), t.parent)
 done(::ImmutableDict, t) = !isdefined(t, :parent)
 length(t::ImmutableDict) = count(x->true, t)
 isempty(t::ImmutableDict) = done(t, start(t))

base/env.jl

@@ -81,7 +81,7 @@ pop!(::EnvDict, k::AbstractString) = (v = ENV[k]; _unsetenv(k); v)
 pop!(::EnvDict, k::AbstractString, def) = haskey(ENV,k) ? pop!(ENV,k) : def
 delete!(::EnvDict, k::AbstractString) = (_unsetenv(k); ENV)
 setindex!(::EnvDict, v, k::AbstractString) = _setenv(k,string(v))
-push!(::EnvDict, k::AbstractString, v) = setindex!(ENV, v, k)
+push!(::EnvDict, k::AbstractString, v) = setindex!(ENV, v, k) # Should this be a `Pair`?
 
 if Sys.iswindows()
     start(hash::EnvDict) = (pos = ccall(:GetEnvironmentStringsW,stdcall,Ptr{UInt16},()); (pos,pos))
@@ -92,7 +92,7 @@ if Sys.iswindows()
         end
         return false
     end
-    function next(hash::EnvDict, block::Tuple{Ptr{UInt16},Ptr{UInt16}})
+    function next(hash::PairIterator{EnvDict}, block::Tuple{Ptr{UInt16},Ptr{UInt16}})
         pos = block[1]
         blk = block[2]
         len = ccall(:wcslen, UInt, (Ptr{UInt16},), pos)
@@ -109,7 +109,7 @@ else # !windows
     start(::EnvDict) = 0
     done(::EnvDict, i) = (ccall(:jl_environ, Any, (Int32,), i) === nothing)
 
-    function next(::EnvDict, i)
+    function next(::PairIterator{EnvDict}, i)
         env = ccall(:jl_environ, Any, (Int32,), i)
         if env === nothing
             throw(BoundsError())

base/exports.jl

@@ -82,6 +82,7 @@ export
     ObjectIdDict,
     OrdinalRange,
     Pair,
+    PairIterator,
     PartialQuickSort,
     PermutedDimsArray,
     QuickSort,

base/inference.jl

@@ -5686,7 +5686,7 @@ function remove_redundant_temp_vars!(src::CodeInfo, nargs::Int, sa::ObjectIdDict
     slottypes = src.slottypes
     ssavaluetypes = src.ssavaluetypes
     repls = ObjectIdDict()
-    for (v, init) in sa
+    for (v, init) in pairs(sa)
         repl = get_replacement(sa, v, init, nargs, slottypes, ssavaluetypes)
         compare = isa(repl, TypedSlot) ? normslot(repl) : repl
         if compare !== v