From 28900adceb1c95a65606a07f62a178e2b16f3092 Mon Sep 17 00:00:00 2001
From: Takafumi Arakaki <aka.tkf@gmail.com>
Date: Fri, 12 Nov 2021 19:23:27 -0500
Subject: [PATCH] Return `old => new` pair

---
 base/abstractdict.jl | 33 ++++++++++++++++++++-------------
 base/dict.jl         |  8 +++++---
 test/dict.jl         | 10 +++++-----
 3 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/base/abstractdict.jl b/base/abstractdict.jl
index 11730db5a440f..d75aecefc9999 100644
--- a/base/abstractdict.jl
+++ b/base/abstractdict.jl
@@ -528,18 +528,22 @@ function hash(a::AbstractDict, h::UInt)
 end
 
 """
-    modify!(f, d::AbstractDict{K, V}, key)
+    modify!(f, d::AbstractDict{K, V}, key) -> (old => new)
 
 Lookup and then update, insert or delete in one go without re-computing the hash.
+Return a pair of `old::Union{Some,Nothing}` and `new::::Union{Some,Nothing}`.
 
-`f` is a callable object that must take a single `Union{Some{V}, Nothing}` argument and return
-a `Union{T, Some{T}, Nothing}` value, where `T` is a type [`convert`](@ref)-able to the value type
-`V`.  The value `Some(d[key])` is passed to `f` if `haskey(d, key)`; otherwise `nothing`
-is passed.  If `f` returns `nothing`, corresponding entry in the dictionary `d`  is removed.
-If `f` returns non-`nothing` value `x`, `key => something(x)` is inserted or updated in `d`
-(equivalent to `d[key] = something(x)` but more efficient).
+`f` is a callable object that must take a single `old::Union{Some, Nothing}` argument and
+return a `new::Union{T, Some{T}, Nothing}` value, where `T` is a type [`convert`](@ref)-able
+to the value type `V`.  The value `Some(d[key])` is passed to `f` if `haskey(d, key)`;
+otherwise `nothing` is passed.  If `f` returns `nothing`, corresponding entry in the
+dictionary `d`  is removed.  If `f` returns non-`nothing` value `x`, `key => something(x)`
+is inserted or updated in `d` (equivalent to `d[key] = something(x)` but more efficient).
 
-`modify!` returns whatever `f` returns as-is.
+Whether `Some{V}(value)` or `Some{typeof(value)}(value)` is returned is an implementation
+defined behavior. The callback function `f` must use `old === nothing` or `old isa Some`
+instead of `old isa Some{valtype(d)}` unless the type of the dictionary `d` is known to
+define a certain behavior.
 
 # Examples
 ```jldoctest
@@ -548,7 +552,7 @@ julia> dict = Dict("a" => 1);
 julia> modify!(dict, "a") do val
            Some(val === nothing ? 1 : something(val) + 1)
        end
-Some(2)
+Some(1) => Some(2)
 
 julia> dict
 Dict{String,Int64} with 1 entry:
@@ -559,13 +563,14 @@ julia> dict = Dict();
 julia> modify!(dict, "a") do val
            Some(something(val, 0) + 1)
        end
-Some(1)
+nothing => Some(1)
 
 julia> dict
 Dict{Any,Any} with 1 entry:
   "a" => 1
 
 julia> modify!(_ -> nothing, dict, "a")
+Some("a") => nothing
 
 julia> dict
 Dict{Any,Any} with 0 entries
@@ -573,16 +578,18 @@ Dict{Any,Any} with 0 entries
 """
 function modify!(f, dict::AbstractDict{K,V}, key) where {K, V}
     if haskey(dict, key)
-        val = f(Some{V}(dict[key]))
+        old = Some{V}(dict[key])
+        val = f(old)
     else
-        val = f(nothing)
+        old = nothing
+        val = f(old)
     end
     if val === nothing
         delete!(dict, key)
     else
         dict[key] = something(val)
     end
-    return val
+    return old => val
 end
 
 function getindex(t::AbstractDict, key)
diff --git a/base/dict.jl b/base/dict.jl
index 05ca7587df0d6..62dceb19bd4a8 100644
--- a/base/dict.jl
+++ b/base/dict.jl
@@ -407,9 +407,11 @@ function modify!(f, h::Dict{K, V}, key0) where {K, V}
     age0 = h.age
     if idx > 0
         @inbounds vold = h.vals[idx]
-        vnew = f(Some{V}(vold))
+        vold = Some{V}(vold)
+        vnew = f(vold)
     else
-        vnew = f(nothing)
+        vold = nothing
+        vnew = f(vold)
     end
     if h.age != age0
         idx = ht_keyindex2!(h, key)
@@ -428,7 +430,7 @@ function modify!(f, h::Dict{K, V}, key0) where {K, V}
             @inbounds _setindex!(h, something(vnew), key, -idx)
         end
     end
-    return vnew
+    return vold => vnew
 end
 
 """
diff --git a/test/dict.jl b/test/dict.jl
index 4c3996a0ddb67..4115b0df54427 100644
--- a/test/dict.jl
+++ b/test/dict.jl
@@ -1218,7 +1218,7 @@ end
 
         @test modify!(dict, "a") do val
             Some(val === nothing ? 1 : something(val) + 1)
-        end == Some(2)
+        end == (Some(1) => Some(2))
 
         @test Dict(dict) == Dict("a" => 2)
     end
@@ -1228,20 +1228,20 @@ end
 
         @test modify!(dict, "a") do val
             Some(val === nothing ? 1 : something(val) + 1)
-        end == Some(1)
+        end == (nothing => Some(1))
 
         @test Dict(dict) == Dict("a" => 1)
     end
 
     @testset "delete" begin
         dict = constructor(Dict("a" => 1))
-        @test modify!(_ -> nothing, dict, "a") === nothing
+        @test modify!(_ -> nothing, dict, "a") == (Some(1) => nothing)
         @test Dict(dict) == Dict()
     end
 
     @testset "no-op" begin
         dict = constructor(Dict("a" => 1))
-        @test modify!(_ -> nothing, dict, "b") === nothing
+        @test modify!(_ -> nothing, dict, "b") == (nothing => nothing)
         @test Dict(dict) == Dict("a" => 1)
     end
 
@@ -1251,7 +1251,7 @@ end
         @test modify!(dict, "a") do val
             dict["a"] = 0
             Some(val === nothing ? 1 : something(val) + 1)
-        end == Some(1)
+        end == (nothing => Some(1))
 
         @test Dict(dict) == Dict("a" => 1)
     end