Skip to content

Commit 6f6419c

Browse files
authored
Make pop!(::Set{A}, ::B) return an A, not B (#52017)
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 ```
1 parent 9f2f3ce commit 6f6419c

File tree

2 files changed

+16
-1
lines changed

2 files changed

+16
-1
lines changed

base/set.jl

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,15 @@ function in!(x, s::Set)
101101
end
102102

103103
push!(s::Set, x) = (s.dict[x] = nothing; s)
104-
pop!(s::Set, x) = (pop!(s.dict, x); x)
104+
105105
pop!(s::Set, x, default) = (x in s ? pop!(s, x) : default)
106+
function pop!(s::Set, x)
107+
index = ht_keyindex(s.dict, x)
108+
index < 1 && throw(KeyError(x))
109+
result = @inbounds s.dict.keys[index]
110+
_delete!(s.dict, index)
111+
result
112+
end
106113

107114
function pop!(s::Set)
108115
isempty(s) && throw(ArgumentError("set must be non-empty"))

test/sets.jl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,14 @@ end
124124
@test isempty(s)
125125
@test_throws ArgumentError pop!(s)
126126
@test length(Set(['x',120])) == 2
127+
128+
# Test that pop! returns the element in the set, not the query
129+
s = Set{Any}(Any[0x01, UInt(2), 3, 4.0])
130+
@test pop!(s, 1) === 0x01
131+
@test pop!(s, 2) === UInt(2)
132+
@test pop!(s, 3) === 3
133+
@test pop!(s, 4) === 4.0
134+
@test_throws KeyError pop!(s, 5)
127135
end
128136
@testset "copy" begin
129137
data_in = (1,2,9,8,4)

0 commit comments

Comments
 (0)