Merge 18e7d4a into f6678f5

.github/workflows/tests.yml

@@ -177,6 +177,8 @@ jobs:
     needs: build-package
     runs-on: ${{ matrix.os }}
     strategy:
+      # We want to see all failures:
+      fail-fast: false
       matrix:
         python-version:
           - 2.7
@@ -188,6 +190,7 @@ jobs:
           - 3.8
           - 3.9
         os: [ubuntu-20.04, macos-latest]
+        c3: [0, 1]
         exclude:
           - os: macos-latest
             python-version: pypy-2.7
@@ -239,9 +242,13 @@ jobs:
           pip install -U -e .[test]
       - name: Run tests with C extensions
         if: ${{ !startsWith(matrix.python-version, 'pypy') }}
+        env:
+          EXTENSIONCLASS_C3_MRO: ${{ matrix.c3 }}
         run: |
           python -m coverage run -p -m zope.testrunner --test-path=src --auto-color --auto-progress
       - name: Run tests without C extensions
+        env:
+          EXTENSIONCLASS_C3_MRO: ${{ matrix.c3 }}
         run:
           # coverage makes PyPy run about 3x slower!
           PURE_PYTHON=1 python -m coverage run -p -m zope.testrunner --test-path=src --auto-color --auto-progress

.meta.toml

@@ -2,7 +2,7 @@
 # https://github.com/zopefoundation/meta/tree/master/config/c-code
 [meta]
 template = "c-code"
-commit-id = "3cabefa1ea66c0536869c1e82ce5c94cdbff5f34"
+commit-id = "ce405f15dc740bf7cebc3ffe0fd800a633012fb1"
 
 [python]
 with-appveyor = true
@@ -14,6 +14,17 @@ with-sphinx-doctests = false
 
 [tox]
 use-flake8 = true
+additional-envlist = [
+    "py27-c3",
+    "py35-c3",
+    "py36-c3",
+    "py37-c3",
+    "py38-c3",
+    "py39-c3",
+    ]
+testenv-setenv = [
+    "c3: EXTENSIONCLASS_C3_MRO=1",
+    ]
 
 [coverage]
 fail-under = 100

CHANGES.rst

@@ -5,7 +5,11 @@
 4.5.2 (unreleased)
 ==================
 
-- Nothing changed yet.
+- Let the envvar ``EXTENSIONCLASS_C3_MRO`` control whether ``ExtensionClass``
+  uses Python's new C3 (Python 2.2+) or the old method resolution order.
+
+- Fix
+  `#33 <https://github.com/zopefoundation/ExtensionClass/issues/33>`_.
 
 
 4.5.1 (2021-06-11)

src/ExtensionClass/_ExtensionClass.c

@@ -451,8 +451,8 @@ _is_bad_setattr_name(PyTypeObject* type, PyObject* as_bytes)
                 PyExc_TypeError,
                 "can't set attributes of built-in/extension type '%s' if the "
                 "attribute name begins and ends with __ and contains only "
-                "4 _ characters",
-                type->tp_name
+                "4 _ characters: %s",
+                type->tp_name, cname
             );
             return 1;
         }
@@ -587,7 +587,7 @@ copy_classic(PyObject *base, PyObject *result)
 static PyObject *
 mro(PyTypeObject *self)
 {
-  /* Compute an MRO for a class */
+  /* Compute a classic MRO for a class */
   PyObject *result, *base, *basemro, *mro=NULL;
   int i, l, err;
 
@@ -643,6 +643,48 @@ mro(PyTypeObject *self)
   return mro;
 }
 
+static PyObject *
+mro_C3(PyTypeObject *self)
+{
+  /* Compute a C3 MRO for a class */
+  PyObject *mro = NULL;
+  PyObject *result = NULL;
+  PyObject *b = NULL; /* borrowed reference */
+  Py_ssize_t i, n;
+  mro = PyObject_GetAttrString((PyObject *) &PyType_Type, "mro");
+  if (mro == NULL)
+    goto end;
+  result = PyObject_CallFunctionObjArgs(mro, (PyObject *)self, NULL);
+  if (result == NULL)
+    goto end;
+  n = PyList_GET_SIZE(result);
+  b = PyList_GetItem(result, n - 2);
+  if (b == NULL)
+    goto cleanup;
+  if (b == (PyObject *) &BaseType)
+    goto end;
+  for (i = 0; i < n - 2; i++) {
+    if (PyList_GET_ITEM(result, i) == (PyObject *) &BaseType)
+      break;
+  }
+  if (i == n - 2) {
+    PyErr_SetString(PyExc_SystemError, "mro does not contain `Base`");
+    goto cleanup;
+  }
+  for (; i < n - 2; i++) {
+    PyList_SET_ITEM(result, i, PyList_GET_ITEM(result, i + 1));
+  }
+  PyList_SET_ITEM(result, n - 2, (PyObject *) &BaseType);
+  goto end;
+
+ cleanup:
+  Py_XDECREF(result); result = NULL;
+ end:
+  Py_XDECREF(mro);
+  return result;
+}
+
+
 static struct PyMethodDef EC_methods[] = {
   {"__basicnew__", (PyCFunction)__basicnew__, METH_NOARGS, 
    "Create a new empty object"},
@@ -1038,6 +1080,28 @@ module_init(void)
                          (PyObject *)&NoInstanceDictionaryBaseType) < 0)
       return NULL;
 
