[Lldb-commits] [PATCH] D59976: [Python] Remove Python include from ScriptInterpreterPython.h

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h:394-410
+  std::unique_ptr<PythonFile> m_saved_stdin;
+  std::unique_ptr<PythonFile> m_saved_stdout;
+  std::unique_ptr<PythonFile> m_saved_stderr;
+  std::unique_ptr<PythonObject> m_main_module;
+  std::unique_ptr<PythonObject> m_lldb_module;
+  std::unique_ptr<PythonDictionary> m_session_dict;
+  std::unique_ptr<PythonDictionary> m_sys_module_dict;
----------------
labath wrote:
> 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).
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?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59976