mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.
LGTM with some comments.
================
Comment at: lldb/include/lldb/API/SBBreakpoint.h:18-19
class ScriptInterpreter;
+namespace python {
+class SWIGBridge;
}
----------------
We've talked about this offline, but I think we should stay language agnostic
in the SBAPI, so exposing a python namespace here is bothering me a little bit.
================
Comment at: lldb/source/API/SBCommandInterpreter.cpp:37
+namespace lldb_private {
class CommandPluginInterfaceImplementation : public CommandObjectParsed {
public:
----------------
I find it a bit odd to have this here ... May be we should move this class out
the the `API` directory ?
================
Comment at:
lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h:64-90
+class SWIGBridge {
+public:
+ static PythonObject ToSWIGWrapper(std::unique_ptr<lldb::SBValue> value_sb);
+ static PythonObject ToSWIGWrapper(lldb::ValueObjectSP value_sp);
+ static PythonObject ToSWIGWrapper(lldb::TargetSP target_sp);
+ static PythonObject ToSWIGWrapper(lldb::ProcessSP process_sp);
+ static PythonObject ToSWIGWrapper(lldb::ThreadPlanSP thread_plan_sp);
----------------
Although this is nested in the `python` namespace, I think `SWIGBridge` should
be a templated class defined in the `ScriptedInterpreter` header and this class
should be instead renamed as `SWIGPythonBridge` and be a specialization of the
`SWIGBridge` class. We can do that as a follow-up but a `TODO` comment would be
nice.
================
Comment at:
lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h:248-254
+void *LLDBSWIGPython_CastPyObjectToSBData(PyObject *data);
+void *LLDBSWIGPython_CastPyObjectToSBBreakpoint(PyObject *data);
+void *LLDBSWIGPython_CastPyObjectToSBAttachInfo(PyObject *data);
+void *LLDBSWIGPython_CastPyObjectToSBLaunchInfo(PyObject *data);
+void *LLDBSWIGPython_CastPyObjectToSBError(PyObject *data);
+void *LLDBSWIGPython_CastPyObjectToSBValue(PyObject *data);
+void *LLDBSWIGPython_CastPyObjectToSBMemoryRegionInfo(PyObject *data);
----------------
Can we move these in the `python` namespace as well ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150157/new/
https://reviews.llvm.org/D150157
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
