fix MRO for multimethod.__call__ using iterative algorithm.

Add tests MRO for inherited multimethods with multiple inheritance
Add tests for inherited and overridden multimethods

2 files changed, 59 insertions, 9 deletions

diff --git a/lib/spack/spack/multimethod.py b/lib/spack/spack/multimethod.py
index 7682ca063f..b95c8f2341 100644
--- a/lib/spack/spack/multimethod.py
+++ b/lib/spack/spack/multimethod.py
@@ -25,6 +25,7 @@ depending on the scenario, regular old conditionals might be clearer,
 so package authors should use their judgement.
 """
 import functools
+import inspect
 
 from llnl.util.lang import caller_locals
 
@@ -107,14 +108,26 @@ class SpecMultiMethod(object):
             return self.default(package_self, *args, **kwargs)
 
         else:
-            superclass = super(package_self.__class__, package_self)
-            superclass_fn = getattr(superclass, self.__name__, None)
-            if callable(superclass_fn):
-                return superclass_fn(*args, **kwargs)
-            else:
-                raise NoSuchMethodError(
-                    type(package_self), self.__name__, spec,
-                    [m[0] for m in self.method_list])
+            # Unwrap MRO by hand because super binds to the subclass
+            # and causes infinite recursion for inherited methods
+            for cls in inspect.getmro(package_self.__class__)[1:]:
+                superself = cls.__dict__.get(self.__name__, None)
+                if isinstance(superself, self.__class__):
+                    # Parent class method is a multimethod
+                    # check it locally for methods, conditional or default
+                    # Do not recurse, that will mess up MRO
+                    for spec, method in superself.method_list:
+                        if package_self.spec.satisfies(spec):
+                            return method(package_self, *args, **kwargs)
+                    if superself.default:
+                        return superself.default(package_self, *args, **kwargs)
+                elif superself:
+                    return superself(package_self, *args, **kwargs)
+
+            raise NoSuchMethodError(
+                type(package_self), self.__name__, package_self.spec,
+                [m[0] for m in self.method_list]
+            )
 
     def __str__(self):
         return "SpecMultiMethod {\n\tdefault: %s,\n\tspecs: %s\n}" % (
diff --git a/lib/spack/spack/test/multimethod.py b/lib/spack/spack/test/multimethod.py
index b475b3ae93..35ecce7aab 100644
--- a/lib/spack/spack/test/multimethod.py
+++ b/lib/spack/spack/test/multimethod.py
@@ -117,7 +117,44 @@ def test_virtual_dep_match(pkg_name):
 
 def test_multimethod_with_base_class(pkg_name):
     pkg = spack.repo.get(pkg_name + '@3')
-    assert pkg.base_method() == "subclass_method"
+    assert pkg.base_method() == pkg.spec.name
 
     pkg = spack.repo.get(pkg_name + '@1')
     assert pkg.base_method() == "base_method"
+
+
+def test_multimethod_inherited_and_overridden():
+    pkg = spack.repo.get('multimethod-inheritor@1.0')
+    assert pkg.inherited_and_overridden() == 'inheritor@1.0'
+
+    pkg = spack.repo.get('multimethod-inheritor@2.0')
+    assert pkg.inherited_and_overridden() == 'base@2.0'
+
+    pkg = spack.repo.get('multimethod@1.0')
+    assert pkg.inherited_and_overridden() == 'base@1.0'
+
+    pkg = spack.repo.get('multimethod@2.0')
+    assert pkg.inherited_and_overridden() == 'base@2.0'
+
+
+def test_multimethod_diamond_inheritance():
+    pkg = spack.repo.get('multimethod-diamond@1.0')
+    assert pkg.diamond_inheritance() == 'base_package'
+
+    pkg = spack.repo.get('multimethod-base@1.0')
+    assert pkg.diamond_inheritance() == 'base_package'
+
+    pkg = spack.repo.get('multimethod-diamond@2.0')
+    assert pkg.diamond_inheritance() == 'first_parent'
+
+    pkg = spack.repo.get('multimethod-inheritor@2.0')
+    assert pkg.diamond_inheritance() == 'first_parent'
+
+    pkg = spack.repo.get('multimethod-diamond@3.0')
+    assert pkg.diamond_inheritance() == 'second_parent'
+
+    pkg = spack.repo.get('multimethod-diamond-parent@3.0')
+    assert pkg.diamond_inheritance() == 'second_parent'
+
+    pkg = spack.repo.get('multimethod-diamond@4.0')
+    assert pkg.diamond_inheritance() == 'subclass'