zturner requested changes to this revision. zturner added a comment. This revision now requires changes to proceed.
In D56511#1351768 <https://reviews.llvm.org/D56511#1351768>, @JDevlieghere wrote: > There's two more mentions of this function: > > - `unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp` > - `source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp` > > 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. ================ Comment at: lldb/scripts/Python/python-swigsafecast.swig:28-39 template <> PyObject* SBTypeToSWIGWrapper (const char* c_str) { if (c_str) +#if PY_MAJOR_VERSION >= 3 + return PyUnicode_FromString(c_str); ---------------- This entire function can just be deleted, it's not used. ================ Comment at: lldb/scripts/Python/python-wrapper.swig:829-835 +#if PY_MAJOR_VERSION >= 3 + PyObject *str_arg = PyUnicode_FromString(callee_name); +#else + PyObject *str_arg = PyString_FromString(callee_name); +#endif + PyObject* result = PyObject_CallMethodObjArgs(implementor, str_arg, arg, NULL); return result; ---------------- Instead of the version checks, use this: ``` PythonString str(callee_name); PyObject *result = PyObject_CallMethodObjectArgs(implementor, str.GetObject(), arg, nullptr); ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56511/new/ https://reviews.llvm.org/D56511 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
