This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG67f94e5a9745: [lldb/lua] Supplement Lua bindings for lldb
module (authored by Siger Yang sigerye...@gmail.com).
Changed
siger-young updated this revision to Diff 378971.
siger-young added a comment.
Pull and merge conflicts, will soon be merged into main.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108090/new/
https://reviews.llvm.org/D108090
Files:
tammela accepted this revision.
tammela added a comment.
LGTM.
You will have to rebase to main.
Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108090/new/
https://reviews.llvm.org/D108090
___
siger-young updated this revision to Diff 375240.
siger-young added a comment.
Add assertion functions and error status detection to remove "luaunit"
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108090/new/
https://reviews.llvm.org/D108090
siger-young added inline comments.
Comment at: lldb/test/API/lua_api/lua_lldb_test.lua:3
+EXPORT_ASSERT_TO_GLOBALS = true
+require('luaunit')
+
tammela wrote:
> siger-young wrote:
> > tammela wrote:
> > > Could we not use an external dependency?
> > > For
tammela added inline comments.
Comment at: lldb/test/API/lua_api/lua_lldb_test.lua:3
+EXPORT_ASSERT_TO_GLOBALS = true
+require('luaunit')
+
siger-young wrote:
> tammela wrote:
> > Could we not use an external dependency?
> > For instance in my setup it fails
siger-young added inline comments.
Comment at: lldb/test/API/lua_api/lua_lldb_test.lua:3
+EXPORT_ASSERT_TO_GLOBALS = true
+require('luaunit')
+
tammela wrote:
> Could we not use an external dependency?
> For instance in my setup it fails because it couldn't find
tammela added a comment.
Just one last thing and I think we are done!
Thanks for your work.
Comment at: lldb/test/API/lua_api/lua_lldb_test.lua:3
+EXPORT_ASSERT_TO_GLOBALS = true
+require('luaunit')
+
Could we not use an external dependency?
For instance in
siger-young updated this revision to Diff 375103.
siger-young added a comment.
Rebase commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108090/new/
https://reviews.llvm.org/D108090
Files:
lldb/CMakeLists.txt
siger-young updated this revision to Diff 375100.
siger-young added a comment.
Fix typo in SBData test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108090/new/
https://reviews.llvm.org/D108090
Files:
siger-young updated this revision to Diff 375081.
siger-young added a comment.
This update mainly fixed problematic typemaps and adding necessary comments.
Together, it forced Lua installation path as "PREFIX/lib/lua/5.3" and removed
"lit.util" in tests.
Repository:
rG LLVM Github Monorepo
JDevlieghere requested changes to this revision.
JDevlieghere added inline comments.
This revision now requires changes to proceed.
Comment at: lldb/CMakeLists.txt:55-60
+ # FIXME: Lua 5.3 is hardcoded but it should support 5.3+!
+ find_program(Lua_EXECUTABLE lua5.3)
+ if
tammela added a comment.
Hi Siger,
We are almost there.
I encourage you to continue working on this patch.
I have been busy the past weeks, but I will try to review ASAP once you address
my comments.
Comment at: lldb/bindings/lua/lua-typemaps.swig:219-221
siger-young added inline comments.
Comment at: lldb/bindings/lua/lua-typemaps.swig:219-221
+%typecheck(SWIG_TYPECHECK_STRING_ARRAY) char ** {
+ $1 = (lua_istable(L, $input) || lua_isnil(L, $input));
+}
tammela wrote:
> This is not being generated by SWIG for
tammela added a comment.
Trying to run the tests in my system failed with the following:
Comment at: lldb/CMakeLists.txt:60-68
+ execute_process(
+ COMMAND ${Lua_EXECUTABLE}
+ -e "for w in string.gmatch(package.cpath, ';?([^;]+);?') do \
+ if
siger-young updated this revision to Diff 367971.
siger-young added a comment.
This update adds some tests for Lua LLDB module.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108090/new/
https://reviews.llvm.org/D108090
Files:
tammela requested changes to this revision.
tammela added a comment.
This revision now requires changes to proceed.
Missing test cases!
Either a script that test it all, individual unit tests or a combination of
both.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
tammela added inline comments.
Comment at: lldb/bindings/lua/lua-typemaps.swig:198
+ size_t size = lua_rawlen(L, $input);
+ $1 = (char **)malloc((size + 1) * sizeof(char *));
+ int i = 0, j = 0;
This seems it could leak.
Are you sure it doesn't?
18 matches
Mail list logo
