Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StackOverflowError on a push! into a Dict #37349

Open
jcampolongo opened this issue Sep 2, 2020 · 1 comment
Open

StackOverflowError on a push! into a Dict #37349

jcampolongo opened this issue Sep 2, 2020 · 1 comment

Comments

@jcampolongo
Copy link

The creation of a dictionary with a key of type Tuple{Type{<:Real}} causes a StackOverflowError on a push!. This occurs on both 1.5.0 and 1.5.1 versions of Julia (on windows).

julia> d = Dict{Tuple{Type{<:Real}},Int64}()
Dict{Tuple{Type{var"#s2248"} where var"#s2248"<:Real},Int64}()
julia> push!(d, (Int64,)=>10)
ERROR: StackOverflowError:
Stacktrace:
 [1] convert(::Type{Tuple{Type{var"#s2247"} where var"#s2247"<:Real}}, ::Tuple{DataType}) at .\essentials.jl:310
 [2] setindex!(::Dict{Tuple{Type{var"#s2248"} where var"#s2248"<:Real},Int64}, ::Int64, ::Tuple{DataType}) at .\dict.jl:372
 [3] setindex!(::Dict{Tuple{Type{var"#s2248"} where var"#s2248"<:Real},Int64}, ::Int64, ::Tuple{DataType}) at .\dict.jl:376 (repeats 29007 times)
 [4] push!(::Dict{Tuple{Type{var"#s2248"} where var"#s2248"<:Real},Int64}, ::Pair{Tuple{DataType},Int64}) at .\abstractdict.jl:510
 [5] top-level scope at REPL[150]:1

Using a Type{<:Real} does not have this issue.

julia> d = Dict{Type{<:Real}, Int64}()
Dict{Type{var"#s2249"} where var"#s2249"<:Real,Int64}()
julia> push!(d, Int64=>10)
Dict{Type{var"#s2249"} where var"#s2249"<:Real,Int64} with 1 entry:
  Int64 => 10

Version Information

julia> versioninfo()
Julia Version 1.5.1
Commit 697e782ab8 (2020-08-25 20:08 UTC)
Platform Info:
  OS: Windows (x86_64-w64-mingw32)
  CPU: Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-9.0.1 (ORCJIT, skylake-avx512)
@vtjnash
Copy link
Sponsor Member

vtjnash commented Sep 2, 2020

The issue is that convert(Tuple{...}, x) doesn't guarantee that the result is of type T (mostly because NamedTuples now rely upon it), so it's not getting the expected type, and just tries (and fails) repeatedly. I tried this patch, but it runs into inference problem (an incorrect answer by the typeassert_tfunc tuple_tfunc)

diff --git a/base/dict.jl b/base/dict.jl
index cc5c9efb6a..9628285bf4 100644
--- a/base/dict.jl
+++ b/base/dict.jl
@@ -373,3 +373,3 @@ end
 function setindex!(h::Dict{K,V}, v0, key0) where V where K
-    key = convert(K, key0)
+    key = convert(K, key0)::K
     if !isequal(key, key0)
@@ -381,3 +381,3 @@ end
 function setindex!(h::Dict{K,V}, v0, key::K) where V where K
-    v = convert(V, v0)
+    v = convert(V, v0)::V
     index = ht_keyindex2!(h, key)
@@ -451,3 +451,3 @@ get!(f::Function, collection, key)
 function get!(default::Callable, h::Dict{K,V}, key0) where V where K
-    key = convert(K, key0)
+    key = convert(K, key0)::K
     if !isequal(key, key0)
@@ -464,3 +464,3 @@ function get!(default::Callable, h::Dict{K,V}, key::K) where V where K
     age0 = h.age
-    v = convert(V, default())
+    v = convert(V, default())::V
     if h.age != age0

vtjnash added a commit that referenced this issue Sep 2, 2020
The tuple_tfunc failed to account for the possibility that the Type might be obscured inside a UnionAll or Union, and would fail to widen the params in that case.

Refs #37349
vtjnash added a commit that referenced this issue Sep 2, 2020
Avoids a stack overflow and gives a proper error instead (as well as
possibly improving inference of the consumers).

Fixes #37349
vtjnash added a commit that referenced this issue Sep 2, 2020
The tuple_tfunc failed to account for the possibility that the Type might be obscured inside a UnionAll or Union, and would fail to widen the params in that case.

Refs #37349
vtjnash added a commit that referenced this issue Sep 2, 2020
Avoids a stack overflow and gives a proper error instead (as well as
possibly improving inference of the consumers).

Fixes #37349
vtjnash added a commit that referenced this issue Sep 2, 2020
The tuple_tfunc failed to account for the possibility that the Type might be obscured inside a UnionAll or Union, and would fail to widen the params in that case.

Refs #37349
vtjnash added a commit that referenced this issue Sep 2, 2020
Avoids a stack overflow and gives a proper error instead (as well as
possibly improving inference of the consumers).

Fixes #37349
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants