Fix for http://collector.zope.org/Zope/419,

"BTreeItems slice contains 1 too many elements"
This also fixes many related glitches, such as that giving an
out-of-bound slice index raised IndexError instead of clipping.

BTreeItems_slice():  Emulate Python slicing semantics in all cases.

testBTrees.py:  new testSlicing() tests in MappingBase and NormalSetTests
to ensure that slicing of .keys()/.items()/.values() works exactly the
same way as slicing of Python lists, in all one-sided, two-sided and
whole-structure ([:]) cases, with both negative and positive slice indices,
and mixtures of + and -, and whether in bounds or out of bounds.

src/BTrees/BTreeItemsTemplate.c

@@ -12,15 +12,15 @@
 
  ****************************************************************************/
 
-#define BTREEITEMSTEMPLATE_C "$Id: BTreeItemsTemplate.c,v 1.10 2002/06/09 17:19:05 tim_one Exp $\n"
+#define BTREEITEMSTEMPLATE_C "$Id: BTreeItemsTemplate.c,v 1.11 2002/06/09 19:27:48 tim_one Exp $\n"
 
 typedef struct {
   PyObject_HEAD
-  Bucket *firstbucket;			/* First bucket known		*/
-  Bucket *currentbucket;		/* Current bucket position	*/
-  Bucket *lastbucket;			/* Last bucket position		*/
-  int currentoffset;			/* Start count of current bucket*/
-  int pseudoindex;			/* Its an indicator		*/
+  Bucket *firstbucket;		/* First bucket known		*/
+  Bucket *currentbucket;	/* Current bucket position	*/
+  Bucket *lastbucket;		/* Last bucket position		*/
+  int currentoffset;		/* Start count of current bucket*/
+  int pseudoindex;		/* It's an indicator		*/
   int first, last;
   char kind;
 } BTreeItems;
@@ -348,18 +348,70 @@ BTreeItems_slice(BTreeItems *self, int ilow, int ihigh)
   Bucket *highbucket;
   int lowoffset;
   int highoffset;
+  int length = -1;  /* len(self), but computed only if needed */
+
+  /* Complications:
+   * A Python slice never raises IndexError, but BTreeItems_seek does.
+   * Python did only part of index normalization before calling this:
+   *     ilow may be < 0 now, and ihigh may be arbitrarily large.  It's
+   *     our responsibility to clip them.
+   * A Python slice is exclusive of the high index, but a BTreeItems
+   *     struct is inclusive on both ends.
+   */
+
+  /* First adjust ilow and ihigh to be legit endpoints in the Python
+   * sense (ilow inclusive, ihigh exclusive).  This block duplicates the
+   * logic from Python's list_slice function (slicing for builtin lists).
+   */
+  if (ilow < 0)
+      ilow = 0;
+  else {
+      if (length < 0)
+          length = BTreeItems_length(self);
+      if (ilow > length)
+          ilow = length;
+  }
 
-
-  if (BTreeItems_seek(self, ilow) < 0) return NULL;
-
-  lowbucket = self->currentbucket;
-  lowoffset = self->currentoffset;
-
-  if (BTreeItems_seek(self, ihigh) < 0) return NULL;
-
-  highbucket = self->currentbucket;
-  highoffset = self->currentoffset;
-
+  if (ihigh < ilow)
+      ihigh = ilow;
+  else {
+      if (length < 0)
+          length = BTreeItems_length(self);
+      if (ihigh > length)
+          ihigh = length;
+  }
+  assert(0 <= ilow && ilow <= ihigh);
+  assert(length < 0 || ihigh <= length);
+
+  /* Now adjust for that our struct is inclusive on both ends.  This is
+   * easy *except* when the slice is empty:  there's no good way to spell
+   * that in an inclusive-on-both-ends scheme.  For example, if the
+   * slice is btree.items([:0]), ilow == ihigh == 0 at this point, and if
+   * we were to subtract 1 from ihigh that would get interpreted by
+   * BTreeItems_seek as meaning the *entire* set of items.  Setting ilow==1
+   * and ihigh==0 doesn't work either, as BTreeItems_seek raises IndexError
+   * if we attempt to seek to ilow==1 when the underlying sequence is empty.
+   * It seems simplest to deal with empty slices as a special case here.
+   */
+   if (ilow == ihigh) {
+       /* empty slice */
+       lowbucket = highbucket = self->currentbucket;
+       lowoffset = 1;
+       highoffset = 0;
+   }
+   else {
+       assert(ilow < ihigh);
+       --ihigh;  /* exclusive -> inclusive */
+
+       if (BTreeItems_seek(self, ilow) < 0) return NULL;
+       lowbucket = self->currentbucket;
+       lowoffset = self->currentoffset;
+
+       if (BTreeItems_seek(self, ihigh) < 0) return NULL;
+
+       highbucket = self->currentbucket;
+       highoffset = self->currentoffset;
+  }
   return newBTreeItems(self->kind,
                        lowbucket, lowoffset, highbucket, highoffset);
 }

