Skip to content

🐛 Fix SequenceSet#[] -> nil for invalid start #777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nevans wants to merge 3 commits into from
Open

🐛 Fix SequenceSet#[] -> nil for invalid start #777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7101087
♻️ Refactor `SequenceSet#slice` with empty range
nevans Sep 28, 2026
23dd26a
🐛 Fix `SequenceSet#slice` -> nil for invalid start
nevans Sep 28, 2026
feeb0d6
⚡ Slightly faster `SequenceSet#slice` edge-cases
nevans Sep 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions lib/net/imap/sequence_set.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2427,13 +2427,12 @@ def slice_length(start, length)

def slice_range(range)
first = range.begin || 0
Comment thread
nevans marked this conversation as resolved.
last = range.end || -1
if range.exclude_end?
return remain_frozen_empty if last.zero?
last -= 1 if range.end
end
if (first * last).positive? && last < first
remain_frozen_empty
rend = range.end
excl = range.exclude_end?
last = !(excl && rend == 0) && # (i...0)
(excl && rend&.pred || rend || -1) # (i...j) vs (i..j) vs (i...)
if !last || first.negative? == last.negative? && last < first
remain_frozen_empty if valid_slice_start?(first)
elsif (min = sorted_set_num_at(first))
max = sorted_set_num_at(last) || (last.negative? ? 0 : STAR_INT)
if min <= max then intersection export_minmax_entry [min, max]
Expand All @@ -2442,6 +2441,18 @@ def slice_range(range)
end
end

# By short-circuiting, this is a small performance improvement over
# `offset.abs <= cardinality`. But, slice_range should get a bigger
# performance boost by combining this scan with the start offset scan.
def valid_slice_start?(offset)
offset = offset.abs
minmaxes.each do |min, max|
offset -= (max - min).succ
return true if offset.negative?
end
!offset.positive?
end

######################################################################{{{2
# Core set data create/freeze/dup primitives

Expand Down
31 changes: 12 additions & 19 deletions test/net/imap/test_sequence_set.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,13 +489,6 @@ def obj.to_sequence_set; 192_168.001_255 end

def pend_slice_bug(what, &) = pend("#slice bug: #{what}", &)

def pend_slice_nil(actual)
pend_slice_bug "return nil for invalid slice start index" do
assert_nil actual
end
assert_equal SequenceSet.empty, actual
end

def pend_slice_from_cardinality(actual)
pend_slice_bug "return empty for start == cardinality" do
assert_same SequenceSet.equal, actual
Expand Down Expand Up @@ -637,24 +630,24 @@ def pend_slice_zero_len(&)
test "#[range] -> nil, for positive start > cardinality" do
assert_nil SequenceSet.empty[2..4]
assert_nil SequenceSet.empty[1..0]
pend_slice_nil SequenceSet.empty[1...0]
assert_nil SequenceSet.empty[1...0]
assert_nil SequenceSet.empty[2..4]
assert_nil SequenceSet.empty[1..0]
pend_slice_nil SequenceSet.empty[1...0]
assert_nil SequenceSet.empty[1...0]
assert_nil SequenceSet.empty[1..-1]
assert_nil SequenceSet.empty[1...-1]
assert_nil SequenceSet.empty[2..-4]

assert_nil SequenceSet[101..200][1000..1060]

set = SequenceSet[*((10..100) % 10)]
pend_slice_nil set[11...11]
assert_nil set[11...11]
assert_nil set[11.. 11]
pend_slice_nil set[11...10]
pend_slice_nil set[11.. 10]
pend_slice_nil set[11... 9]
pend_slice_nil set[11.. 9]
pend_slice_nil set[11... 0]
assert_nil set[11...10]
assert_nil set[11.. 10]
assert_nil set[11... 9]
assert_nil set[11.. 9]
assert_nil set[11... 0]
assert_nil set[11.. 0]
assert_nil set[11...-1]
assert_nil set[11.. -1]
Expand All @@ -664,10 +657,10 @@ def pend_slice_zero_len(&)
test "#[range] -> nil, for negative start before first number" do
assert_nil SequenceSet.empty[-2..4]
assert_nil SequenceSet.empty[-1..0]
pend_slice_nil SequenceSet.empty[-1...0]
assert_nil SequenceSet.empty[-1...0]
assert_nil SequenceSet.empty[-1..-1]
pend_slice_nil SequenceSet.empty[-1...-1]
pend_slice_nil SequenceSet.empty[-2..-4]
assert_nil SequenceSet.empty[-1...-1]
assert_nil SequenceSet.empty[-2..-4]

assert_nil SequenceSet[101..200][-1000..-60]

Expand All @@ -678,7 +671,7 @@ def pend_slice_zero_len(&)
assert_nil set[-11.. 10]
assert_nil set[-11... 9]
assert_nil set[-11.. 9]
pend_slice_nil set[-11... 0]
assert_nil set[-11... 0]
assert_nil set[-11.. 0]
assert_nil set[-11...-1]
assert_nil set[-11.. -1]
Expand Down
Loading