Use C3 (mostly) to compute IRO.

Fixes #21

The 'mostly' is because interfaces are used in cases that C3 forbids; when there's a conflict, we move forward by adding the simple legacy base to the list at the conflicting location and continuing.

I hoped the fix for #8 might shake out automatically, but it didn't.

CHANGES.rst

@@ -5,6 +5,16 @@
 5.0.0 (unreleased)
 ==================
 
+- Adopt Python's standard `C3 resolution order
+  <https://www.python.org/download/releases/2.3/mro/>`_ for interface
+  linearization, with tweaks to support additional cases that are
+  common in interfaces but disallowed for Python classes.
+
+  In complex multiple-inheritance like scenerios, this may change the
+  interface resolution order, resulting in finding different adapters.
+  However, the results should make more sense. See `issue 21
+  <https://github.com/zopefoundation/zope.interface/issues/21>`_.
+
 - Make an internal singleton object returned by APIs like
   ``implementedBy`` and ``directlyProvidedBy`` immutable. Previously,
   it was fully mutable and allowed changing its ``__bases___``. That

docs/api/declarations.rst

@@ -592,7 +592,7 @@ Exmples for :meth:`Declaration.flattened`:
    >>> spec = Declaration(I4, spec)
    >>> i = spec.flattened()
    >>> [x.getName() for x in i]
-   ['I4', 'I2', 'I1', 'I3', 'Interface']
+   ['I4', 'I2', 'I3', 'I1', 'Interface']
    >>> list(i)
    []
 

src/zope/interface/declarations.py

@@ -462,18 +462,17 @@ def classImplements(cls, *interfaces):
 
     # compute the bases
     bases = []
-    seen = {}
+    seen = set()
     for b in spec.declared:
         if b not in seen:
-            seen[b] = 1
+            seen.add(b)
             bases.append(b)
 
     if spec.inherit is not None:
-
         for c in spec.inherit.__bases__:
             b = implementedBy(c)
             if b not in seen:
-                seen[b] = 1
+                seen.add(b)
                 bases.append(b)
 
     spec.__bases__ = tuple(bases)

src/zope/interface/ro.py

@@ -11,15 +11,22 @@
 # FOR A PARTICULAR PURPOSE.
 #
 ##############################################################################
-"""Compute a resolution order for an object and its bases
+"""
+Compute a resolution order for an object and its bases.
+
+.. versionchanged:: 5.0
+   The resolution order is now based on the same C3 order that Python
+   uses for classes. In complex instances of multiple inheritance, this
+   may result in a different ordering.
 """
 __docformat__ = 'restructuredtext'
 
 __all__ = [
     'ro',
 ]
 
-def _mergeOrderings(orderings):
+
+def _legacy_mergeOrderings(orderings):
     """Merge multiple orderings so that within-ordering order is preserved
 
     Orderings are constrained in such a way that if an object appears
