Why is type_modified() in typeobject.c not a public function?

[reposting this to python-dev, as it affects both 2.6 and 3.0] Hi, when we build extension classes in Cython, we have to first build the type to make it available to user code, and then update the type's tp_dict while we run the class body code (PyObject_SetAttr() does not work here). In Py2.6+, this requires invalidating the method cache after each attribute change, which Python does internally using the type_modified() function. Could this function get a public interface? I do not think Cython is the only case where C code wants to modify a type after its creation, and copying the code over seems like a hack to me. Stefan

I'm fine with giving it a public interface. Please submit a patch with docs included. On Tue, May 27, 2008 at 9:47 AM, Stefan Behnel <stefan_ml@behnel.de> wrote:
[reposting this to python-dev, as it affects both 2.6 and 3.0]
Hi,
when we build extension classes in Cython, we have to first build the type to make it available to user code, and then update the type's tp_dict while we run the class body code (PyObject_SetAttr() does not work here). In Py2.6+, this requires invalidating the method cache after each attribute change, which Python does internally using the type_modified() function.
Could this function get a public interface? I do not think Cython is the only case where C code wants to modify a type after its creation, and copying the code over seems like a hack to me.
Stefan
_______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/guido%40python.org
-- --Guido van Rossum (home page: http://www.python.org/~guido/)

Hi, Guido van Rossum wrote:
On Tue, May 27, 2008 at 9:47 AM, Stefan Behnel <stefan_ml@behnel.de> wrote:
Could this function get a public interface? I do not think Cython is the only case where C code wants to modify a type after its creation, and copying the code over seems like a hack to me.
I'm fine with giving it a public interface. Please submit a patch with docs included.
Straight forward patch is attached (against 3.0a5). Works for me in Cython. I thought about a name like "Taint(t)" or "ClearTypeCache(t)", but then went with the coward solution of calling the function PyType_Modified() as it was (almost) named internally. BTW, I noticed that the code in typeobject.c uses "DECREF before set" two times, like this: method_cache[h].version = type->tp_version_tag; method_cache[h].value = res; /* borrowed */ Py_INCREF(name); Py_DECREF(method_cache[h].name); method_cache[h].name = name; During the call to Py_DECREF, the cache content is incorrect, so can't this run into the same problem that Py_CLEAR() aims to solve? I attached a patch for that, too, just in case. Stefan

Stefan Behnel wrote:
Straight forward patch is attached (against 3.0a5). [...] BTW, I noticed that the code in typeobject.c uses "DECREF before set" two times.
Filed on the bug tracker as issues 2989 and 2990. http://bugs.python.org/issue2989 http://bugs.python.org/issue2990 Stefan

Stefan Behnel schrieb:
Straight forward patch is attached (against 3.0a5). Works for me in Cython. I thought about a name like "Taint(t)" or "ClearTypeCache(t)", but then went with the coward solution of calling the function PyType_Modified() as it was (almost) named internally.
BTW, I noticed that the code in typeobject.c uses "DECREF before set" two times, like this:
method_cache[h].version = type->tp_version_tag; method_cache[h].value = res; /* borrowed */ Py_INCREF(name); Py_DECREF(method_cache[h].name); method_cache[h].name = name;
During the call to Py_DECREF, the cache content is incorrect, so can't this run into the same problem that Py_CLEAR() aims to solve? I attached a patch for that, too, just in case.
Please create two tickets in the bug tracker and append the patches. Christian

Stefan Behnel <stefan_ml <at> behnel.de> writes:
BTW, I noticed that the code in typeobject.c uses "DECREF before set" two times, like this:
method_cache[h].version = type->tp_version_tag; method_cache[h].value = res; /* borrowed */ Py_INCREF(name); Py_DECREF(method_cache[h].name); method_cache[h].name = name;
Since this is so common, would it be useful to define a macro for the correct construct? Something like #define Py_SETREF(var, obj) \ { PyObject *tmp = (var); Py_INCREF(obj); \ (var) = (obj); Py_XDECREF(tmp); } Or, if we want to allow more optimizations, make two versions of it: #define Py_SETREF(var, obj) \ { PyObject *tmp = (var); Py_INCREF(obj); \ (var) = (obj); Py_DECREF(tmp); } #define Py_XSETREF(var, obj) \ { PyObject *tmp = (var); Py_INCREF(obj); \ (var) = (obj); Py_XDECREF(tmp); } Regards Antoine.

Antoine Pitrou wrote:
Stefan Behnel <stefan_ml <at> behnel.de> writes:
BTW, I noticed that the code in typeobject.c uses "DECREF before set" two times, like this:
method_cache[h].version = type->tp_version_tag; method_cache[h].value = res; /* borrowed */ Py_INCREF(name); Py_DECREF(method_cache[h].name); method_cache[h].name = name;
Since this is so common, would it be useful to define a macro for the correct construct?
Something like
#define Py_SETREF(var, obj) \ { PyObject *tmp = (var); Py_INCREF(obj); \ (var) = (obj); Py_XDECREF(tmp); }
Or, if we want to allow more optimizations, make two versions of it:
#define Py_SETREF(var, obj) \ { PyObject *tmp = (var); Py_INCREF(obj); \ (var) = (obj); Py_DECREF(tmp); }
#define Py_XSETREF(var, obj) \ { PyObject *tmp = (var); Py_INCREF(obj); \ (var) = (obj); Py_XDECREF(tmp); }
Both sound like a good idea to me. Having to think about whether or not DECREF poses a problem in a specific case is just cumbersome. Stefan
participants (4)
-
Antoine Pitrou
-
Christian Heimes
-
Guido van Rossum
-
Stefan Behnel