src/BTrees/tests/testBTrees.py

@@ -250,6 +250,82 @@ class MappingBase(Base):
         self.assertEqual(list(t.keys(6,8)),[], list(t.keys(6,8)))
         self.assertEqual(list(t.keys(10,12)),[], list(t.keys(10,12)))
 
+    def testSlicing(self):
+        # Test that slicing of .keys()/.values()/.items() works exactly the
+        # same way as slicing a Python list with the same contents.
+        # This tests fixes to several bugs in this area, starting with
+        # http://collector.zope.org/Zope/419,
+        # "BTreeItems slice contains 1 too many elements".
+
+        t = self.t
+        for n in range(10):
+            t.clear()
+            self.assertEqual(len(t), 0)
+
+            keys = []
+            values = []
+            items = []
+            for key in range(n):
+                value = -2 * key
+                t[key] = value
+                keys.append(key)
+                values.append(value)
+                items.append((key, value))
+            self.assertEqual(len(t), n)
+
+            kslice = t.keys()
+            vslice = t.values()
+            islice = t.items()
+            self.assertEqual(len(kslice), n)
+            self.assertEqual(len(vslice), n)
+            self.assertEqual(len(islice), n)
+
+            # Test whole-structure slices.
+            x = kslice[:]
+            self.assertEqual(list(x), keys[:])
+
+            x = vslice[:]
+            self.assertEqual(list(x), values[:])
+
+            x = islice[:]
+            self.assertEqual(list(x), items[:])
+
+            for lo in range(-2*n, 2*n+1):
+                # Test one-sided slices.
+                x = kslice[:lo]
+                self.assertEqual(list(x), keys[:lo])
+                x = kslice[lo:]
+                self.assertEqual(list(x), keys[lo:])
+
+                x = vslice[:lo]
+                self.assertEqual(list(x), values[:lo])
+                x = vslice[lo:]
+                self.assertEqual(list(x), values[lo:])
+
+                x = islice[:lo]
+                self.assertEqual(list(x), items[:lo])
+                x = islice[lo:]
+                self.assertEqual(list(x), items[lo:])
+
+                for hi in range(-2*n, 2*n+1):
+                    # Test two-sided slices.
+                    x = kslice[lo:hi]
+                    self.assertEqual(list(x), keys[lo:hi])
+
+                    x = vslice[lo:hi]
+                    self.assertEqual(list(x), values[lo:hi])
+
+                    x = islice[lo:hi]
+                    self.assertEqual(list(x), items[lo:hi])
+
+        # The specific test case from Zope collector 419.
+        t.clear()
+        for i in xrange(100):
+            t[i] = 1
+        tslice = t.items()[20:80]
+        self.assertEqual(len(tslice), 60)
+        self.assertEqual(list(tslice), zip(range(20, 80), [1]*60))
+
 
 class NormalSetTests(Base):
     """ Test common to all set types """
@@ -352,6 +428,39 @@ class NormalSetTests(Base):
         self.assertEqual(list(t.keys(6,8)),[], list(t.keys(6,8)))
         self.assertEqual(list(t.keys(10,12)),[], list(t.keys(10,12)))
 
+    def testSlicing(self):
+        # Test that slicing of .keys() works exactly the same way as slicing
+        # a Python list with the same contents.
+
+        t = self.t
+        for n in range(10):
+            t.clear()
+            self.assertEqual(len(t), 0)
+
+            keys = range(10*n, 11*n)
+            t.update(keys)
+            self.assertEqual(len(t), n)
+
+            kslice = t.keys()
+            self.assertEqual(len(kslice), n)
+
+            # Test whole-structure slices.
+            x = kslice[:]
+            self.assertEqual(list(x), keys[:])
+
+            for lo in range(-2*n, 2*n+1):
+                # Test one-sided slices.
+                x = kslice[:lo]
+                self.assertEqual(list(x), keys[:lo])
+                x = kslice[lo:]
+                self.assertEqual(list(x), keys[lo:])
+
+                for hi in range(-2*n, 2*n+1):
+                    # Test two-sided slices.
+                    x = kslice[lo:hi]
+                    self.assertEqual(list(x), keys[lo:hi])
+
+
 class ExtendedSetTests(NormalSetTests):
     def testLen(self):
         t = self.t