Skip to content

Commit

Permalink
Make pop!(::Set{A}, ::B) return an A, not B (JuliaLang#52017)
Browse files Browse the repository at this point in the history
Previously, `pop!(::Set, x)` returned `x`, not the element in the
set. This matters if multiple different elements are equal and hash to
the same.

Before:
```julia
julia> pop!(Set([1]), 0x01)
0x01
```
Now:
```julia
julia> pop!(Set([1]), 0x01)
1
```
  • Loading branch information
jakobnissen authored Nov 8, 2023
1 parent 9f2f3ce commit 6f6419c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
9 changes: 8 additions & 1 deletion base/set.jl
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,15 @@ function in!(x, s::Set)
end

push!(s::Set, x) = (s.dict[x] = nothing; s)
pop!(s::Set, x) = (pop!(s.dict, x); x)

pop!(s::Set, x, default) = (x in s ? pop!(s, x) : default)
function pop!(s::Set, x)
index = ht_keyindex(s.dict, x)
index < 1 && throw(KeyError(x))
result = @inbounds s.dict.keys[index]
_delete!(s.dict, index)
result
end

function pop!(s::Set)
isempty(s) && throw(ArgumentError("set must be non-empty"))
Expand Down
8 changes: 8 additions & 0 deletions test/sets.jl
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,14 @@ end
@test isempty(s)
@test_throws ArgumentError pop!(s)
@test length(Set(['x',120])) == 2

# Test that pop! returns the element in the set, not the query
s = Set{Any}(Any[0x01, UInt(2), 3, 4.0])
@test pop!(s, 1) === 0x01
@test pop!(s, 2) === UInt(2)
@test pop!(s, 3) === 3
@test pop!(s, 4) === 4.0
@test_throws KeyError pop!(s, 5)
end
@testset "copy" begin
data_in = (1,2,9,8,4)
Expand Down

0 comments on commit 6f6419c

Please sign in to comment.