set! is not copy safe

[mtfishman]

julia> using Dictionaries

julia> d = Dictionary(["X", "Y"], [1, 2])
2-element Dictionary{String, Int64}
 "X" │ 1
 "Y" │ 2

julia> dc = copy(d)
2-element Dictionary{String, Int64}
 "X" │ 1
 "Y" │ 2

julia> set!(dc, "Z", 3)
3-element Dictionary{String, Int64}
 "X" │ 1
 "Y" │ 2
 "Z" │ 3

julia> dc
3-element Dictionary{String, Int64}
 "X" │ 1
 "Y" │ 2
 "Z" │ 3

julia> d
3-element Dictionary{String, Int64}
 "X" │ 1
 "Y" │ 2
 "Z" │ #undef

deepcopy is ok:

julia> d = Dictionary(["X", "Y"], [1, 2])
2-element Dictionary{String, Int64}
 "X" │ 1
 "Y" │ 2

julia> dc = deepcopy(d)
2-element Dictionary{String, Int64}
 "X" │ 1
 "Y" │ 2

julia> set!(dc, "Z", 3)
3-element Dictionary{String, Int64}
 "X" │ 1
 "Y" │ 2
 "Z" │ 3

julia> d
2-element Dictionary{String, Int64}
 "X" │ 1
 "Y" │ 2

[andyferris]

Yes. Currently copy only copies values and not the keys, so that keys(d) === keys(copy(d)). Internally it uses similar and copyto! to copy the values.

This is something I have come to regret, and we might like to change the semantics of copy or else introduce a new function to copy keys and values. I have found in practice the (theoretically faster) value-only copy hurts more than it helps, and it's easier to e.g. map(identity, d) when you want that then it is to make a proper copy of both keys and values.

[andyferris]

(I'd welcome a PR on this one)

[mtfishman]

Yes, I think the surprise of the behavior above outweighs the performance gain from not copying the indices (the current behavior led to a bug that took me a while to pin down). I changed the copying behavior in #101.