Update comments and add a test for more coverage.

6 files changed, 108 insertions, 54 deletions

diff --git a/.gitignore b/.gitignore
index 35946f4..cbf117f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,5 +1,6 @@
 *.egg-info
 *.pyc
+*.pyo
 *.so
 __pycache__
 .coverage
diff --git a/benchmarks/.gitignore b/benchmarks/.gitignore
new file mode 100644
index 0000000..a6c57f5
--- /dev/null
+++ b/benchmarks/.gitignore
@@ -0,0 +1 @@
+*.json
diff --git a/src/zope/interface/declarations.py b/src/zope/interface/declarations.py
index 502e26e..d85dbf4 100644
--- a/src/zope/interface/declarations.py
+++ b/src/zope/interface/declarations.py
@@ -46,6 +46,8 @@ __all__ = [
     # re-exported from zope.interface directly.
 ]
 
+# pylint:disable=too-many-lines
+
 # Registry of class-implementation specifications
 BuiltinImplementationSpecifications = {}
 
@@ -102,12 +104,14 @@ class Declaration(Specification):
     def __sub__(self, other):
         """Remove interfaces from a specification
         """
-        return Declaration(
-            *[i for i in self.interfaces()
-              if not [j for j in other.interfaces()
-                      if i.extends(j, 0)]
+        return Declaration(*[
+            i for i in self.interfaces()
+            if not [
+                j
+                for j in other.interfaces()
+                if i.extends(j, 0) # non-strict extends
             ]
-        )
+        ])
 
     def __add__(self, other):
         """Add two specifications or a specification and an interface
@@ -229,7 +233,9 @@ class Implements(NameAndModuleComparisonMixin,
     declared = ()
 
     # Weak cache of {class: <implements>} for super objects.
-    # Created on demand.
+    # Created on demand. These are rare, as of 5.0 anyway. Using a class
+    # level default doesn't take space in instances. Using _v_attrs would be
+    # another place to store this without taking space unless needed.
     _super_cache = None
 
     __name__ = '?'
@@ -247,7 +253,10 @@ class Implements(NameAndModuleComparisonMixin,
         return inst
 
     def changed(self, originally_changed):
-        self._super_cache = None
+        try:
+            del self._super_cache
+        except AttributeError:
+            pass
         return super(Implements, self).changed(originally_changed)
 
     def __repr__(self):
@@ -283,7 +292,7 @@ def _implementedBy_super(sup):
     # that excludes the classes being skipped over but
     # includes everything else.
     implemented_by_self = implementedBy(sup.__self_class__)
-    cache = implemented_by_self._super_cache
+    cache = implemented_by_self._super_cache # pylint:disable=protected-access
     if cache is None:
         cache = implemented_by_self._super_cache = weakref.WeakKeyDictionary()
 
@@ -318,7 +327,7 @@ def _implementedBy_super(sup):
 
 
 @_use_c_impl
-def implementedBy(cls):
+def implementedBy(cls): # pylint:disable=too-many-return-statements,too-many-branches
     """Return the interfaces implemented for a class' instances
 
       The value returned is an `~zope.interface.interfaces.IDeclaration`.
@@ -393,8 +402,7 @@ def implementedBy(cls):
             cls.__providedBy__ = objectSpecificationDescriptor
 
         if (isinstance(cls, DescriptorAwareMetaClasses)
-            and
-            '__provides__' not in cls.__dict__):
+                and '__provides__' not in cls.__dict__):
             # Make sure we get a __provides__ descriptor
             cls.__provides__ = ClassProvides(
                 cls,
@@ -499,9 +507,9 @@ def _classImplements_ordered(spec, before=(), after=()):
 
 
 def _implements_advice(cls):
-    interfaces, classImplements = cls.__dict__['__implements_advice_data__']
+    interfaces, do_classImplements = cls.__dict__['__implements_advice_data__']
     del cls.__implements_advice_data__
-    classImplements(cls, *interfaces)
+    do_classImplements(cls, *interfaces)
     return cls
 
 
@@ -533,6 +541,7 @@ class implementer(object):
 
       after the class has been created.
       """
+    __slots__ = ('interfaces',)
 
     def __init__(self, *interfaces):
         self.interfaces = interfaces
@@ -583,7 +592,7 @@ class implementer_only(object):
         if isinstance(ob, (FunctionType, MethodType)):
             # XXX Does this decorator make sense for anything but classes?
             # I don't think so. There can be no inheritance of interfaces
-            # on a method pr function....
+            # on a method or function....
             raise ValueError('The implementer_only decorator is not '
                              'supported for methods or functions.')
         else:
@@ -591,11 +600,11 @@ class implementer_only(object):
             classImplementsOnly(ob, *self.interfaces)
             return ob
 
-def _implements(name, interfaces, classImplements):
+def _implements(name, interfaces, do_classImplements):
     # This entire approach is invalid under Py3K.  Don't even try to fix
     # the coverage for this block there. :(
-    frame = sys._getframe(2)
-    locals = frame.f_locals
+    frame = sys._getframe(2) # pylint:disable=protected-access
+    locals = frame.f_locals # pylint:disable=redefined-builtin
 
     # Try to make sure we were called from a class def. In 2.2.0 we can't
     # check for __module__ since it doesn't seem to be added to the locals
@@ -606,7 +615,7 @@ def _implements(name, interfaces, classImplements):
     if '__implements_advice_data__' in locals:
         raise TypeError(name+" can be used only once in a class definition.")
 
-    locals['__implements_advice_data__'] = interfaces, classImplements
+    locals['__implements_advice_data__'] = interfaces, do_classImplements
     addClassAdvisor(_implements_advice, depth=3)
 
 def implements(*interfaces):
@@ -713,7 +722,7 @@ ProvidesClass = Provides
 # This is a memory optimization to allow objects to share specifications.
 InstanceDeclarations = weakref.WeakValueDictionary()
 
-def Provides(*interfaces):
+def Provides(*interfaces): # pylint:disable=function-redefined
     """Cache instance declarations
 
       Instance declarations are shared among instances that have the same
@@ -729,7 +738,7 @@ def Provides(*interfaces):
 Provides.__safe_for_unpickling__ = True
 
 
-def directlyProvides(object, *interfaces):
+def directlyProvides(object, *interfaces): # pylint:disable=redefined-builtin
     """Declare interfaces declared directly for an object
 
       The arguments after the object are one or more interfaces or interface
@@ -765,7 +774,7 @@ def directlyProvides(object, *interfaces):
         object.__provides__ = Provides(cls, *interfaces)
 
 
-def alsoProvides(object, *interfaces):
+def alsoProvides(object, *interfaces): # pylint:disable=redefined-builtin
     """Declare interfaces declared directly for an object
 
     The arguments after the object are one or more interfaces or interface
@@ -777,7 +786,7 @@ def alsoProvides(object, *interfaces):
     directlyProvides(object, directlyProvidedBy(object), *interfaces)
 
 
-def noLongerProvides(object, interface):
+def noLongerProvides(object, interface): # pylint:disable=redefined-builtin
     """ Removes a directly provided interface from an object.
     """
     directlyProvides(object, directlyProvidedBy(object) - interface)
@@ -794,6 +803,8 @@ class ClassProvidesBase(SpecificationBase):
     )
 
     def __get__(self, inst, cls):
