[Python-checkins] cpython (3.3): Issue #18594: Fix the fallback path in collections.Counter().

[raymond.hettinger]

http://hg.python.org/cpython/rev/1ee6f8a96fb9
changeset:   85916:1ee6f8a96fb9
branch:      3.3
parent:      85911:423736775f6b
user:        Raymond Hettinger <python at rcn.com>
date:        Tue Oct 01 21:36:09 2013 -0700
summary:
  Issue #18594:  Fix the fallback path in collections.Counter().

files:
  Misc/NEWS                    |   3 +-
  Modules/_collectionsmodule.c |  41 +++++++++++++----------
  2 files changed, 25 insertions(+), 19 deletions(-)


diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -79,7 +79,8 @@
   when necessary.  Patch by Oscar Benjamin.
 
 - Issue #18594: The fast path for collections.Counter() was never taken
-  due to an over-restrictive type check.
+  due to an over-restrictive type check.  And the fallback path did
+  not implement the same algorithm as the pure python code.
 
 - Properly initialize all fields of a SSL object after allocation.
 
diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c
--- a/Modules/_collectionsmodule.c
+++ b/Modules/_collectionsmodule.c
@@ -1694,7 +1694,9 @@
     PyObject *it, *iterable, *mapping, *oldval;
     PyObject *newval = NULL;
     PyObject *key = NULL;
+    PyObject *zero = NULL;
     PyObject *one = NULL;
+    PyObject *mapping_get = NULL;
     PyObject *mapping_getitem;
     PyObject *mapping_setitem;
     PyObject *dict_getitem;
@@ -1708,10 +1710,8 @@
         return NULL;
 
     one = PyLong_FromLong(1);
-    if (one == NULL) {
-        Py_DECREF(it);
-        return NULL;
-    }
+    if (one == NULL)
+        goto done;
 
     mapping_getitem = _PyType_LookupId(Py_TYPE(mapping), &PyId___getitem__);
     dict_getitem = _PyType_LookupId(&PyDict_Type, &PyId___getitem__);
@@ -1741,23 +1741,25 @@
             Py_DECREF(key);
         }
     } else {
+        mapping_get = PyObject_GetAttrString(mapping, "get");
+        if (mapping_get == NULL)
+            goto done;
+
+        zero = PyLong_FromLong(0);
+        if (zero == NULL)
+            goto done;
+
         while (1) {
             key = PyIter_Next(it);
             if (key == NULL)
                 break;
-            oldval = PyObject_GetItem(mapping, key);
-            if (oldval == NULL) {
-                if (!PyErr_Occurred() || !PyErr_ExceptionMatches(PyExc_KeyError))
-                    break;
-                PyErr_Clear();
-                Py_INCREF(one);
-                newval = one;
-            } else {
-                newval = PyNumber_Add(oldval, one);
-                Py_DECREF(oldval);
-                if (newval == NULL)
-                    break;
-            }
+            oldval = PyObject_CallFunctionObjArgs(mapping_get, key, zero, NULL);
+            if (oldval == NULL)
+                break;
+            newval = PyNumber_Add(oldval, one);
+            Py_DECREF(oldval);
+            if (newval == NULL)
+                break;
             if (PyObject_SetItem(mapping, key, newval) == -1)
                 break;
             Py_CLEAR(newval);
@@ -1765,10 +1767,13 @@
         }
     }
 
+done:
     Py_DECREF(it);
     Py_XDECREF(key);
     Py_XDECREF(newval);
-    Py_DECREF(one);
+    Py_XDECREF(mapping_get);
+    Py_XDECREF(zero);
+    Py_XDECREF(one);
     if (PyErr_Occurred())
         return NULL;
     Py_RETURN_NONE;

-- 
Repository URL: http://hg.python.org/cpython