From a01e8474f605c5127a512c4ede2b094206db0570 Mon Sep 17 00:00:00 2001
From: William S Fulton <wsf@fultondesigns.co.uk>
Date: Thu, 6 Feb 2020 06:45:11 +0000
Subject: [PATCH] Fixing setting this in Python when using __slots__

Don't attempt to use the class's __dict__ for setting 'this' when
a user has extended a class with:
    __slots__ = ['this'].
Was segfaulting. Now we fall back to a simple PyObject_SetAttr if the
usual approach to setting 'this' in __dict__ does not work.

Closes #1673 Closes #1674
---
 CHANGES.current                               |  3 +++
 .../test-suite/python/python_append_runme.py  | 13 ++++++++++
 Examples/test-suite/python_append.i           | 20 ++++++++++++++-
 Lib/python/pyrun.swg                          | 25 ++++++++-----------
 4 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/CHANGES.current b/CHANGES.current
index 75043b3ea..aba67f451 100644
--- a/CHANGES.current
+++ b/CHANGES.current
@@ -7,6 +7,9 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/
 Version 4.0.2 (in progress)
 ===========================
 
+2020-02-06: wsfulton
+            [Python] #1673 #1674 Fix setting 'this' when extending a proxy class with __slots__.
+
 2020-01-31: vadz
             [Ruby] Add std::auto_ptr<> typemaps.
 
diff --git a/Examples/test-suite/python/python_append_runme.py b/Examples/test-suite/python/python_append_runme.py
index eddda53ff..e5f6b07cf 100644
--- a/Examples/test-suite/python/python_append_runme.py
+++ b/Examples/test-suite/python/python_append_runme.py
@@ -21,3 +21,16 @@ if grabstaticpath() != None:
 Test.static_func()
 if grabstaticpath() != os.path.basename(mypath):
     raise RuntimeError("grabstaticpath failed")
+
+# slots test
+fs = ForSlots()
+if fs.ValidVariable != 99:
+    raise RuntimeError("ValidVariable failed")
+fs.ValidVariable = 11
+if fs.ValidVariable != 11:
+    raise RuntimeError("ValidVariable failed")
+try:
+    fs.Invalid = 22
+    raise RuntimeError("It should not be possible to set a random variable name")
+except AttributeError:
+    pass
diff --git a/Examples/test-suite/python_append.i b/Examples/test-suite/python_append.i
index 049494319..883097ec6 100644
--- a/Examples/test-suite/python_append.i
+++ b/Examples/test-suite/python_append.i
@@ -42,13 +42,31 @@ import os.path
 %}
 
 %inline %{
-
 class Test {
 public:
   static void static_func() {};
   void funk() {};
 };
+%}
 
+// Github issue #1674
+%extend ForSlots {
+  %pythoncode %{
+        __slots__ = ["this"]
+    %}
+}
+// While __slots__ does not contain 'ValidVariable' in the list, it is still possible
+// to set 'ValidVariable'. A little odd, but the whole attribute setting is bypassed
+// for setting C/C++ member variables.
+// Not sure how to test the equivalent for -builtin.
+%inline %{
+struct ForSlots {
+  int ValidVariable;
+  ForSlots() : ValidVariable(99) {}
+};
+%}
+
+%inline %{
 #ifdef SWIGPYTHON_BUILTIN
 bool is_python_builtin() { return true; }
 #else
diff --git a/Lib/python/pyrun.swg b/Lib/python/pyrun.swg
index d6eeda984..66fa67055 100644
--- a/Lib/python/pyrun.swg
+++ b/Lib/python/pyrun.swg
@@ -1248,22 +1248,19 @@ SWIG_Python_NewShadowInstance(SwigPyClientData *data, PyObject *swig_this)
 SWIGRUNTIME void
 SWIG_Python_SetSwigThis(PyObject *inst, PyObject *swig_this)
 {
- PyObject *dict;
 #if !defined(SWIG_PYTHON_SLOW_GETSET_THIS)
- PyObject **dictptr = _PyObject_GetDictPtr(inst);
- if (dictptr != NULL) {
-   dict = *dictptr;
-   if (dict == NULL) {
-     dict = PyDict_New();
-     *dictptr = dict;
-   }
-   PyDict_SetItem(dict, SWIG_This(), swig_this);
-   return;
- }
+  PyObject **dictptr = _PyObject_GetDictPtr(inst);
+  if (dictptr != NULL) {
+    PyObject *dict = *dictptr;
+    if (dict == NULL) {
+      dict = PyDict_New();
+      *dictptr = dict;
+    }
+    PyDict_SetItem(dict, SWIG_This(), swig_this);
+    return;
+  }
 #endif
- dict = PyObject_GetAttrString(inst, "__dict__");
- PyDict_SetItem(dict, SWIG_This(), swig_this);
- Py_DECREF(dict);
+  PyObject_SetAttr(inst, SWIG_This(), swig_this);
 }