+        # member slots are set by subclass
+        # pylint:disable=no-member
         if cls is self._cls:
             # We only work if called on the class we were defined for
 
@@ -815,6 +826,10 @@ class ClassProvides(Declaration, ClassProvidesBase):
     interfaces a bit quicker.
     """
 
+    __slots__ = (
+        '__args',
+    )
+
     def __init__(self, cls, metacls, *interfaces):
         self._cls = cls
         self._implements = implementedBy(cls)
@@ -835,18 +850,18 @@ class ClassProvides(Declaration, ClassProvidesBase):
     __get__ = ClassProvidesBase.__get__
 
 
-def directlyProvidedBy(object):
+def directlyProvidedBy(object): # pylint:disable=redefined-builtin
     """Return the interfaces directly provided by the given object
 
     The value returned is an `~zope.interface.interfaces.IDeclaration`.
     """
     provides = getattr(object, "__provides__", None)
-    if (provides is None # no spec
-        or
-        # We might have gotten the implements spec, as an
-        # optimization. If so, it's like having only one base, that we
-        # lop off to exclude class-supplied declarations:
-        isinstance(provides, Implements)
+    if (
+            provides is None # no spec
+            # We might have gotten the implements spec, as an
+            # optimization. If so, it's like having only one base, that we
+            # lop off to exclude class-supplied declarations:
+            or isinstance(provides, Implements)
     ):
         return _empty
 
@@ -888,8 +903,8 @@ def classProvides(*interfaces):
     if PYTHON3:
         raise TypeError(_ADVICE_ERROR % 'provider')
 
-    frame = sys._getframe(1)
-    locals = frame.f_locals
+    frame = sys._getframe(1) # pylint:disable=protected-access
+    locals = frame.f_locals # pylint:disable=redefined-builtin
 
     # Try to make sure we were called from a class def
     if (locals is frame.f_globals) or ('__module__' not in locals):
@@ -947,8 +962,8 @@ def moduleProvides(*interfaces):
 
       directlyProvides(sys.modules[__name__], I1)
     """
