Problem in DataStructures.jl

[Scott Jones]

While trying to get Gadfly.jl working (and eliminating the many warning messages that come up simply trying to do "using Gadfly" under v0.5),

I ran into the following problem in DataStructures.jl/src/accumulator.jl, that I'd like some guidance on.

The Accumulator type is simply a wrapper around a Dict.

The issue is that in v0.5, there is a warning, because there is an outer constructor defined, which simply takes a dictionary passed as an argument, copies it,

and then calls the inner constructor, however, this also overwrites the inner constructor's method, so it is no longer accessible.

Further down, in the two places where the the Accumulator constructor is used, there are explicit copies, so in fact, the dictionary gets copied twice, unnecessarily.

This seems like a bug, where somebody added the outer Accumulator constructor in order to ensure that a copy was always made, but neglected to remove the copy from places where it was done explicitly.

Should the simple fix of simply removing the outer constructor with the extra copy, which also prevents the overwriting warning be done,

or rather change to using only inner constructors, which force any dictionaries to be passed to always be copied? (IMO, it would be better to leave any copying

up to the caller, and document that the dictionary passed to Accumulator will be modified.

I have already made a branch that fixes this and a couple other bugs, in https://github.com/ScottPJones/DataStructures.jl/tree/spj/fixv05.

I would have submitted a PR, however, due to current circumstances, since DataStructures.jl is in JuliaLang, I am unable to do so.

Thanks,

Scott

[Daniel Arndt]

Just to close the gap, this PR is now here: https://github.com/JuliaLang/DataStructures.jl/pull/187