[Lldb-commits] [PATCH] D71235: [lldb/Lua] Generate Lua Bindings and Make Them Available to the Script Interpreter

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/scripts/CMakeLists.txt:61-83
+  add_custom_command(
+    OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/LLDBWrapLua.cpp
+    DEPENDS ${SWIG_SOURCES}
+    DEPENDS ${SWIG_INTERFACES}
+    DEPENDS ${SWIG_HEADERS}
+    COMMAND ${SWIG_EXECUTABLE}
+        -c++
----------------
This looks like it could/should be shared between lua and python.


================
Comment at: lldb/scripts/lldb_lua.swig:10-170
+%{
+#include <algorithm>
+#include <string>
+%}
+
+/* The liblldb header files to be included. */
+%{
----------------
Can we figure out a way to share this stuff? I guess it could be just put into a common header and %included from the language-specific files..


================
Comment at: lldb/scripts/lldb_lua.swig:173
+%luacode%{
+lldb.SBDebugger.Initialize()
+%}
----------------
This shouldn't be needed if lua is only ever used from within lldb (mode a).


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt:16
   ScriptInterpreterLua.cpp
+  ${lldb_lua_wrapper}
 
----------------
I'd like to discuss this bit in more detail, as you've chosen to do things differently than python. While it does seem strange to have python (&lua) code built in the API folder, there is a reason for it -- this stuff depends on the lldb public api. Having it here creates a loop between the API folder and this script interpreter. One of the effects of that will be that it will be impossible to create a unit test binary for testing the lower level lua code (which I think it would be useful, just as equivalent python unit tests are shown themselves to be useful). 

Unfortunately, moving this stuff to the API folder does not completely remove the cycle (though it makes it much easier to work around it), because there will still be some API code which you'll need to call from within the script interpreter. Right now, that's just the `luaopen_lldb` but there will be others in the future.

I'd like to take this opportunity to create a solid story for the separation of responsibilities and layering between these two entities. If we can figure out a good solution here, then the we could also retrofit python to follow the same pattern. They way I would define "good layering" is that if "A is implemented on top of B" then it should be possible to define the purpose of "B" as well as implement it without referencing any concepts which are only defined in "A".

The way I would try to define the relationship of lua/python script interpreters and the API code is that the script interpreter operates on the lower level lldb_private debugger objects -- i.e., it sits "on top of" pretty much everything else *except* the API folder. It implements all of the interesting stuff about interacting with python/lua *except* how to actually represent the lldb_private entities in python. This last bit a customization point, which needs to be provided by the thing which it uses it -- in our case the API folder. This could be done by having funtcions which take the lldb_private objects, wrap them in the appropriate lldb object, and then wrap *that* via swig. And unwrapping could proceed similarly. This way, the python/lua code would never see any lldb::SB*** classes -- it would just see lldb_private classes, and their fully lua wrapped versions.

And the API folder would be left with the job of wrapping lldb_private objects into lua form and back, which isn't totally unreasonable -- the main job of the API folder is to provide a stable interface to lldb, and that is something we want to have in lua too. It does mean that there will have to be some language specific code in API folder, but I currently don't see a way to avoid that (though I'd like to see someone try to do that).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D71235