[Lldb-commits] [PATCH] D71801: [lldb/Lua] Make lldb.debugger et al available to Lua

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:46
+      m_lua(std::make_unique<Lua>()) {
+  llvm::cantFail(GetLua().EnterSession(debugger.GetID()));
+}
----------------
JDevlieghere wrote:
> labath wrote:
> > I don't think this is right. You should be able to set `lldb.debugger` once and for all during initialization, but the rest of the variables (thread, process, target) still need to be set upon entering the lua script as these can vary during the lifetime of an Debugger.
> I prefer setting/unsetting them together. This also ensures you don't have access to `lldb.debugger` outside of an interactive session. 
(Un)setting them together is fine, but i don't see why you need to enter a session in the ScriptInterpreterLua constructor now that it is done in IOHandlerLuaInterpreter. I think this part should be deleted.


================
Comment at: lldb/test/Shell/ScriptInterpreter/Lua/nested_sessions.test:7-17
+# CHECK-NEXT: foo
+# CHECK-NEXT: foo
+# CHECK-NEXT: foo
+# CHECK-NEXT: bar
+# CHECK-NEXT: foo
+# CHECK-NEXT: bar
+# CHECK-NEXT: foo
----------------
Would it be possible to group these by two or otherwise give some context to each line (e.g. inlining nested_sessions.in into this file would help). This way, the checks are very opaque...


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

https://reviews.llvm.org/D71801