Add tests for comparing InterfaceClass/Implements objects to things w…

…ithout the required attributes.

And fix the C handling of this case.

CHANGES.rst

@@ -163,7 +163,7 @@
   registry lookup functions. See issue 11.
 
 - Use Python's standard C3 resolution order to compute the
-  ``__iro___`` and ``__sro___`` of interfaces. Previously, an ad-hoc
+  ``__iro__`` and ``__sro__`` of interfaces. Previously, an ad-hoc
   ordering that made no particular guarantees was used.
 
   This has many beneficial properties, including the fact that base
@@ -195,6 +195,18 @@
   the future). For details, see the documentation for
   ``zope.interface.ro``.
 
+- Implement sorting, equality, and hashing in C for ``Interface``
+  objects. In micro benchmarks, this makes those operations 40% to 80%
+  faster. This translates to a 20% speed up in querying adapters.
+
+  Note that this changes certain implementation details. In
+  particular, ``InterfaceClass`` now has a non-default metaclass, and
+  it is enforced that ``__module__`` in instances of
+  ``InterfaceClass`` is read-only.
+
+  See `PR 183 <https://github.com/zopefoundation/zope.interface/pull/183>`_.
+
+
 4.7.2 (2020-03-10)
 ==================
 

src/zope/interface/_zope_interface_coptimizations.c

