[Lldb-commits] [PATCH] D56511: [Python] Update PyString_FromString() to work for python 2 and 3.

[Zachary Turner via Phabricator via lldb-commits]

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