bulbazord added inline comments.
================
Comment at: lldb/include/lldb/API/SBBreakpoint.h:18-19
class ScriptInterpreter;
+namespace python {
+class SWIGBridge;
}
----------------
mib wrote:
> 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.
I agree, but we can refactor this later. Removing and/or changing forward
declarations doesn't break ABI.
================
Comment at: lldb/source/API/SBCommandInterpreter.cpp:37
+namespace lldb_private {
class CommandPluginInterfaceImplementation : public CommandObjectParsed {
public:
----------------
mib wrote:
> I find it a bit odd to have this here ... May be we should move this class
> out the the `API` directory ?
It's a little odd for sure, but it is an implementation detail so having it in
the cpp file seems alright. Where would you put it?
================
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);
----------------
mib wrote:
> 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.
Sure, I can add the TODO. I'd rather not add the templates or subclass in this
patch as it is complicated enough already.
================
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);
----------------
mib wrote:
> Can we move these in the `python` namespace as well ?
Sure
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