+
+  {
+    /* switch to `mro_C3` */
+    PyObject *ec = NULL, *C3_MRO = NULL;
+    ec = PyImport_ImportModule("ExtensionClass");
+    if (ec == NULL)
+      return NULL;
+    C3_MRO = PyObject_GetAttrString(ec, "_C3_MRO");
+    if (C3_MRO == NULL)
+      return NULL;
+    if (INT_AS_LONG(C3_MRO)) {
+      PyMethodDef *d;
+      for (d = EC_methods; d->ml_name; d++) {
+	if (strcmp(d->ml_name, "mro") == 0) {
+	  d->ml_meth = (PyCFunction) mro_C3;
+	  break;
+	}
+      }
+    }
+    Py_DECREF(ec); Py_DECREF(C3_MRO);
+  }
+
   return m;
 }
 

src/ExtensionClass/__init__.py

@@ -105,11 +105,14 @@ class init called
 
 if sys.version_info > (3, ):
     import copyreg as copy_reg
+    PY3 = True
 else:  # pragma: no cover
     import copy_reg
+    PY3 = False
 
 _IS_PYPY = platform.python_implementation() == 'PyPy'
 _IS_PURE = int(os.environ.get('PURE_PYTHON', '0'))
+_C3_MRO = int(os.environ.get('EXTENSIONCLASS_C3_MRO', '0'))
 C_EXTENSION = not (_IS_PYPY or _IS_PURE)
 
 
@@ -164,7 +167,7 @@ def __new__(cls, name, bases=(), attrs=None):
         cls = type.__new__(cls, name, bases, attrs)
 
         # Inherit docstring
-        if not cls.__doc__:
+        if cls.__doc__ is None:
             cls.__doc__ = super(cls, cls).__doc__
 
         # set up __get__ if __of__ is implemented
@@ -182,41 +185,56 @@ def __basicnew__(self):
         """Create a new empty object"""
         return self.__new__(self)
 
-    def mro(self):
-        """Compute an mro using the 'encapsulated base' scheme"""
-        mro = [self]
-        for base in self.__bases__:
-            if hasattr(base, '__mro__'):
-                for c in base.__mro__:
-                    if c in (BasePy, NoInstanceDictionaryBasePy, object):
-                        continue
-                    if c in mro:
-                        continue
-                    mro.append(c)
-            else:  # pragma: no cover (python 2 only)
-                _add_classic_mro(mro, base)
-
-        if NoInstanceDictionaryBasePy in self.__bases__:
-            mro.append(NoInstanceDictionaryBasePy)
-        elif self.__name__ != 'Base':
-            mro.append(BasePy)
-        mro.append(object)
-        return mro
+    if _C3_MRO:
+        def mro(self):
+            mro = type.mro(self)
+            # ensure `BasePy` is at location `-2`
+            if mro[-2] is not BasePy and (
+                BasePy.__name__ != "dummy"  # initialized
+                ):
+                mro.remove(BasePy)
+                mro[-1:-1] = BasePy,
+            return mro
+    else:
+        def mro(self):
+            """Compute an mro using the 'encapsulated base' scheme"""
+            mro = [self]
+            for base in self.__bases__:
+                if hasattr(base, '__mro__'):
+                    for c in base.__mro__:
+                        if c in (BasePy, NoInstanceDictionaryBasePy, object):
+                            continue
+                        if c in mro:
+                            continue
+                        mro.append(c)
+                else:  # pragma: no cover (python 2 only)
+                    _add_classic_mro(mro, base)
+
+            if NoInstanceDictionaryBasePy in self.__bases__:
+                mro.append(NoInstanceDictionaryBasePy)
+            elif self.__name__ != 'Base':
+                mro.append(BasePy)
+            mro.append(object)
+            return mro
 
     def inheritedAttribute(self, name):
         """Look up an inherited attribute"""
         return getattr(super(self, self), name)
 
