Fix dumpers/loaders after __slots__ cleanup

Commit 154b958 changed (correctly) the __slots__ usage, but this broke
dumpers/loaders since we relied directly on the own class __slots__
field.

To compensate, we introduce a simple function for computing the slots
across all parent classes (if any), and use this instead of __slots__
directly.

Note: the _all_slots() function is duplicated between objects.py and
opcodes.py, but the only other options is to introduce a lang.py for
such very basic language items.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

diff --git a/lib/objects.py b/lib/objects.py
index 76ca042..7fb7d5b 100644 (file)
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -109,16 +109,27 @@ class ConfigObject(object):
       setattr(self, k, v)
 
   def __getattr__(self, name):
-    if name not in self.__slots__:
+    if name not in self._all_slots():
       raise AttributeError("Invalid object attribute %s.%s" %
                            (type(self).__name__, name))
     return None
 
   def __setstate__(self, state):
+    slots = self._all_slots()
     for name in state:
-      if name in self.__slots__:
+      if name in slots:
         setattr(self, name, state[name])
 
+  @classmethod
+  def _all_slots(cls):
+    """Compute the list of all declared slots for a class.
+
+    """
+    slots = []
+    for parent in cls.__mro__:
+      slots.extend(getattr(parent, "__slots__", []))
+    return slots
+
   def ToDict(self):
     """Convert to a dict holding only standard python types.
 
@@ -130,7 +141,7 @@ class ConfigObject(object):
 
     """
     result = {}
-    for name in self.__slots__:
+    for name in self._all_slots():
       value = getattr(self, name, None)
       if value is not None:
         result[name] = value

diff --git a/lib/opcodes.py b/lib/opcodes.py
index 3aed41e..2520101 100644 (file)
--- a/lib/opcodes.py
+++ b/lib/opcodes.py
@@ -52,8 +52,9 @@ class BaseOpCode(object):
     __slots__ attribute for this class.
 
     """
+    slots = self._all_slots()
     for key in kwargs:
-      if key not in self.__slots__:
+      if key not in slots:
         raise TypeError("Object %s doesn't support the parameter '%s'" %
                         (self.__class__.__name__, key))
       setattr(self, key, kwargs[key])
@@ -69,7 +70,7 @@ class BaseOpCode(object):
 
     """
     state = {}
-    for name in self.__slots__:
+    for name in self._all_slots():
       if hasattr(self, name):
         state[name] = getattr(self, name)
     return state
@@ -88,13 +89,23 @@ class BaseOpCode(object):
       raise ValueError("Invalid data to __setstate__: expected dict, got %s" %
                        type(state))
 
-    for name in self.__slots__:
+    for name in self._all_slots():
       if name not in state:
         delattr(self, name)
 
     for name in state:
       setattr(self, name, state[name])
 
+  @classmethod
+  def _all_slots(cls):
+    """Compute the list of all declared slots for a class.
+
+    """
+    slots = []
+    for parent in cls.__mro__:
+      slots.extend(getattr(parent, "__slots__", []))
+    return slots
+
 
 class OpCode(BaseOpCode):
   """Abstract OpCode.

diff --git a/tools/cfgshell b/tools/cfgshell
index 57ed0bd..3c53d81 100755 (executable)
--- a/tools/cfgshell
+++ b/tools/cfgshell
@@ -93,7 +93,9 @@ class ConfigShell(cmd.Cmd):
     dirs = []
     entries = []
     if isinstance(obj, objects.ConfigObject):
-      for name in obj.__slots__:
+      # pylint: disable-msg=W0212
+      # yes, we're using a protected member
+      for name in obj._all_slots():
         child = getattr(obj, name, None)
         if isinstance(child, (list, dict, tuple, objects.ConfigObject)):
           dirs.append(name)