RFC: Split delete!(h::Dict,key) into pop!(), delete!() (#3405)

base/array.jl

@@ -1619,7 +1619,7 @@ function union(vs...)
         for v_elem in v
             if !contains(seen, v_elem)
                 push!(ret, v_elem)
-                add!(seen, v_elem)
+                push!(seen, v_elem)
             end
         end
     end
@@ -1634,7 +1634,7 @@ function setdiff(a, b)
     for a_elem in a
         if !contains(seen, a_elem) && !contains(bset, a_elem)
             push!(ret, a_elem)
-            add!(seen, a_elem)
+            push!(seen, a_elem)
         end
     end
     ret

base/collections.jl

@@ -254,13 +254,12 @@ function dequeue!(pq::PriorityQueue)
 end
 
 function dequeue!(pq::PriorityQueue, key)
-     !haskey(pq.index, key) && throw(KeyError(key))
-     idx = delete!(pq.index, key)
-     splice!(pq.xs, idx)
-     for (k,v) in pq.index
-       (v >= idx) && (pq.index[k] = (v-1))
-     end
-     key
+    idx = pop!(pq.index, key)  # throws key error if missing
+    splice!(pq.xs, idx)
+    for (k,v) in pq.index
+        (v >= idx) && (pq.index[k] = (v-1))
+    end
+    key
 end
 
 

base/deprecated.jl

@@ -33,7 +33,6 @@ end
 @deprecate  insert          insert!
 @deprecate  del             delete!
 @deprecate  del_all         empty!
-@deprecate  add             add!
 @deprecate  add_each        add_each!
 @deprecate  del_each        del_each!
 @deprecate  toggle          symdiff!
@@ -223,6 +222,9 @@ export PipeString
 @deprecate finfer                     code_typed
 @deprecate disassemble(f::Function,t::Tuple)           code_llvm(f,t)
 @deprecate disassemble(f::Function,t::Tuple,asm::Bool) (asm ? code_native(f,t) : code_llvm(f,t))
+@deprecate  add(s::Set, x)                  push!(s,x)
+@deprecate  add!(s::Set, x)                 push!(s,x)
+@deprecate  delete!(d::Dict, key, default)  pop!(d, key, default)
 
 deprecated_ls() = run(`ls -l`)
 deprecated_ls(args::Cmd) = run(`ls -l $args`)

base/dict.jl

@@ -131,6 +131,8 @@ function getindex(t::Associative, key)
     return v
 end
 
+push!(t::Associative, key, v) = setindex!(t, v, key)
+
 # hashing objects by identity
 
 type ObjectIdDict <: Associative{Any,Any}
@@ -148,14 +150,19 @@ end
 get(t::ObjectIdDict, key::ANY, default::ANY) =
     ccall(:jl_eqtable_get, Any, (Any, Any, Any), t.ht, key, default)
 
-delete!(t::ObjectIdDict, key::ANY, default::ANY) =
-    ccall(:jl_eqtable_del, Any, (Any, Any, Any), t.ht, key, default)
+pop!(t::ObjectIdDict, key::ANY, default::ANY) =
+    ccall(:jl_eqtable_pop, Any, (Any, Any, Any), t.ht, key, default)
 
-function delete!(t::ObjectIdDict, key::ANY)
-    val = delete!(t, key, secret_table_token)
+function pop!(t::ObjectIdDict, key::ANY)
+    val = pop!(t, key, secret_table_token)
     !is(val,secret_table_token) ? val : throw(KeyError(key))
 end
 
+function delete!(t::ObjectIdDict, key::ANY)
+    ccall(:jl_eqtable_pop, Any, (Any, Any), t.ht, key)
+    t
+end
+
 empty!(t::ObjectIdDict) = (t.ht = cell(length(t.ht)); t)
 
 start(t::ObjectIdDict) = 0
@@ -502,7 +509,7 @@ function getkey{K,V}(h::Dict{K,V}, key, deflt)
     return (index<0) ? deflt : h.keys[index]::K
 end
 
-function _delete!(h::Dict, index)
+function _pop!(h::Dict, index)
     val = h.vals[index]
     h.slots[index] = 0x2
     ccall(:jl_arrayunset, Void, (Any, Uint), h.keys, index-1)
@@ -512,14 +519,30 @@ function _delete!(h::Dict, index)
     return val
 end
 
-function delete!(h::Dict, key)
+function pop!(h::Dict, key)
     index = ht_keyindex(h, key)
-    index > 0 ? _delete!(h, index) : throw(KeyError(key))
+    index > 0 ? _pop!(h, index) : throw(KeyError(key))
+end
+
+function pop!(h::Dict, key, default)
+    index = ht_keyindex(h, key)
+    index > 0 ? _pop!(h, index) : default
+end
+
+function _delete!(h::Dict, index)
+    val = h.vals[index]
+    h.slots[index] = 0x2
+    ccall(:jl_arrayunset, Void, (Any, Uint), h.keys, index-1)
+    ccall(:jl_arrayunset, Void, (Any, Uint), h.vals, index-1)
+    h.ndel += 1
+    h.count -= 1
+    h
 end
 
-function delete!(h::Dict, key, default)
+function delete!(h::Dict, key)
     index = ht_keyindex(h, key)
-    index > 0 ? _delete!(h, index) : default
+    if index > 0; _delete!(h, index); end
+    h
 end
 
 function skip_deleted(h::Dict, i)
@@ -594,8 +617,9 @@ function getkey{K}(wkh::WeakKeyDict{K}, kk, deflt)
 end
 
 get{K}(wkh::WeakKeyDict{K}, key, def) = get(wkh.ht, key, def)
+pop!{K}(wkh::WeakKeyDict{K}, key) = pop!(wkh.ht, key)
+pop!{K}(wkh::WeakKeyDict{K}, key, def) = pop!(wkh.ht, key, def)
 delete!{K}(wkh::WeakKeyDict{K}, key) = delete!(wkh.ht, key)
-delete!{K}(wkh::WeakKeyDict{K}, key, def) = delete!(wkh.ht, key, def)
 empty!(wkh::WeakKeyDict)  = (empty!(wkh.ht); wkh)
 haskey{K}(wkh::WeakKeyDict{K}, key) = haskey(wkh.ht, key)
 getindex{K}(wkh::WeakKeyDict{K}, key) = getindex(wkh.ht, key)

base/env.jl

@@ -73,9 +73,16 @@ const ENV = EnvHash()
 getindex(::EnvHash, k::String) = @accessEnv k throw(KeyError(k))
 get(::EnvHash, k::String, def) = @accessEnv k (return def)
 contains(::KeyIterator{EnvHash}, k::String) = _hasenv(k)
-delete!(::EnvHash, k::String) = (v = ENV[k]; _unsetenv(k); v)
+pop!(::EnvHash, k::String) = (v = ENV[k]; _unsetenv(k); v)
+pop!(::EnvHash, k::String, def) = haskey(ENV,k) ? pop!(ENV,k) : def
+function delete!(::EnvHash, k::String)
+    warn_once("delete!(h::EnvHash,key) now returns the modified environment.\nUse pop!(h::EnvHash,key) to retrieve the value instead.\n")
+    _unsetenv(k)
+    ENV
+end
 delete!(::EnvHash, k::String, def) = haskey(ENV,k) ? delete!(ENV,k) : def
 setindex!(::EnvHash, v, k::String) = _setenv(k,string(v))
+push!(::EnvHash, k::String, v) = setindex!(ENV, v, k)
 
 @unix_only begin
 start(::EnvHash) = 0

base/exports.jl

@@ -680,7 +680,6 @@ export
     unshift!,
 
 # collections
-    add!,
     all,
     any,
     collect,
@@ -709,6 +708,8 @@ export
     mapreduce,
     merge!,
     merge,
+    #pop!,
+    #push!,
     reduce,
     setdiff!,
     setdiff,

base/git.jl

@@ -166,7 +166,7 @@ function config_sections(cfg::Dict)
     sections = Set{ByteString}()
     for (key,_) in cfg
         m = match(r"^(.+)\.", key)
-        if m != nothing add!(sections,m.captures[1]) end
+        if m != nothing; push!(sections,m.captures[1]); end
     end
     sections
 end
@@ -181,7 +181,7 @@ function merge_configs(Bc::Dict, Lc::Dict, Rc::Dict)
     for section in Bs - Ls & Rs
         filter!((k,v)->!beginswith(k,"$section."),Lc)
         filter!((k,v)->!beginswith(k,"$section."),Rc)
-        add!(deleted, section)
+        push!(deleted, section)
     end
     # merge the remaining config key-value pairs
     cfg = Dict()

base/inference.jl

@@ -1133,7 +1133,7 @@ function typeinf(linfo::LambdaStaticData,atypes::Tuple,sparams::Tuple, def, cop)
     recpts = IntSet()  # statements that depend recursively on our value
     W = IntSet()
     # initial set of pc
-    add!(W,1)
+    push!(W,1)
     # initial types
     s[1] = ObjectIdDict()
     for v in vars
@@ -1191,7 +1191,7 @@ function typeinf(linfo::LambdaStaticData,atypes::Tuple,sparams::Tuple, def, cop)
         pc = first(W)
         while true
             #print(pc,": ",s[pc],"\n")
-            delete!(W, pc, 0)
+            delete!(W, pc)
             if is(handler_at[pc],())
                 handler_at[pc] = cur_hand
             else
@@ -1204,7 +1204,7 @@ function typeinf(linfo::LambdaStaticData,atypes::Tuple,sparams::Tuple, def, cop)
                 if !(isa(frame.prev,CallStack) && frame.prev.cycleid == frame.cycleid)
                     toprec = true
                 end
-                add!(recpts, pc)
+                push!(recpts, pc)
                 #if dbg
                 #    show(pc); print(" recurred\n")
                 #end
@@ -1214,7 +1214,7 @@ function typeinf(linfo::LambdaStaticData,atypes::Tuple,sparams::Tuple, def, cop)
                 # propagate type info to exception handler
                 l = cur_hand[1]::Int
                 if stchanged(changes, s[l], vars)
-                    add!(W, l)
+                    push!(W, l)
                     s[l] = stupdate(s[l], changes, vars)
                 end
             end
@@ -1234,7 +1234,7 @@ function typeinf(linfo::LambdaStaticData,atypes::Tuple,sparams::Tuple, def, cop)
                         # general case
                         handler_at[l] = cur_hand
                         if stchanged(changes, s[l], vars)
-                            add!(W, l)
+                            push!(W, l)
                             s[l] = stupdate(s[l], changes, vars)
                         end
                     end
@@ -1257,7 +1257,7 @@ function typeinf(linfo::LambdaStaticData,atypes::Tuple,sparams::Tuple, def, cop)
                         if ot === NF || !typeseq(vt,ot)
                             # l+1 is the statement after the label, where the
                             # static_typeof occurs.
-                            add!(W, l+1)
+                            push!(W, l+1)
                             s[l+1][var] = vt
                         end
                     end
@@ -1269,7 +1269,7 @@ function typeinf(linfo::LambdaStaticData,atypes::Tuple,sparams::Tuple, def, cop)
                         if !(isa(frame.prev,CallStack) && frame.prev.cycleid == frame.cycleid)
                             toprec = true
                         end
-                        add!(recpts, pc)
+                        push!(recpts, pc)
                         #if dbg
                         #    show(pc); print(" recurred\n")
                         #end
@@ -1290,7 +1290,7 @@ function typeinf(linfo::LambdaStaticData,atypes::Tuple,sparams::Tuple, def, cop)
                             #    show(r)
                             #    print("\n")
                             #end
-                            add!(W,r)
+                            push!(W,r)
                         end
                     end
                 elseif is(hd,:enter)
@@ -2030,7 +2030,7 @@ function find_sa_vars(ast)
     for vi in ast.args[2][2]
         if (vi[3]&1)!=0
             # remove captured vars
-            delete!(av, vi[1], nothing)
+            delete!(av, vi[1])
         end
     end
     av

base/intset.jl

@@ -6,7 +6,7 @@ type IntSet
     IntSet() = new(zeros(Uint32,256>>>5), 256, false)
 end
 
-IntSet(args...) = (s=IntSet(); for a in args; add!(s,a); end; s)
+IntSet(args...) = (s=IntSet(); for a in args; push!(s,a); end; s)
 
 similar(s::IntSet) = IntSet()
 
@@ -28,7 +28,7 @@ function sizehint(s::IntSet, top::Integer)
     s
 end
 
-function add!(s::IntSet, n::Integer)
+function push!(s::IntSet, n::Integer)
     if n >= s.limit
         if s.fill1s
             return s
@@ -43,12 +43,12 @@ end
 
 function union!(s::IntSet, ns)
     for n in ns
-        add!(s, n)
+        push!(s, n)
     end
     return s
 end
 
-function delete!(s::IntSet, n::Integer, deflt)
+function pop!(s::IntSet, n::Integer, deflt)
     if n >= s.limit
         if s.fill1s
             lim = int(n + div(n,2))
@@ -65,16 +65,24 @@ function delete!(s::IntSet, n::Integer, deflt)
     return n
 end
 
-function delete!(s::IntSet, n::Integer)
-    if delete!(s, n, n+1) == n+1
+function pop!(s::IntSet, n::Integer)
+    if pop!(s, n, n+1) == n+1
         throw(KeyError(n))
     end
     return n
 end
 
+# TODO: what should happen when fill1s == true?
+pop!(s::IntSet) = pop!(s, last(s))
+
+function delete!(s::IntSet, n::Integer)
+    pop!(s, n, n)
+    return s
+end
+
 function setdiff!(s::IntSet, ns)
     for n in ns
-        delete!(s, n, nothing)
+        delete!(s, n)
     end
     return s
 end
@@ -141,7 +149,7 @@ function first(s::IntSet)
     return n
 end
 
-shift!(s::IntSet) = delete!(s, first(s))
+shift!(s::IntSet) = pop!(s, first(s))
 
 function last(s::IntSet)
     if !s.fill1s
@@ -155,8 +163,6 @@ function last(s::IntSet)
     error("last: set has no last element")
 end
 
-pop!(s::IntSet) = delete!(s, last(s))
-
 length(s::IntSet) = int(ccall(:bitvector_count, Uint64, (Ptr{Uint32}, Uint64, Uint64), s.bits, 0, s.limit)) +
     (s.fill1s ? typemax(Int) - s.limit : 0)
 

base/loading.jl

@@ -112,7 +112,7 @@ function reload_path(path)
     end
     package_list[path] = time()
     tls = task_local_storage()
-    prev = delete!(tls, :SOURCE_PATH, nothing)
+    prev = pop!(tls, :SOURCE_PATH, nothing)
     try
         eval(Main, :(Base.include_from_node1($path)))
     catch e