[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

This revision was automatically updated to reflect the committed changes. Closed by commit rG572b9f468ad6: [lldb/Lua] Support loading Lua modules (authored by JDevlieghere). Changed prior to commit: https://reviews.llvm.org/D71825?vs=237088=237383#toc Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

JDevlieghere added a comment. In D71825#1813766 , @labath wrote: > This looks fine to me now. Thanks for your patience. Thank you for the review! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71825/new/ https://reviews.llvm.org/D71825

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

labath added inline comments. Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:46 + int error = luaL_loadfile(m_lua_state, filename.data()) || + lua_pcall(m_lua_state, 0, LUA_MULTRET, 0); + if (error) { It would be better to put `1`

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. This looks fine to me now. Thanks for your patience. Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h:43-46 + llvm::Expected FormatQuoted(llvm::StringRef str);

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

JDevlieghere marked an inline comment as done. JDevlieghere added a comment. In D71825#1811587 , @labath wrote: > Now back to the current patch: I see that you've dropped the part which > assigns the result of evaluating the module to a global variable

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

JDevlieghere updated this revision to Diff 237088. JDevlieghere added a comment. - Remove unused dense map. - Set the global variable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71825/new/ https://reviews.llvm.org/D71825 Files: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

labath added a comment. Thanks. For posterity, I'm going to summarize my thoughts from that discussion. I was arguing that "require" is not a good thing to be using here, because it's hard for it to guarantee that it will load a specific file. Now, that does not matter for "normal" uses of

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

JDevlieghere updated this revision to Diff 236957. JDevlieghere added a comment. Skip the package loader as per our discussion on IRC Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71825/new/ https://reviews.llvm.org/D71825 Files:

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

JDevlieghere updated this revision to Diff 236853. JDevlieghere added a comment. - Use `?.lua` to import modules - Use Lua runtime to quote strings CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71825/new/ https://reviews.llvm.org/D71825 Files:

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

JDevlieghere planned changes to this revision. JDevlieghere marked an inline comment as done. JDevlieghere added a comment. In D71825#1809825 , @labath wrote: > I think this needs more discussion. The current method of appending to > `package.path` seems

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

labath requested changes to this revision. labath added a comment. This revision now requires changes to proceed. I think this needs more discussion. The current method of appending to `package.path` seems to be completely broken. `package.path` contains patterns (e.g.

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

JDevlieghere marked 5 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:49 +if (llvm::Error e = +Run(llvm::formatv("package.path = package.path .. \";{0}\"", +

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

JDevlieghere updated this revision to Diff 236624. JDevlieghere added a comment. - Validate extension - Safely double quote strings CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71825/new/ https://reviews.llvm.org/D71825 Files: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

labath added a comment. Sorry for the delay, I was OOO and I'm still processing the backlog... Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:39 + + ConstString module_name = file.GetFileNameStrippingExtension(); + if (!module_name) { I guess

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

clayborg added a comment. lgtm. Pavel? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71825/new/ https://reviews.llvm.org/D71825 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

JDevlieghere added a comment. friendly ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71825/new/ https://reviews.llvm.org/D71825 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

JDevlieghere updated this revision to Diff 235092. JDevlieghere added a comment. Rebase after `can_reload` removal CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71825/new/ https://reviews.llvm.org/D71825 Files: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:57 + + // Reload the module if requested. + if (llvm::Error e = Run( `s/ if requested//` CHANGES SINCE LAST

[Lldb-commits] [PATCH] D71825: [lldb/Lua] Support loading Lua modules.

JDevlieghere created this revision. JDevlieghere added reviewers: LLDB, labath. Herald added a project: LLDB. This implements the `command script import` command for Lua. Repository: rLLDB LLDB https://reviews.llvm.org/D71825 Files: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp