Make __dict__ accessible for Python builtin classes

[amaeldoe]

Attribute set within instance of a SWIG Python wrapped class are
stored in SwigPyObject->dict, which tp_dictoffset slot is pointing to.

However, SWIG wrapped classes did not have a dict attribute.
Inheriting subclasses did not get the attribute either because the
SWIG wrapped classes initialize the tp_dictoffset slot:

From http://bugs.python.org/issue16272:

"If a type defines a nonzero tp_dictoffset, that type is responsible for
defining a __dict__ slot as part of the tp_getset structures. Failure to
do so will result in the dict being inaccessible from Python via
obj.__dict__ from instances of the type or subtypes."

Provide a SwigPyObject_get___dict__() function to retrieve the dict
attribute or create it when it does not exist yet (it is normally
created when setting attribute set), and a PyGetSetDef entry pointing
to this function.

[wsfulton]

This looks like an improvement. dir() now show's that the type has a
'dict' and adding attributes/instance variables (non-C++) still works, and
they then appear in dict. %pythonnondynamic, which prevents the addition
of non-C++ attributes from being added, still works too.

I'm just surprised that a 'dict' is not provided 'for free' and that
we have to go to the trouble of adding in this code. I tried setting
tp_dictoffset to 0 in the hope that it would trigger some default dict,
but without any luck. My main concern is that a simpler default behaviour is
available and we're missing what it is.

Some minor comments:

• I don't see the point of the changes in SWIG_Python_NewPointerObj. It would be
  better to keep all the initialisation in one place as is currently done. The
  new implementation also assumes newobj is non-zero, which may not always be the
  case.
• Please change

SwigPyObject_get___dict__(PyObject *v, PyObject *args)

to

SwigPyObject_get___dict__(PyObject *v, PyObject *SWIGUNUSEDPARM(args))

• Is SwigPyObject_get___dict__() thread safe or will the GIL protect this
  function from being called twice if .dict is called from two
  threads? Wouldn't it be better to initialise this in
  SWIG_Python_NewPointerObj instead of the lazy initialization in
  SwigPyObject_get___dict__()?
• The SWIGPY_BUILTIN_DESTROY_DICT macro is not needed as this macro is defined
  in builtin.swg which is only used for builtin. Just use Py_XDECREF always.

[amaeldoe]

A dictionnary is provided for free when you set tp_dictoffset to 0 (this is why setting random instance attribute is possible). However, for unknown reason, this dictionnary is not accessible through the dict attribute.

The change in SWIG_Python_NewPointerObj() is required to prevent subclasse attributes to be reset when the main classes init() method is called. My commit message show an example of the problem.

Moving the dictionnary creation within SWIG_NewPointerObj(), will exhibit the same issue. Given this test case:

class Test(MySwigWrappedClass):
        def __init__(self):
                self.val = "Random Value"
                MySwigWrappedClass.__init__(self)

The dictionary is overwritten as soon as MySwigWrappedClass.init() is called, which is why I originally implemented the dictionary creation in the getter function.

Let me know if you have any idea concerning this issue, otherwise I'll keep looking

[amaeldoe]

Just came accross the following original Python code, which work exactly the same. Have a look at the subtype_dict function : http://www.opensource.apple.com/source/python/python-3/python/Objects/typeobject.c

I thus assume it is the correct way to do it, and that the GIL is protecting the function.

[wsfulton]

Okay, I think all the points have been addressed except the changes in SWIG_Python_NewPointerObj should be reverted.

I would have said it is bad practice to expect a base class's functionality to work before calling the base's init, but I think the lazy initialization is okay now, so doesn't matter for this use case.

[amaeldoe]

If you revert the change in SWIG_Python_NewPointerObj(), newobj->dict will be reset to NULL anytime init() is called.

Setting random attribute before calling the parent init() function sounds perfectly fine by me ?

[wsfulton]

Sorry, I didn't quite follow all of your reply about SWIG_Python_NewPointerObj(), but I understand now.

Not calling init first is very unnatural to a c++ programmer where the base class is fully initialized (via constructors calls) before you can modify the derived class. However, your example does actually work with a pure Python class, so mirroring that behaviour is probably best, so I'll merge this as is once the Travis tests have passed.