There's two more mentions of this function:
I guess these have to be updated as well? Would it be worth to define a PYSTRING_FROM_STRING macro?
I think it's the other way around. Definitely the one in PythonDataObjects.cpp should remain, and the code in this patch should be updated to use PythonString, which already abstracts all this away. In general we should not need explicit version checks anywhere outside of PythonDataObjects.cpp, as the whole purpose of that is to abstract away the differences so we don't need ifdefs in our code.
|28–39 ↗||(On Diff #180912)|
This entire function can just be deleted, it's not used.
|829–835 ↗||(On Diff #180912)|
Instead of the version checks, use this:
PythonString str(callee_name); PyObject *result = PyObject_CallMethodObjectArgs(implementor, str.GetObject(), arg, nullptr);
|834 ↗||(On Diff #180912)|
Note that there appears to be an object leak here, as the str_arg is never released. Changing to PythonString should also fix that bug as a free side effect.
|827 ↗||(On Diff #180924)|
This looks like another possible object leak. The documentation for PyObject_CallMethodObjArgs doesn't mention whether it takes ownership of the argument, but I think it wouldn't make sense for it to do so, because you're just calling a function. In any case, just make sure the current change doesn't regress the test suite.