Dict construction calls generator side effects twice on failure

[BrantCarlson]

Greetings...

This is not a major issue, but potentially reflects some underlying bug and is certainly confusing: when creating a Dict with a generator expression and an exception occurs, for some reason the generator restarts until it hits the exception a second time. Once it hits the exception a second time, it stops, thankfully. For example (using @show to track execution) Dict(x => sqrt(@show x) for x=[1,2,3,4,-5,6]) does everything (but the 6) twice. I would have expected it to immediately error out, but...

julia> Dict(x => sqrt(@show x) for x=[1,2,3,4,-5,6])
x = 1
x = 2
x = 3
x = 4
x = -5
x = 1
x = 2
x = 3
x = 4
x = -5
ERROR: DomainError with -5.0:
sqrt will only return a complex result if called with a complex argument. Try sqrt(Complex(x)).
Stacktrace:
 [1] throw_complex_domainerror(::Symbol, ::Float64) at ./math.jl:31
 [2] sqrt at ./math.jl:493 [inlined]
 [3] sqrt at ./math.jl:519 [inlined]
 [4] #21 at ./show.jl:576 [inlined]
 [5] iterate at ./generator.jl:47 [inlined]
 [6] _all(::getfield(Base, Symbol("##238#240")), ::Base.Generator{Array{Int64,1},getfield(Main, Symbol("##21#22"))}, ::Colon) at ./reduce.jl:744
 [7] all at ./reduce.jl:732 [inlined]
 [8] Dict(::Base.Generator{Array{Int64,1},getfield(Main, Symbol("##21#22"))}) at ./dict.jl:130
 [9] top-level scope at REPL[7]:1
caused by [exception 1]
DomainError with -5.0:
sqrt will only return a complex result if called with a complex argument. Try sqrt(Complex(x)).
Stacktrace:
 [1] throw_complex_domainerror(::Symbol, ::Float64) at ./math.jl:31
 [2] sqrt at ./math.jl:493 [inlined]
 [3] sqrt at ./math.jl:519 [inlined]
 [4] #21 at ./show.jl:576 [inlined]
 [5] iterate at ./generator.jl:47 [inlined]
 [6] Dict{Int64,Float64}(::Base.Generator{Array{Int64,1},getfield(Main, Symbol("##21#22"))}) at ./dict.jl:103
 [7] dict_with_eltype at ./abstractdict.jl:538 [inlined]
 [8] dict_with_eltype at ./abstractdict.jl:545 [inlined]
 [9] Dict(::Base.Generator{Array{Int64,1},getfield(Main, Symbol("##21#22"))}) at ./dict.jl:128
 [10] top-level scope at REPL[7]:1

In my case an exception occurred near the end of a calculation that should have taken 30 minutes. I started getting suspicious when it was still running after 40 minutes, even more so when I noticed it was re-doing parts of the job that should have been finished earlier. Granted I of course should have written code that didn't crash, but still. :)

Many thanks for a great programming environment...

Version info, if that helps. (The same issue also occurs on 1.3.0-rc1.0)

julia> versioninfo()
Julia Version 1.2.0
Commit c6da87ff4b (2019-08-20 00:03 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: Intel(R) Core(TM) i7-7600U CPU @ 2.80GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-6.0.1 (ORCJIT, skylake)

[c42f]

This seems to be an unintended consequence of the error handling introduced in #12453: namely that if the generator has side effects, those side effects occur twice (once in the initial construction, and once in the check all(x->isa(x,Union{Tuple,Pair}),kv) for generating the error message.

[c42f]

Also, thanks for the bug report!

[vtjnash]

Maybe just drop that code now for #12451? I think our default error reporting now may provide enough context from showing the argument types and bounds error information that this won't be needed anymore.

[c42f]

I tried that out, but the resulting errors were a lot less obvious. Dict is such a commonly used basic type that it seems worth going to a little effort to preserve a more friendly error.

Maybe a heuristic like

if !isiterable(typeof(kv)) || (eltype(kv) !== Any && !isa(eltype(kv), Union{Tuple,Pair}))

would serve fine — we don't need to be precise here, only offer a more helpful error "when possible"

[omus]

On Julia 1.6 the "caused by" statement appears on the same line as the redundant error message resulting in this misleading statement:

caused by: DomainError with -5.0:

Edit: This was just an error formatting change which I was unfamiliar with. I had only seen the caused by with this duplicate error issue

[lmshk]

I just ran into this issue; it confused me for a while, especially since side-effects of the generator were caused twice before the double error was printed. I am not sure what guarantees Julia normally makes regarding iteration restarts, but this behavior is especially confusing with once-iterables like channels:

f(xs) = Channel() do c
    push!.(Ref(c), xs)
end

Dict(x => @show log(x) for x in f([1, 2]))  # ok
Dict(x => @show log(x) for x in f([1, -1, 2]))  # DomainError with -1.0
Dict(x => @show log(x) for x in f([1, -1, 2, -2]))  # DomainError with -2.0, caused by: DomainError with -1.0
Dict(x => @show log(x) for x in f([1, -1, 2, -2, 3]))  # same, but 3 is left in channel

The solution proposed in #40445 does not fix this.

I think I would prefer the BoundsError: attempt to access [...] at index [2] to the current behavior, especially since the actual error (if the generator does not itself throw, i.e. ArgumentError: Dict(kv): kv needs to be an iterator of tuples or pairs) is inaccurate too: kv apparently just needs to be indexable at 1 and 2, and trailing elements seem to be ignored:

julia> Dict([[1, 2, 3]])

Dict{Int64, Int64} with 1 entry:
  1 => 2

Alternatively, maybe the code that fails to index could wrap the BoundsError right there? (It looks like that would be here and here.)

Should I try a PR or am I missing something?