Fleshed out WeakKeyDictionary.__delitem__ NEWS to cover issues raised on

Python-Dev.  Fixed typos in test comments.  Added some trivial new test
guts to show the parallelism (now) among __delitem__, __setitem__ and
__getitem__ wrt error conditions.

Still a bugfix candidate for 2.2.3 final, but waiting for Fred to get a
chance to chime in.

2 files changed, 23 insertions, 7 deletions

diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py
index c5fbb8d..5969f35 100644
--- a/Lib/test/test_weakref.py
+++ b/Lib/test/test_weakref.py
@@ -520,8 +520,15 @@ class MappingTestCase(TestBase):
         d = weakref.WeakKeyDictionary()
         o = Object('1')
         # An attempt to delete an object that isn't there should raise
-        # KetError.  It didn't before 2.3.
+        # KeyError.  It didn't before 2.3.
         self.assertRaises(KeyError, d.__delitem__, o)
+        self.assertRaises(KeyError, d.__getitem__, o)
+
+        # If a key isn't of a weakly referencable type, __getitem__ and
+        # __setitem__ raise TypeError.  __delitem__ should too.
+        self.assertRaises(TypeError, d.__delitem__,  13)
+        self.assertRaises(TypeError, d.__getitem__,  13)
+        self.assertRaises(TypeError, d.__setitem__,  13, 13)
 
     def test_weak_keyed_cascading_deletes(self):
         # SF bug 742860.  For some reason, before 2.3 __delitem__ iterated
@@ -552,12 +559,13 @@ class MappingTestCase(TestBase):
         # Reverse it, so that the iteration implementation of __delitem__
         # has to keep looping to find the first object we delete.
         objs.reverse()
+
         # Turn on mutation in C.__eq__.  The first time thru the loop,
         # under the iterkeys() business the first comparison will delete
         # the last item iterkeys() would see, and that causes a
         #     RuntimeError: dictionary changed size during iteration
         # when the iterkeys() loop goes around to try comparing the next
-        # key.  After ths was fixed, it just deletes the last object *our*
+        # key.  After this was fixed, it just deletes the last object *our*
         # "for o in obj" loop would have gotten to.
         mutate = True
         count = 0
diff --git a/Misc/NEWS b/Misc/NEWS
index bc15f42..9c9e3f8 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,11 +12,19 @@ What's New in Python 2.3 beta 2?
 Core and builtins
 -----------------
 
-- SF bug 742860: WeakKeyDictionary __delitem__ uses iterkeys.  This
-  wasn't as threadsafe as it should be, was very inefficient, and could
-  raise RuntimeError if another thread mutated the dict during
-  __delitem__, or if a comparison function mutated it.  A new
-  implementation of WeakKeyDictionary.__delitem__ repairs all that.
+- SF bug 742860: "WeakKeyDictionary __delitem__ uses iterkeys".  This
+  wasn't threadsafe, was very inefficient (expected time O(len(dict))
+  instead of O(1)), and could raise a spurious RuntimeError if another
+  thread mutated the dict during __delitem__, or if a comparison function
+  mutated it.  It also neglected to raise KeyError when the key wasn't
+  present; didn't raise TypeError when the key wasn't of a weakly
+  referencable type; and broke various more-or-less obscure dict
+  invariants by using a sequence of equality comparisons over the whole
+  set of dict keys instead of computing the key's hash code to narrow
+  the search to those keys with the same hash code.  All of these are
+  considered to be bugs.  A new implementation of __delitem__ repairs all
+  that, but note that fixing these bugs may change visible behavior in
+  code relying (whether intentionally or accidentally) on old behavior.
 
 - SF bug 705231:  builtin pow() no longer lets the platform C pow()
   raise -1.0 to integer powers, because (at least) glibc gets it wrong