-    frame = sys._getframe(1)
-    locals = frame.f_locals
+    frame = sys._getframe(1) # pylint:disable=protected-access
+    locals = frame.f_locals # pylint:disable=redefined-builtin
 
     # Try to make sure we were called from a class def
     if (locals is not frame.f_globals) or ('__name__' not in locals):
diff --git a/src/zope/interface/interface.py b/src/zope/interface/interface.py
index 2d9a873..e83b512 100644
--- a/src/zope/interface/interface.py
+++ b/src/zope/interface/interface.py
@@ -545,7 +545,7 @@ class _InterfaceMetaClass(type):
     # to be able to read it on a type and get the expected string. We
     # also need to be able to set it on an instance and get the value
     # we set. So far so good. But what gets tricky is that we'd like
-    # to store the value in the C structure (``__ibmodule__``) for
+    # to store the value in the C structure (``InterfaceBase.__ibmodule__``) for
     # direct access during equality, sorting, and hashing. "No
     # problem, you think, I'll just use a property" (well, the C
     # equivalents, ``PyMemberDef`` or ``PyGetSetDef``).
@@ -555,12 +555,12 @@ class _InterfaceMetaClass(type):
     # string in the class's dictionary under ``__module__``, thus
     # overriding the property inherited from the superclass. Writing
     # ``Subclass.__module__`` still works, but
-    # ``instance_of_subclass.__module__`` fails.
+    # ``Subclass().__module__`` fails.
     #
-    # There are multiple ways to workaround this:
+    # There are multiple ways to work around this:
     #
-    # (1) Define ``__getattribute__`` to watch for ``__module__`` and return
-    # the C storage.
+    # (1) Define ``InterfaceBase.__getattribute__`` to watch for
+    # ``__module__`` and return the C storage.
     #
     # This works, but slows down *all* attribute access (except,
     # ironically, to ``__module__``) by about 25% (40ns becomes 50ns)
@@ -575,20 +575,27 @@ class _InterfaceMetaClass(type):
     # It can't be a straight up ``@property`` descriptor, though, because accessing
     # it on the class returns a ``property`` object, not the desired string.
     #
-    # (3) Implement a data descriptor (``__get__`` and ``__set__``) that
-    # is both a string, and also does the redirect of ``__module__`` to ``__ibmodule__``
-    # and does the correct thing with the ``instance`` argument to ``__get__`` is None
-    # (returns the class's value.)
+    # (3) Implement a data descriptor (``__get__`` and ``__set__``)
+    # that is both a subclass of string, and also does the redirect of
+    # ``__module__`` to ``__ibmodule__`` and does the correct thing
+    # with the ``instance`` argument to ``__get__`` is None (returns
+    # the class's value.) (Why must it be a subclass of string? Because
+    # when it' s in the class's dict, it's defined on an *instance* of the
+    # metaclass; descriptors in an instance's dict aren't honored --- their
+    # ``__get__`` is never invoked --- so it must also *be* the value we want
+    # returned.)
     #
     # This works, preserves the ability to read and write
     # ``__module__``, and eliminates any penalty accessing other
-    # attributes. But it slows down accessing ``__module__`` of instances by 200%
-    # (40ns to 124ns).
+    # attributes. But it slows down accessing ``__module__`` of
+    # instances by 200% (40ns to 124ns), requires editing class dicts on the fly
+    # (in InterfaceClass.__init__), thus slightly slowing down all interface creation,
+    # and is ugly.
     #
     # (4) As in the last step, but make it a non-data descriptor (no ``__set__``).
     #
     # If you then *also* store a copy of ``__ibmodule__`` in
-    # ``__module__`` in the instances dict, reading works for both
+    # ``__module__`` in the instance's dict, reading works for both
     # class and instance and is full speed for instances. But the cost
     # is storage space, and you can't write to it anymore, not without
     # things getting out of sync.
