From 1450930f29ffb4729c3e579e928ed4267f7e7748 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 28 Sep 2026 12:18:38 -0400 Subject: [PATCH 1/3] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Refactor=20`SequenceSe?= =?UTF-8?q?t#slice`=20with=20empty=20range?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This merges together both `#slice_range` short-circuit cases where we can immediately deduce that the range is empty. NOTE that this refactoring does _not_ fix an existing bug: the same-sign detection works for positive..positive and negative..negative, but it messes up when beginning or ending with zero. But fixing that bug causes more scenarios to return empty when they should return nil`ZZ. --- lib/net/imap/sequence_set.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/net/imap/sequence_set.rb b/lib/net/imap/sequence_set.rb index ecd113e9a..94e133df7 100644 --- a/lib/net/imap/sequence_set.rb +++ b/lib/net/imap/sequence_set.rb @@ -2427,12 +2427,11 @@ def slice_length(start, length) def slice_range(range) first = range.begin || 0 - 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 + 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 * last).positive? && last < first remain_frozen_empty elsif (min = sorted_set_num_at(first)) max = sorted_set_num_at(last) || (last.negative? ? 0 : STAR_INT) From 604a9544cf95dbbc5721f5e802aaeb4b71a58837 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 28 Sep 2026 11:48:17 -0400 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=90=9B=20Fix=20`SequenceSet#slice`=20?= =?UTF-8?q?->=20nil=20for=20invalid=20start?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes `SequenceSet#slice` so it always returns `nil` when the starting offset is out-of-range, changing a couple situations where it previously returned an empty set: when an exclusive range end is zero, and when range.end is less than same-sign range.begin. Note that `set[cardinality..]` is _not_ considered invalid. This intentionally mimics the behavior of `Array#slice`. This can be refactored more, to simplify and reduce code duplication, and for performance. --- lib/net/imap/sequence_set.rb | 4 ++-- test/net/imap/test_sequence_set.rb | 31 ++++++++++++------------------ 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/lib/net/imap/sequence_set.rb b/lib/net/imap/sequence_set.rb index 94e133df7..5d2be47bb 100644 --- a/lib/net/imap/sequence_set.rb +++ b/lib/net/imap/sequence_set.rb @@ -2431,8 +2431,8 @@ def slice_range(range) 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 * last).positive? && last < first - remain_frozen_empty + if !last || first.negative? == last.negative? && last < first + remain_frozen_empty if first.abs <= cardinality 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] diff --git a/test/net/imap/test_sequence_set.rb b/test/net/imap/test_sequence_set.rb index b1b9acb35..1b95ef8f7 100644 --- a/test/net/imap/test_sequence_set.rb +++ b/test/net/imap/test_sequence_set.rb @@ -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 @@ -637,10 +630,10 @@ 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] @@ -648,13 +641,13 @@ def pend_slice_zero_len(&) 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] @@ -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] @@ -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] From fd0118609634be87156cb25fa9be1dafbda1cd45 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 28 Sep 2026 11:20:38 -0400 Subject: [PATCH 3/3] =?UTF-8?q?=E2=9A=A1=20Slightly=20faster=20`SequenceSe?= =?UTF-8?q?t#slice`=20edge-cases?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By short-circuiting the check that `offset.abs <= cardinality`, this is a small performance improvement over the naive approach. A future refactoring should combine this with the start offset scan (`min = sorted_set_num_at`). --- lib/net/imap/sequence_set.rb | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/net/imap/sequence_set.rb b/lib/net/imap/sequence_set.rb index 5d2be47bb..62db9905c 100644 --- a/lib/net/imap/sequence_set.rb +++ b/lib/net/imap/sequence_set.rb @@ -2432,7 +2432,7 @@ def slice_range(range) 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 first.abs <= cardinality + 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] @@ -2441,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