[Lldb-commits] [PATCH] D117462: [lldb/python] Use PythonObject in LLDBSwigPython functions

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:284
+    return StructuredData::ObjectSP(new StructuredPythonObject(
+        PythonObject(PyRefType::Borrowed, m_py_obj)));
   }
----------------
fixed a leak here


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1442
   Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
-  void *ret_val = LLDBSWIGPython_CreateFrameRecognizer(
+  PythonObject ret_val = LLDBSWIGPython_CreateFrameRecognizer(
       class_name, m_dictionary_name.c_str());
----------------
all of these were leaked


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2159
                    Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
-    callee_wrapper_sp = std::make_shared<StructuredPythonObject>(new_callee);
+    callee_wrapper_sp = std::make_shared<StructuredPythonObject>(
+        PythonObject(PyRefType::Borrowed, static_cast<PyObject *>(new_callee)));
----------------
I didn't convert this one now because `LLDBSwigPythonCallTypeScript` is doing something weird (and most likely incorrect) with reference counts.


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2808
     void *module_pyobj = nullptr;
     if (ExecuteOneLineWithReturn(
             command_stream.GetData(),
----------------
Refactoring ExecuteOneLineWithReturn into something type-safe would be a bigger undertaking than all of the changes in this patch put together.


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp:54
   m_object_instance_sp =
-      StructuredData::GenericSP(new StructuredPythonObject(ret_val));
+      StructuredData::GenericSP(new StructuredPythonObject(std::move(ret_val)));
 
----------------
btw, all of the `std::move`s are just performance optimizations. They don't impact correctness.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117462/new/

https://reviews.llvm.org/D117462