@@ -895,21 +895,23 @@ IB_richcompare(IB* self, PyObject* other, int op)
     }
 
     if (PyObject_TypeCheck(other, &InterfaceBaseType)) {
+        // This branch borrows references. No need to clean
+        // up if otherib is not null.
         otherib = (IB*)other;
         othername = otherib->__name__;
         othermod = otherib->__module__;
     }
     else {
         othername = PyObject_GetAttrString(other, "__name__");
-        // TODO: Optimize this case.
-        if (othername == NULL) {
-            PyErr_Clear();
-            othername = PyNative_FromString("");
+        if (othername) {
+            othermod = PyObject_GetAttrString(other, "__module__");
         }
-        othermod = PyObject_GetAttrString(other, "__module__");
-        if (othermod == NULL) {
-            PyErr_Clear();
-            othermod = PyNative_FromString("");
+        if (!othername || !othermod) {
+            if (PyErr_Occurred() && PyErr_ExceptionMatches(PyExc_AttributeError)) {
+                PyErr_Clear();
+                oresult = Py_NotImplemented;
+            }
+            goto cleanup;
         }
     }
 #if 0
@@ -928,15 +930,17 @@ IB_richcompare(IB* self, PyObject* other, int op)
         result = PyObject_RichCompareBool(self->__module__, othermod, op);
     }
     // If either comparison failed, we have an error set.
+    // Leave oresult NULL so we raise it.
     if (result == -1) {
         goto cleanup;
     }
 
     oresult = result ? Py_True : Py_False;
-    Py_INCREF(oresult);
 
 
 cleanup:
+    Py_XINCREF(oresult);
+
     if (!otherib) {
         Py_XDECREF(othername);
         Py_XDECREF(othermod);

src/zope/interface/interface.py

@@ -155,6 +155,7 @@ def isOrExtends(self, interface):
 
     __call__ = isOrExtends
 
+
 class NameAndModuleComparisonMixin(object):
     # Internal use. Implement the basic sorting operators (but not (in)equality
     # or hashing). Subclasses must provide ``__name__`` and ``__module__``
@@ -168,6 +169,7 @@ class NameAndModuleComparisonMixin(object):
 
     # pylint:disable=assigning-non-slot
     __slots__ = ()
+
     def _compare(self, other):
         """
         Compare *self* to *other* based on ``__name__`` and ``__module__``.
@@ -179,6 +181,14 @@ def _compare(self, other):
         If *other* does not have ``__name__`` or ``__module__``, then
         return ``NotImplemented``.
 
+        .. caution::
+           This allows comparison to things well outside the type hierarchy,
+           perhaps not symmetrically.
+
+           For example, ``class Foo(object)`` and ``class Foo(Interface)``
+           in the same file would compare equal, depending on the order of
+           operands. Writing code like this by hand would be unusual, but it could
+           happen with dynamic creation of types and interfaces.
 
         None is treated as a pseudo interface that implies the loosest
         contact possible, no contract. For that reason, all interfaces

src/zope/interface/tests/test_declarations.py

@@ -17,6 +17,7 @@
 
 from zope.interface._compat import _skip_under_py3k
 from zope.interface.tests import OptimizationTestMixin
+from zope.interface.tests.test_interface import NameAndModuleComparisonTestsMixin
 
 
 class _Py3ClassAdvice(object):
@@ -296,7 +297,8 @@ def test_get_always_default(self):
         self.assertIsNone(self._getEmpty().get('name'))
         self.assertEqual(self._getEmpty().get('name', 42), 42)
 
-class TestImplements(unittest.TestCase):
+class TestImplements(NameAndModuleComparisonTestsMixin,
+                     unittest.TestCase):
 
     def _getTargetClass(self):
         from zope.interface.declarations import Implements
@@ -305,6 +307,13 @@ def _getTargetClass(self):
     def _makeOne(self, *args, **kw):
         return self._getTargetClass()(*args, **kw)
 
+    def _makeOneToCompare(self):
+        from zope.interface.declarations import implementedBy
+        class A(object):
+            pass
+
+        return implementedBy(A)
+
     def test_ctor_no_bases(self):
         impl = self._makeOne()
         self.assertEqual(impl.inherit, None)

src/zope/interface/tests/test_interface.py

@@ -229,7 +229,112 @@ def _providedBy(obj):
             self.assertTrue(sb.providedBy(object()))
 
 
-class InterfaceBaseTestsMixin(CleanUp):
+class NameAndModuleComparisonTestsMixin(CleanUp):
+
+    def _makeOneToCompare(self):
+        return self._makeOne('a', 'b')
+
+    def __check_NotImplemented_comparison(self, name):
+        # Without the correct attributes of __name__ and __module__,
+        # comparison switches to the reverse direction.
+
+        import operator
+        ib = self._makeOneToCompare()
+        op = getattr(operator, name)
+        meth = getattr(ib, '__%s__' % name)
+
+        # If either the __name__ or __module__ attribute
+        # is missing from the other object, then we return
+        # NotImplemented.
+        class RaisesErrorOnMissing(object):
+            Exc = AttributeError
+            def __getattribute__(self, name):
+                try:
+                    return object.__getattribute__(self, name)
+                except AttributeError:
+                    exc = RaisesErrorOnMissing.Exc
+                    raise exc(name)
+
+        class RaisesErrorOnModule(RaisesErrorOnMissing):
+            def __init__(self):
+                self.__name__ = 'foo'
+            @property
+            def __module__(self):
+                raise AttributeError
+
+        class RaisesErrorOnName(RaisesErrorOnMissing):
+            def __init__(self):
+                self.__module__ = 'foo'
+
+        self.assertEqual(RaisesErrorOnModule().__name__, 'foo')
+        self.assertEqual(RaisesErrorOnName().__module__, 'foo')
+        with self.assertRaises(AttributeError):
+            getattr(RaisesErrorOnModule(), '__module__')
+        with self.assertRaises(AttributeError):
+            getattr(RaisesErrorOnName(), '__name__')
+
+        for cls in RaisesErrorOnModule, RaisesErrorOnName:
+            self.assertIs(meth(cls()), NotImplemented)
+
+        # If the other object has a comparison function, returning
+        # NotImplemented means Python calls it.
+
+        class AllowsAnyComparison(RaisesErrorOnMissing):
+            def __eq__(self, other):
+                return True
+            __lt__ = __eq__
+            __le__ = __eq__
+            __gt__ = __eq__
+            __ge__ = __eq__
+            __ne__ = __eq__
+
+        self.assertTrue(op(ib, AllowsAnyComparison()))
+        self.assertIs(meth(AllowsAnyComparison()), NotImplemented)
+
+        # If it doesn't have the comparison, Python raises a TypeError.
+        class AllowsNoComparison(object):
+            __eq__ = None
+            __lt__ = __eq__
+            __le__ = __eq__
+            __gt__ = __eq__
+            __ge__ = __eq__
+            __ne__ = __eq__
+
+        self.assertIs(meth(AllowsNoComparison()), NotImplemented)
+        with self.assertRaises(TypeError):
+            op(ib, AllowsNoComparison())
+
+        # Errors besides AttributeError are passed
+        class MyException(Exception):
+            pass
+
+        RaisesErrorOnMissing.Exc = MyException
+
+        with self.assertRaises(MyException):
+            getattr(RaisesErrorOnModule(), '__module__')
+        with self.assertRaises(MyException):
+            getattr(RaisesErrorOnName(), '__name__')
+
+        for cls in RaisesErrorOnModule, RaisesErrorOnName:
+            with self.assertRaises(MyException):
+                op(ib, cls())
+            with self.assertRaises(MyException):
+                meth(cls())
+
+    def test__lt__NotImplemented(self):
+        self.__check_NotImplemented_comparison('lt')
+
+    def test__le__NotImplemented(self):
+        self.__check_NotImplemented_comparison('le')
+
+    def test__gt__NotImplemented(self):
+        self.__check_NotImplemented_comparison('gt')
+
+    def test__ge__NotImplemented(self):
+        self.__check_NotImplemented_comparison('ge')
+
+
+class InterfaceBaseTestsMixin(NameAndModuleComparisonTestsMixin):
     # Tests for both C and Python implementation
 
     def _getTargetClass(self):
@@ -240,21 +345,21 @@ def _getFallbackClass(self):
         from zope.interface.interface import InterfaceBasePy
         return InterfaceBasePy
 
-    def _makeOne(self, object_should_provide):
+    def _makeOne(self, object_should_provide=False, name=None, module=None):
         class IB(self._getTargetClass()):
             def _call_conform(self, conform):
                 return conform(self)
             def providedBy(self, obj):
                 return object_should_provide
-        return IB()
+        return IB(name, module)
 
     def test___call___w___conform___returning_value(self):
         ib = self._makeOne(False)
         conformed = object()
         class _Adapted(object):
             def __conform__(self, iface):
                 return conformed
-        self.assertTrue(ib(_Adapted()) is conformed)
+        self.assertIs(ib(_Adapted()), conformed)
 
     def test___call___wo___conform___ob_no_provides_w_alternate(self):
         ib = self._makeOne(False)