@@ -597,10 +604,16 @@ class _InterfaceMetaClass(type):
     # so would break BTrees and normal dictionaries, as well as the
     # repr, maybe more.)
     #
-    # That leaves us with a metaclass. Here we can have our cake and
-    # eat it too: no extra storage, and C-speed access to the
-    # underlying storage. The only cost is that metaclasses tend to
-    # make people's heads hurt. (But still less than the descriptor-is-string, I think.)
+    # That leaves us with a metaclass. (Recall that a class is an
+    # instance of its metaclass, so properties/descriptors defined in
+    # the metaclass are used when accessing attributes on the
+    # instance/class. We'll use that to define ``__module__``.) Here
+    # we can have our cake and eat it too: no extra storage, and
+    # C-speed access to the underlying storage. The only substantial
+    # cost is that metaclasses tend to make people's heads hurt. (But
+    # still less than the descriptor-is-string, hopefully.)
+
+    __slots__ = ()
 
     def __new__(cls, name, bases, attrs):
         # Figure out what module defined the interface.
@@ -630,12 +643,14 @@ class _InterfaceMetaClass(type):
             cls.__name__,
         )
 
+
 _InterfaceClassBase = _InterfaceMetaClass(
     'InterfaceClass',
     (InterfaceBase, Element, Specification),
-    {}
+    {'__slots__': ()}
 )
 
+
 class InterfaceClass(_InterfaceClassBase):
     """
     Prototype (scarecrow) Interfaces Implementation.
@@ -674,8 +689,9 @@ class InterfaceClass(_InterfaceClassBase):
                     pass
 
         InterfaceBase.__init__(self, name, __module__)
-        assert '__module__' not in self.__dict__
-        assert self.__ibmodule__ is self.__module__ is __module__
+        # These asserts assisted debugging the metaclass
+        # assert '__module__' not in self.__dict__
+        # assert self.__ibmodule__ is self.__module__ is __module__
 
         d = attrs.get('__doc__')
         if d is not None:
diff --git a/src/zope/interface/tests/test_declarations.py b/src/zope/interface/tests/test_declarations.py
index c391cdb..b0875c4 100644
--- a/src/zope/interface/tests/test_declarations.py
+++ b/src/zope/interface/tests/test_declarations.py
@@ -390,6 +390,27 @@ class TestImplements(NameAndModuleComparisonTestsMixin,
         self.assertTrue(proxy != implementedByB)
         self.assertTrue(implementedByB != proxy)
 
+    def test_changed_deletes_super_cache(self):
+        impl = self._makeOne()
+        self.assertIsNone(impl._super_cache)
+        self.assertNotIn('_super_cache', impl.__dict__)
+
+        impl._super_cache = 42
+        self.assertIn('_super_cache', impl.__dict__)
+
+        impl.changed(None)
+        self.assertIsNone(impl._super_cache)
+        self.assertNotIn('_super_cache', impl.__dict__)
+
+    def test_changed_does_not_add_super_cache(self):
+        impl = self._makeOne()
+        self.assertIsNone(impl._super_cache)
+        self.assertNotIn('_super_cache', impl.__dict__)
+
+        impl.changed(None)
+        self.assertIsNone(impl._super_cache)
+        self.assertNotIn('_super_cache', impl.__dict__)
+
 
 class Test_implementedByFallback(unittest.TestCase):
 
diff --git a/tox.ini b/tox.ini
index 2c60a50..be90fe3 100644
--- a/tox.ini
+++ b/tox.ini
@@ -43,7 +43,7 @@ commands =
     coverage report -i
     coverage html -i
     coverage xml -i
-depends = py27,py27-pure,py35,py35-pure,py36,py37,py38,py38-cext,pypy,pypy3
+depends = py27,py27-pure,py35,py35-pure,py36,py37,py38,py38-cext,pypy,pypy3,docs
 parallel_show_output = true
 
 [testenv:docs]
@@ -51,7 +51,7 @@ basepython =
     python3
 commands =
     sphinx-build -b html -d docs/_build/doctrees docs docs/_build/html
-    sphinx-build -b doctest -d docs/_build/doctrees docs docs/_build/doctest
+    coverage run -p -m sphinx -b doctest -d docs/_build/doctrees docs docs/_build/doctest
 deps =
     Sphinx
     repoze.sphinx.autointerface