This patch limits the scope of the python header to the python script interpreter plugin by removing the include from ScriptInterpreterPython.h. To make that possible I had to forward declare the PythonDataObjects and move the PythonScriptInterpreterLocker into ScriptInterpreterPython.cpp.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h | ||
---|---|---|
327 ↗ | (On Diff #192763) | You could still keep this class inside ScriptInterpreterPython, and just forward-declare it in the header. |
394–410 ↗ | (On Diff #192763) | Instead of pimpl-ing each individual field, what if we just have one struct which contains all of these fields, and pimpl that? This should make things more efficient (less heap traffic), and allow you to cut down on the number of forward declarations (particularly the ones from python headers worry me). |
lldb/source/Plugins/ScriptInterpreter/Python/lldb-python.h | ||
35 ↗ | (On Diff #192763) | Python headers are fun. :) |
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h | ||
---|---|---|
394–410 ↗ | (On Diff #192763) | In fact, since ScriptInterpreterPython is already polymorphic, we might not need to pimpl anything at all. Since the only place where we create an instance if this class is the static Create function, we could just have that function create a subclass of ScriptInterpreterPython (and make ScriptInterpreterPython by moving all non-interface stuff into the subclass). WDYT? |
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp | ||
---|---|---|
1229–1230 ↗ | (On Diff #192892) | The ScriptInterpreterPythonImpl qualification on the Locker class is probably overkill. (I know it was present before this patch, but it's overkill there too :P ). |
Thanks for the review, Pavel!
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp | ||
---|---|---|
1229–1230 ↗ | (On Diff #192892) | Makes sense, I'll remove it before landing. |