From b7b4d0a7850b3c1289acaeaf3fec620a8d2a9e1a Mon Sep 17 00:00:00 2001 From: Curtis Vogt Date: Mon, 12 Apr 2021 11:38:08 -0500 Subject: [PATCH 1/3] Fix duplicate error when using generator in Dict Fixes: #33147 Replaces/Closes: #40445 Co-authored-by: Jameson Nash --- base/abstractdict.jl | 49 ++++++++++++++++++++++++++++++++++++++++++++ base/dict.jl | 40 +----------------------------------- base/iddict.jl | 13 +----------- base/weakkeydict.jl | 12 +---------- test/dict.jl | 24 ++++++++++++++++++++++ 5 files changed, 76 insertions(+), 62 deletions(-) diff --git a/base/abstractdict.jl b/base/abstractdict.jl index 5ca485610f053..915ba21e51615 100644 --- a/base/abstractdict.jl +++ b/base/abstractdict.jl @@ -579,6 +579,55 @@ _tablesz(x::T) where T <: Integer = x < 16 ? T(16) : one(T)<<(top_set_bit(x-one( TP{K,V} = Union{Type{Tuple{K,V}},Type{Pair{K,V}}} +# validate the contents of this iterator by testing the iteration protocol (isiterable) on it +_throw_dict_kv_error() = throw(ArgumentError("AbstractDict(kv): kv needs to be an iterator of 2-tuples or pairs")) + +function grow_to!(dest::AbstractDict, itr) + applicable(iterate, itr) || _throw_dict_kv_error() + y = iterate(itr) + y === nothing && return dest + kv, st = y + applicable(iterate, kv) || _throw_dict_kv_error() + k = iterate(kv) + k === nothing && _throw_dict_kv_error() + k, kvst = k + v = iterate(kv, kvst) + v === nothing && _throw_dict_kv_error() + v, kvst = v + iterate(kv, kvst) === nothing || _throw_dict_kv_error() + if !(dest isa AbstractDict{typeof(k), typeof(v)}) + dest = empty(dest, typeof(k), typeof(v)) + end + dest[k] = v + return grow_to!(dest, itr, st) +end + +function grow_to!(dest::AbstractDict{K,V}, itr, st) where {K, V} + y = iterate(itr, st) + while y !== nothing + kv, st = y + applicable(iterate, kv) || _throw_dict_kv_error() + kst = iterate(kv) + kst === nothing && _throw_dict_kv_error() + k, kvst = kst + vst = iterate(kv, kvst) + vst === nothing && _throw_dict_kv_error() + v, kvst = vst + iterate(kv, kvst) === nothing || _throw_dict_kv_error() + if isa(k, K) && isa(v, V) + dest[k] = v + else + new = empty(dest, promote_typejoin(K, typeof(k)), promote_typejoin(V, typeof(v))) + merge!(new, dest) + new[k] = v + return grow_to!(new, itr, st) + end + y = iterate(itr, st) + end + return dest +end + + dict_with_eltype(DT_apply, kv, ::TP{K,V}) where {K,V} = DT_apply(K, V)(kv) dict_with_eltype(DT_apply, kv::Generator, ::TP{K,V}) where {K,V} = DT_apply(K, V)(kv) dict_with_eltype(DT_apply, ::Type{Pair{K,V}}) where {K,V} = DT_apply(K, V)() diff --git a/base/dict.jl b/base/dict.jl index 2b6fd41d8d075..c924e10ab4d6e 100644 --- a/base/dict.jl +++ b/base/dict.jl @@ -114,45 +114,7 @@ const AnyDict = Dict{Any,Any} Dict(ps::Pair{K,V}...) where {K,V} = Dict{K,V}(ps) Dict(ps::Pair...) = Dict(ps) -function Dict(kv) - try - dict_with_eltype((K, V) -> Dict{K, V}, kv, eltype(kv)) - catch - if !isiterable(typeof(kv)) || !all(x->isa(x,Union{Tuple,Pair}),kv) - throw(ArgumentError("Dict(kv): kv needs to be an iterator of tuples or pairs")) - else - rethrow() - end - end -end - -function grow_to!(dest::AbstractDict{K, V}, itr) where V where K - y = iterate(itr) - y === nothing && return dest - ((k,v), st) = y - dest2 = empty(dest, typeof(k), typeof(v)) - dest2[k] = v - grow_to!(dest2, itr, st) -end - -# this is a special case due to (1) allowing both Pairs and Tuples as elements, -# and (2) Pair being invariant. a bit annoying. -function grow_to!(dest::AbstractDict{K,V}, itr, st) where V where K - y = iterate(itr, st) - while y !== nothing - (k,v), st = y - if isa(k,K) && isa(v,V) - dest[k] = v - else - new = empty(dest, promote_typejoin(K,typeof(k)), promote_typejoin(V,typeof(v))) - merge!(new, dest) - new[k] = v - return grow_to!(new, itr, st) - end - y = iterate(itr, st) - end - return dest -end +Dict(kv) = dict_with_eltype((K, V) -> Dict{K, V}, kv, eltype(kv)) empty(a::AbstractDict, ::Type{K}, ::Type{V}) where {K, V} = Dict{K, V}() diff --git a/base/iddict.jl b/base/iddict.jl index 73f532af9bba6..0b12ae86b2c6a 100644 --- a/base/iddict.jl +++ b/base/iddict.jl @@ -53,18 +53,7 @@ IdDict(ps::Pair{K}...) where {K} = IdDict{K,Any}(ps) IdDict(ps::(Pair{K,V} where K)...) where {V} = IdDict{Any,V}(ps) IdDict(ps::Pair...) = IdDict{Any,Any}(ps) -function IdDict(kv) - try - dict_with_eltype((K, V) -> IdDict{K, V}, kv, eltype(kv)) - catch - if !applicable(iterate, kv) || !all(x->isa(x,Union{Tuple,Pair}),kv) - throw(ArgumentError( - "IdDict(kv): kv needs to be an iterator of tuples or pairs")) - else - rethrow() - end - end -end +IdDict(kv) = dict_with_eltype((K, V) -> IdDict{K, V}, kv, eltype(kv)) empty(d::IdDict, ::Type{K}, ::Type{V}) where {K, V} = IdDict{K,V}() diff --git a/base/weakkeydict.jl b/base/weakkeydict.jl index b827f0d1495da..1a98bf1ee4333 100644 --- a/base/weakkeydict.jl +++ b/base/weakkeydict.jl @@ -54,17 +54,7 @@ WeakKeyDict(ps::Pair{K}...) where {K} = WeakKeyDict{K,Any}(ps) WeakKeyDict(ps::(Pair{K,V} where K)...) where {V} = WeakKeyDict{Any,V}(ps) WeakKeyDict(ps::Pair...) = WeakKeyDict{Any,Any}(ps) -function WeakKeyDict(kv) - try - Base.dict_with_eltype((K, V) -> WeakKeyDict{K, V}, kv, eltype(kv)) - catch - if !isiterable(typeof(kv)) || !all(x->isa(x,Union{Tuple,Pair}),kv) - throw(ArgumentError("WeakKeyDict(kv): kv needs to be an iterator of tuples or pairs")) - else - rethrow() - end - end -end +WeakKeyDict(kv) = Base.dict_with_eltype((K, V) -> WeakKeyDict{K, V}, kv, eltype(kv)) function _cleanup_locked(h::WeakKeyDict) if h.dirty diff --git a/test/dict.jl b/test/dict.jl index f44c6d696bc74..b45ee477de6ef 100644 --- a/test/dict.jl +++ b/test/dict.jl @@ -162,6 +162,30 @@ end # issue #39117 @test Dict(t[1]=>t[2] for t in zip((1,"2"), (2,"2"))) == Dict{Any,Any}(1=>2, "2"=>"2") + + @testset "issue #33147" begin + expected = try; Base._throw_dict_kv_error(); catch e; e; end + @test_throws expected Dict(i for i in 1:2) + @test_throws expected Dict(nothing for i in 1:2) + @test_throws expected Dict(() for i in 1:2) + @test_throws expected Dict((i, i, i) for i in 1:2) + @test_throws expected Dict(nothing) + @test_throws expected Dict((1,)) + @test_throws expected Dict(1:2) + @test_throws expected Dict(((),)) + @test_throws expected IdDict(((),)) + @test_throws expected WeakKeyDict(((),)) + @test_throws expected IdDict(nothing) + @test_throws expected WeakKeyDict(nothing) + @test Dict(1:0) isa Dict + @test Dict(()) isa Dict + try + Dict(i => error("$i") for i in 1:3) + catch ex + @test ex isa ErrorException + @test length(Base.catch_stack()) == 1 + end + end end @testset "empty tuple ctor" begin From 2b329ef402dd881f961da39f5e8be7ee14788994 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Thu, 1 Feb 2024 17:17:38 -0500 Subject: [PATCH 2/3] Update dict.jl --- test/dict.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dict.jl b/test/dict.jl index b45ee477de6ef..ca8a598de0b81 100644 --- a/test/dict.jl +++ b/test/dict.jl @@ -183,7 +183,7 @@ end Dict(i => error("$i") for i in 1:3) catch ex @test ex isa ErrorException - @test length(Base.catch_stack()) == 1 + @test length(Base.current_exceptions()) == 1 end end end From 6e0470ac8422ce019d0566f23129d610cac8ddd3 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Thu, 8 Feb 2024 14:57:02 -0500 Subject: [PATCH 3/3] Update base/abstractdict.jl --- base/abstractdict.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/abstractdict.jl b/base/abstractdict.jl index 915ba21e51615..f73b518930a07 100644 --- a/base/abstractdict.jl +++ b/base/abstractdict.jl @@ -579,7 +579,7 @@ _tablesz(x::T) where T <: Integer = x < 16 ? T(16) : one(T)<<(top_set_bit(x-one( TP{K,V} = Union{Type{Tuple{K,V}},Type{Pair{K,V}}} -# validate the contents of this iterator by testing the iteration protocol (isiterable) on it +# This error is thrown if `grow_to!` cannot validate the contents of the iterator argument to it, which it does by testing the iteration protocol (isiterable) on it each time it is about to start iteration on it _throw_dict_kv_error() = throw(ArgumentError("AbstractDict(kv): kv needs to be an iterator of 2-tuples or pairs")) function grow_to!(dest::AbstractDict, itr)