-    def __setattr__(self, name, value):
-        if name not in ('__get__', '__doc__', '__of__'):
-            if (name.startswith('__') and name.endswith('__') and
-                    name.count('_') == 4):
-                raise TypeError(
-                    "can't set attributes of built-in/extension type '%s.%s' "
-                    "if the attribute name begins and ends with __ and "
-                    "contains only 4 _ characters" %
-                    (self.__module__, self.__name__))
-        return type.__setattr__(self, name, value)
+    # The following should emulate `_ExtensionClass.c:EC_setattr`:
+    #   prevent setting attributes looking like slots on **built-in** types
+    #   However, it prevents this generally.
+    #   Eliminate for the moment.
+##    def __setattr__(self, name, value):
+##        if name not in ('__get__', '__doc__', '__of__'):
+##            if (name.startswith('__') and name.endswith('__') and
+##                    name.count('_') == 4):
+##                raise TypeError(
+##                    "can't set attributes of built-in/extension type '%s.%s' "
+##                    "if the attribute name begins and ends with __ and "
+##                    "contains only 4 _ characters: %s" %
+##                    (self.__module__, self.__name__, name))
+##        return type.__setattr__(self, name, value)
 
 
 # Base and object are always moved to the last two positions

src/ExtensionClass/tests.py

@@ -17,8 +17,12 @@
 import sys
 import unittest
 
+from ExtensionClass import _C3_MRO as c3_mro
 from ExtensionClass import Base
+from ExtensionClass import BasePy
 from ExtensionClass import ExtensionClass
+from ExtensionClass import ExtensionClass
+from ExtensionClass import PY3
 
 
 def print_dict(d):
@@ -479,6 +483,22 @@ def test_setattr_on_extension_type():
     1
     0
 
+    """
+
+if Base is not BasePy:
+    # The C implementation does not allow to set some attributes
+    # (those whose name resembles special method names)
+    # on built in types (such as ``Base``).
+    # However, it is difficult for a Python implementation
+    # to determine for which classes the C implementation
+    # would impose this restriction.
+    # Formerly, the Python implementation forbade it generally
+    # (and not only on built in types). This causes problems
+    # with ``AccessControl`` ``__roles__``.
+    # For the moment, the Python implementation does not
+    # impose this restriction at all.
+    # We extend the test here for the C implementation.
+    test_setattr_on_extension_type.__doc__ += """
     >>> Base.__foo__ = 1  # doctest: +IGNORE_EXCEPTION_DETAIL
     Traceback (most recent call last):
     ...
@@ -645,6 +665,11 @@ def test_mro():
        """'ED', 'EB', 'EA', 'EC', 'ND', 'NB', 'NC', 'NA', 'Base', 'object']
     """
 
+if c3_mro:
+    # we would like to use ``unittest.skip`` but
+    # unfortunately, this does not work with doctests
+    del test_mro
+
 
 def test_avoiding___init__decoy_w_inheritedAttribute():
     """
@@ -1091,6 +1116,36 @@ def test_class_attributes(self):
         self.__check_class_attribute('nothing_special', None)
         self.__check_class_attribute('nothing_special', 'not-none')
 
+    def test_mro(self):
+        Base = self._getTargetClass()
+        class A1(Base):
+            def f(self):
+                return "A1"
+        class A2:
+            def f(self):
+                return "A2"
+        class B1(A1, A2):
+            pass
+        class B2(A2, A1):
+            pass
+        class B3(A2, Base):
+            pass
+        self.assertEqual(B1().f(), "A1")
+        self.assertEqual(B2().f(), "A2")
+        if c3_mro:
+            with self.assertRaises(TypeError):
+                class C(B1, B2):
+                    pass
+        else:
+            class C(B1, B2):
+                pass
+            self.assertIs(C.__mro__[-2], Base)
+            self.assertEqual(C().f(), "A1")
+        if PY3:
+            class C(B1, B3):
+                pass
+            self.assertIs(C.__mro__[-2], Base)
+
 
 class TestBasePy(TestBase):
 

tox.ini

@@ -13,13 +13,20 @@ envlist =
     pypy
     pypy3
     coverage
+    py27-c3
+    py35-c3
+    py36-c3
+    py37-c3
+    py38-c3
+    py39-c3
 
 [testenv]
 usedevelop = true
 deps =
 setenv =
     pure: PURE_PYTHON=1
     !pure-!pypy-!pypy3: PURE_PYTHON=0
+    c3: EXTENSIONCLASS_C3_MRO=1
 commands =
     zope-testrunner --test-path=src {posargs:-vc}
 extras =