From 530d3cd51e974be86b6e54cd9db4e38e63d61e43 Mon Sep 17 00:00:00 2001 From: Mauro Werder Date: Wed, 18 Jul 2018 09:05:08 +0200 Subject: [PATCH 1/2] Converted WeakKeyDict to hash & compare with object-id --- base/abstractdict.jl | 2 +- base/weakkeydict.jl | 43 ++++++++++++++++++++++++++----------------- test/dict.jl | 15 +++++++++++++++ 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/base/abstractdict.jl b/base/abstractdict.jl index c71a98241b719..f9ef7effec9b3 100644 --- a/base/abstractdict.jl +++ b/base/abstractdict.jl @@ -467,7 +467,7 @@ end function ==(l::AbstractDict, r::AbstractDict) l === r && return true - if isa(l,IdDict) != isa(r,IdDict) + if isa(l,IdDict) != isa(r,IdDict) || isa(l,WeakKeyDict) != isa(r,WeakKeyDict) return false end length(l) != length(r) && return false diff --git a/base/weakkeydict.jl b/base/weakkeydict.jl index 32470135045de..270aac865a83e 100644 --- a/base/weakkeydict.jl +++ b/base/weakkeydict.jl @@ -2,6 +2,15 @@ # weak key dictionaries +# Type to wrap a WeakRef to furbish it with objectid comparison and hashing. +struct WeakRefForWeakDict + w::WeakRef + WeakRefForWeakDict(wr::WeakRef) = new(wr) +end +WeakRefForWeakDict(val) = WeakRefForWeakDict(WeakRef(val)) +==(wr1::WeakRefForWeakDict, wr2::WeakRefForWeakDict) = wr1.w.value===wr2.w.value +hash(wr::WeakRefForWeakDict, h::UInt) = hash_uint(3h - objectid(wr.w.value)) + """ WeakKeyDict([itr]) @@ -12,13 +21,13 @@ referenced in a hash table. See [`Dict`](@ref) for further help. """ mutable struct WeakKeyDict{K,V} <: AbstractDict{K,V} - ht::Dict{WeakRef,V} + ht::Dict{WeakRefForWeakDict,V} lock::Threads.RecursiveSpinLock finalizer::Function # Constructors mirror Dict's function WeakKeyDict{K,V}() where V where K - t = new(Dict{Any,V}(), Threads.RecursiveSpinLock(), identity) + t = new(Dict{WeakRefForWeakDict,V}(), Threads.RecursiveSpinLock(), identity) t.finalizer = function (k) # when a weak key is finalized, remove from dictionary if it is still there if islocked(t) @@ -75,32 +84,32 @@ lock(f, wkh::WeakKeyDict) = lock(f, wkh.lock) trylock(f, wkh::WeakKeyDict) = trylock(f, wkh.lock) function setindex!(wkh::WeakKeyDict{K}, v, key) where K - k = convert(K, key) - finalizer(wkh.finalizer, k) + !isa(key, K) && throw(ArgumentError("$key is not a valid key for type $K")) + finalizer(wkh.finalizer, key) lock(wkh) do - wkh.ht[WeakRef(k)] = v + wkh.ht[WeakRefForWeakDict(key)] = v end return wkh end function getkey(wkh::WeakKeyDict{K}, kk, default) where K return lock(wkh) do - k = getkey(wkh.ht, kk, secret_table_token) + k = getkey(wkh.ht, WeakRefForWeakDict(kk), secret_table_token) k === secret_table_token && return default - return k.value::K + return k.w.value::K end end -get(wkh::WeakKeyDict{K}, key, default) where {K} = lock(() -> get(wkh.ht, key, default), wkh) -get(default::Callable, wkh::WeakKeyDict{K}, key) where {K} = lock(() -> get(default, wkh.ht, key), wkh) -get!(wkh::WeakKeyDict{K}, key, default) where {K} = lock(() -> get!(wkh.ht, key, default), wkh) -get!(default::Callable, wkh::WeakKeyDict{K}, key) where {K} = lock(() -> get!(default, wkh.ht, key), wkh) -pop!(wkh::WeakKeyDict{K}, key) where {K} = lock(() -> pop!(wkh.ht, key), wkh) -pop!(wkh::WeakKeyDict{K}, key, default) where {K} = lock(() -> pop!(wkh.ht, key, default), wkh) -delete!(wkh::WeakKeyDict, key) = lock(() -> delete!(wkh.ht, key), wkh) +get(wkh::WeakKeyDict{K}, key, default) where {K} = lock(() -> get(wkh.ht, WeakRefForWeakDict(key), default), wkh) +get(default::Callable, wkh::WeakKeyDict{K}, key) where {K} = lock(() -> get(default, wkh.ht, WeakRefForWeakDict(key)), wkh) +get!(wkh::WeakKeyDict{K}, key, default) where {K} = lock(() -> get!(wkh.ht, WeakRefForWeakDict(key), default), wkh) +get!(default::Callable, wkh::WeakKeyDict{K}, key) where {K} = lock(() -> get!(default, wkh.ht, WeakRefForWeakDict(key)), wkh) +pop!(wkh::WeakKeyDict{K}, key) where {K} = lock(() -> pop!(wkh.ht, WeakRefForWeakDict(key)), wkh) +pop!(wkh::WeakKeyDict{K}, key, default) where {K} = lock(() -> pop!(wkh.ht, WeakRefForWeakDict(key), default), wkh) +delete!(wkh::WeakKeyDict, key) = lock(() -> delete!(wkh.ht, WeakRefForWeakDict(key)), wkh) empty!(wkh::WeakKeyDict) = (lock(() -> empty!(wkh.ht), wkh); wkh) -haskey(wkh::WeakKeyDict{K}, key) where {K} = lock(() -> haskey(wkh.ht, key), wkh) -getindex(wkh::WeakKeyDict{K}, key) where {K} = lock(() -> getindex(wkh.ht, key), wkh) +haskey(wkh::WeakKeyDict{K}, key) where {K} = lock(() -> haskey(wkh.ht, WeakRefForWeakDict(key)), wkh) +getindex(wkh::WeakKeyDict{K}, key) where {K} = lock(() -> getindex(wkh.ht, WeakRefForWeakDict(key)), wkh) isempty(wkh::WeakKeyDict) = isempty(wkh.ht) length(t::WeakKeyDict) = length(t.ht) @@ -120,7 +129,7 @@ function iterate(t::WeakKeyDict{K,V}, state) where V where K y = iterate(t.ht, tail(state)...) y === nothing && return nothing wkv, i = y - kv = Pair{K,V}(wkv[1].value::K, wkv[2]) + kv = Pair{K,V}(wkv[1].w.value::K, wkv[2]) return (kv, (gc_token, i)) end diff --git a/test/dict.jl b/test/dict.jl index ec5dabab136e4..ef971bb4dad58 100644 --- a/test/dict.jl +++ b/test/dict.jl @@ -830,6 +830,21 @@ Dict(1 => rand(2,3), 'c' => "asdf") # just make sure this does not trigger a dep @test_throws ArgumentError WeakKeyDict([1, 2, 3]) + # WeakKeyDict does not convert keys + @test_throws ArgumentError WeakKeyDict{Int,Any}(5.0=>1) + + # WeakKeyDict hashes with object-id + AA = copy(A) + wkd = WeakKeyDict(A=>1, AA=>2) + @test length(wkd)==2 + kk = collect(keys(wkd)) + @test kk[1]==kk[2] + @test kk[1]!==kk[2] + + # WeakKeyDict compares false to non-WeakKeyDict + @test IdDict(A=>1)!=WeakKeyDict(A=>1) + @test Dict(A=>1)!=WeakKeyDict(A=>1) + # issue #26939 d26939 = WeakKeyDict() d26939[big"1.0" + 1.1] = 1 From 92090fe892c58961a5a5f2a535cdcda585779e45 Mon Sep 17 00:00:00 2001 From: Mauro Werder Date: Wed, 18 Jul 2018 17:08:13 +0200 Subject: [PATCH 2/2] Fix by Jameson --- test/dict.jl | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/dict.jl b/test/dict.jl index ef971bb4dad58..f55bad721c3b8 100644 --- a/test/dict.jl +++ b/test/dict.jl @@ -835,11 +835,13 @@ Dict(1 => rand(2,3), 'c' => "asdf") # just make sure this does not trigger a dep # WeakKeyDict hashes with object-id AA = copy(A) - wkd = WeakKeyDict(A=>1, AA=>2) - @test length(wkd)==2 - kk = collect(keys(wkd)) - @test kk[1]==kk[2] - @test kk[1]!==kk[2] + GC.@preserve A AA begin + wkd = WeakKeyDict(A=>1, AA=>2) + @test length(wkd)==2 + kk = collect(keys(wkd)) + @test kk[1]==kk[2] + @test kk[1]!==kk[2] + end # WeakKeyDict compares false to non-WeakKeyDict @test IdDict(A=>1)!=WeakKeyDict(A=>1)