From 7101087fa0e9f917961fde2b933be860dbf494fd 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 23dd26ada7d4dfdc854c3e09554be386757e192b 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 feeb0d62fd797933a198760ceea862794c64aa48 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