@@ -38,17 +45,17 @@ def _mergeOrderings(orderings):
 
     """
 
-    seen = {}
+    seen = set()
     result = []
     for ordering in reversed(orderings):
         for o in reversed(ordering):
             if o not in seen:
-                seen[o] = 1
+                seen.add(o)
                 result.insert(0, o)
 
     return result
 
-def _flatten(ob):
+def _legacy_flatten(ob):
     result = [ob]
     i = 0
     for ob in iter(result):
@@ -61,8 +68,109 @@ def _flatten(ob):
         result[i:i] = ob.__bases__
     return result
 
+def _legacy_ro(ob):
+    return _legacy_mergeOrderings([_legacy_flatten(ob)])
 
-def ro(object):
-    """Compute a "resolution order" for an object
-    """
-    return _mergeOrderings([_flatten(object)])
+###
+# Compare base objects using identity, not equality. This matches what
+# the CPython MRO algorithm does, and is *much* faster to boot: that,
+# plus some other small tweaks makes the difference between 25s and 6s
+# in loading 446 plone/zope interface.py modules (1925 InterfaceClass,
+# 1200 Implements, 1100 ClassProvides objects)
+###
+
+def _can_choose_base(base, base_tree_remaining):
+    for bases in base_tree_remaining:
+        if not bases or bases[0] is base:
+            continue
+        for b in bases:
+            if b is base:
+                return False
+    return True
+
+def _find_next_base(base_tree_remaining):
+    base = None
+    for bases in base_tree_remaining:
+        base = bases[0]
+        can_choose_base = _can_choose_base(base, base_tree_remaining)
+        if can_choose_base:
+            return base
+        # Narf. We have an inconsistent order. Python cannot create
+        # classes like this:
+        #
+        # class B1:
+        #   pass
+        # class B2(B1):
+        #   pass
+        # class C(B1, B2): # -> TypeError
+        #  pass
+        #
+        # However, older versions of zope.interface were fine with this order.
+        # A good example is ``providedBy(IOError())``. Because of the way
+        # ``classImplements`` works, it winds up with ``__bases__`` ==
+        # ``[IEnvironmentError, IIOError, IOSError, <implementedBy Exception>]``
+        # (on Python 3). But ``IEnvironmentError`` is a base of both ``IIOError``
+        # and ``IOSError``. Previously, we would get a resolution order of
+        # ``[IIOError, IOSError, IEnvironmentError, IStandardError, IException, Interface]``
+        # but the standard Python algorithm would forbid creating that order entirely.
+        base = None
+
+    return base
+
+def _merge(C, base_tree, memo):
+    if C in memo:
+        return memo[C]
+
+    result = []
+    base_tree_remaining = base_tree
+    legacy_ro = None
+    legacy_ro_it = None
+
+    while 1:
+        base_tree_remaining = [bases for bases in base_tree_remaining if bases]
+        if not base_tree_remaining:
+            result = tuple(result)
+            memo[C] = result
+            return result
+
+        base = _find_next_base(base_tree_remaining)
+        if base is None:
+            # We couldn't do it. So find the next base from the *legacy* RO that's not present
+            # so far and go with it. This is surprisingly common; in addition to clear errors,
+            # it also happens when you @implementer(ISomeIFace) on a class that already
+            # *inherits* ISomeIFace
+            if legacy_ro is None:
+                legacy_ro = _legacy_ro(C)
+                # We should only need to go through the legacy RO once from
+                # beginning to end to see all possible bases. Save the iterator state
+                # to ensure that we dont start over looking at already examined bases.
+                legacy_ro_it = iter(legacy_ro)
+            for c in legacy_ro_it:
+                if not [x for x in result if x is c]:
+                    base = c
+                    break
+            else:
+                memo[C] = legacy_ro
+                return legacy_ro
+
+        result.append(base)
+
+        # Take it out of the base tree wherever it is.
+        # This differs from the standard Python MRO and is needed
+        # because we have no other step that prevents duplicates
+        # from coming in (e.g., in the legacy path)
+        base_tree_remaining = [
+            [b for b in bases if b is not base]
+            for bases
+            in base_tree_remaining
+        ]
+
+def ro(C, _memo=None):
+    "Compute the precedence list (mro) according to C3"
+    memo = _memo if _memo is not None else {}
+    result = _merge(
+        C,
+        [[C]] + [ro(base, memo) for base in C.__bases__] + [list(C.__bases__)],
+        memo,
+    )
+    return list(result)

src/zope/interface/tests/test_declarations.py

@@ -191,9 +191,13 @@ def test_flattened_w_nested_sequence_overlap(self):
         from zope.interface.interface import InterfaceClass
         IFoo = InterfaceClass('IFoo')
         IBar = InterfaceClass('IBar')
+        # This is the same as calling ``Declaration(IBar, IFoo, IBar)``
+        # which doesn't make much sense, but here it is. In older
+        # versions of zope.interface, the __iro__ would have been
+        # IFoo, IBar, Interface, which especially makes no sense.
         decl = self._makeOne(IBar, (IFoo, IBar))
         # Note that decl.__iro__ has IFoo first.
-        self.assertEqual(list(decl.flattened()), [IFoo, IBar, Interface])
+        self.assertEqual(list(decl.flattened()), [IBar, IFoo, Interface])
 
     def test___sub___unrelated_interface(self):
         from zope.interface.interface import InterfaceClass

src/zope/interface/tests/test_ro.py

@@ -18,8 +18,8 @@
 class Test__mergeOrderings(unittest.TestCase):
 
     def _callFUT(self, orderings):
-        from zope.interface.ro import _mergeOrderings
-        return _mergeOrderings(orderings)
+        from zope.interface.ro import _legacy_mergeOrderings
+        return _legacy_mergeOrderings(orderings)
 
     def test_empty(self):
         self.assertEqual(self._callFUT([]), [])
@@ -30,7 +30,7 @@ def test_single(self):
     def test_w_duplicates(self):
         self.assertEqual(self._callFUT([['a'], ['b', 'a']]), ['b', 'a'])
 
-    def test_suffix_across_multiple_duplicats(self):
+    def test_suffix_across_multiple_duplicates(self):
         O1 = ['x', 'y', 'z']
         O2 = ['q', 'z']
         O3 = [1, 3, 5]
@@ -42,8 +42,8 @@ def test_suffix_across_multiple_duplicats(self):
 class Test__flatten(unittest.TestCase):
 
     def _callFUT(self, ob):
-        from zope.interface.ro import _flatten
-        return _flatten(ob)
+        from zope.interface.ro import _legacy_flatten
+        return _legacy_flatten(ob)
 
     def test_w_empty_bases(self):
         class Foo(object):
@@ -80,8 +80,8 @@ class Qux(Bar, Baz):
 class Test_ro(unittest.TestCase):
 
     def _callFUT(self, ob):
-        from zope.interface.ro import ro
-        return ro(ob)
+        from zope.interface.ro import _legacy_ro
+        return _legacy_ro(ob)
 
     def test_w_empty_bases(self):
         class Foo(object):
@@ -113,3 +113,133 @@ class Qux(Bar, Baz):
             pass
         self.assertEqual(self._callFUT(Qux),
                          [Qux, Bar, Baz, Foo, object])
+
+    def _make_IOErr(self):
+        # This can't be done in the standard C3 ordering.
+        class Foo(object):
+            def __init__(self, name, *bases):
+                self.__name__ = name
+                self.__bases__ = bases
+            def __repr__(self): # pragma: no cover
+                return self.__name__
+
+        # Mimic what classImplements(IOError, IIOError)
+        # does.
+        IEx = Foo('IEx')
+        IStdErr = Foo('IStdErr', IEx)
+        IEnvErr = Foo('IEnvErr', IStdErr)
+        IIOErr = Foo('IIOErr', IEnvErr)
+        IOSErr = Foo('IOSErr', IEnvErr)
+
+        IOErr = Foo('IOErr', IEnvErr, IIOErr, IOSErr)
+        return IOErr, [IOErr, IIOErr, IOSErr, IEnvErr, IStdErr, IEx]
+
+    def test_non_orderable(self):
+        IOErr, bases = self._make_IOErr()
+
+        self.assertEqual(self._callFUT(IOErr), bases)
+
+    def test_mixed_inheritance_and_implementation(self):
+        # https://github.com/zopefoundation/zope.interface/issues/8
+        # This test should fail, but doesn't, as described in that issue.
+        from zope.interface import implementer
+        from zope.interface import Interface
+        from zope.interface import providedBy
+        from zope.interface import implementedBy
+
+        class IFoo(Interface):
+            pass
+
+        @implementer(IFoo)
+        class ImplementsFoo(object):
+            pass
+
+        class ExtendsFoo(ImplementsFoo):
+            pass
+
+        class ImplementsNothing(object):
+            pass
+
+        class ExtendsFooImplementsNothing(ExtendsFoo, ImplementsNothing):
+            pass
+
+        self.assertEqual(
+            self._callFUT(providedBy(ExtendsFooImplementsNothing())),
+            [implementedBy(ExtendsFooImplementsNothing),
+             implementedBy(ExtendsFoo),
+             implementedBy(ImplementsFoo),
+             IFoo,
+             Interface,
+             implementedBy(ImplementsNothing),
+             implementedBy(object)])
+
+
+class Test_c3_ro(Test_ro):
+    def _callFUT(self, ob):
+        from zope.interface.ro import ro
+        return ro(ob)
+
+    def test_complex_diamond(self, base=object):
+        # https://github.com/zopefoundation/zope.interface/issues/21
+        O = base
+        class F(O):
+            pass
+        class E(O):
+            pass
+        class D(O):
+            pass
+        class C(D, F):
+            pass
+        class B(D, E):
+            pass
+        class A(B, C):
+            pass
+
+        if hasattr(A, 'mro'):
+            self.assertEqual(A.mro(), self._callFUT(A))
+
+        return A
+
+    def test_complex_diamond_interface(self):
+        from zope.interface import Interface
+
+        IA = self.test_complex_diamond(Interface)
+
+        self.assertEqual(
+            [x.__name__ for x in IA.__iro__],
+            ['A', 'B', 'C', 'D', 'E', 'F', 'Interface']
+        )
+
+    def test_non_orderable(self):
+        from zope.interface import ro
+        legacy = ro._legacy_ro
+        called_legacy = []
+        def _legacy_ro(o):
+            called_legacy.append(1)
+            return legacy(o)
+
+        ro._legacy_ro = _legacy_ro
+        try:
+            super(Test_c3_ro, self).test_non_orderable()
+        finally:
+            ro._legacy_ro = legacy
+
+        self.assertTrue(called_legacy)
+
+    def test_non_orderable_legacy_ro_weird_return(self):
+        # If legacy_ro returns bases we've already seen,
+        # or nothing to iterate on at all, that's exactly what we return.
+        from zope.interface import ro
+        legacy = ro._legacy_ro
+        called_legacy = []
+        def _legacy_ro(o):
+            called_legacy.append(1)
+            return ()
+
+        IOErr, _ = self._make_IOErr()
+        ro._legacy_ro = _legacy_ro
+        try:
+            self.assertEqual([], self._callFUT(IOErr))
+        finally:
+            ro._legacy_ro = legacy
+        self.assertTrue(called_legacy)