[Lldb-commits] [PATCH] D71234: [lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

2019-12-20 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2861324208e1: [lldb/Lua] Implement a Simple Lua Script 
Interpreter Prototype (authored by JDevlieghere).

Changed prior to commit:
  https://reviews.llvm.org/D71234?vs=234798=234934#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71234

Files:
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Core/IOHandler.h
  lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/ScriptInterpreter/Lua/lua.test
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in
  lldb/unittests/ScriptInterpreter/CMakeLists.txt
  lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
  lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
===
--- /dev/null
+++ lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
@@ -0,0 +1,62 @@
+//===-- LuaTests.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Utility/Reproducer.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace lldb_private::repro;
+using namespace lldb;
+
+namespace {
+class ScriptInterpreterTest : public ::testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+
+// Pretend Linux is the host platform.
+platform_linux::PlatformLinux::Initialize();
+ArchSpec arch("powerpc64-pc-linux");
+Platform::SetHostPlatform(
+platform_linux::PlatformLinux::CreateInstance(true, ));
+  }
+  void TearDown() override {
+platform_linux::PlatformLinux::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+} // namespace
+
+TEST_F(ScriptInterpreterTest, Plugin) {
+  EXPECT_EQ(ScriptInterpreterLua::GetPluginNameStatic(), "script-lua");
+  EXPECT_EQ(ScriptInterpreterLua::GetPluginDescriptionStatic(),
+"Lua script interpreter");
+}
+
+TEST_F(ScriptInterpreterTest, ExecuteOneLine) {
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  ScriptInterpreterLua script_interpreter(*debugger_sp);
+  CommandReturnObject result;
+  EXPECT_TRUE(script_interpreter.ExecuteOneLine("foo = 1", ));
+  EXPECT_FALSE(script_interpreter.ExecuteOneLine("nil = foo", ));
+  EXPECT_TRUE(result.GetErrorData().startswith(
+  "error: lua failed attempting to evaluate 'nil = foo'"));
+}
Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- /dev/null
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -0,0 +1,26 @@
+//===-- LuaTests.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/ScriptInterpreter/Lua/Lua.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(LuaTest, RunValid) {
+  Lua lua;
+  llvm::Error error = lua.Run("foo = 1");
+  EXPECT_FALSE(static_cast(error));
+}
+
+TEST(LuaTest, RunInvalid) {
+  Lua lua;
+  llvm::Error error = lua.Run("nil = foo");
+  EXPECT_TRUE(static_cast(error));
+  EXPECT_EQ(llvm::toString(std::move(error)),
+"[string \"buffer\"]:1: unexpected symbol near 'nil'\n");
+}
Index: lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
@@ -0,0 +1,12 @@
+add_lldb_unittest(ScriptInterpreterLuaTests
+  LuaTests.cpp
+  ScriptInterpreterTests.cpp
+
+  LINK_LIBS
+lldbHost
+lldbPluginScriptInterpreterLua
+

[Lldb-commits] [PATCH] D71234: [lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

2019-12-20 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt:14-15
+
+target_include_directories(ScriptInterpreterLuaTests PRIVATE 
${LUA_INCLUDE_DIR})
+target_link_libraries(ScriptInterpreterLuaTests PRIVATE ${LUA_LIBRARIES})

I think you wouldn't need these if the other CMakeLists file would use PUBLIC 
keyword for headers, and INTERFACE for libraries.



Comment at: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp:53
+TEST_F(ScriptInterpreterTest, ExecuteOneLine) {
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);

JDevlieghere wrote:
> labath wrote:
> > I'm not actually opposed to this (and it's nice to know that this could 
> > work if we need it), but why do this as a unit test? It seems like this 
> > could be tested equally well with a `script` command through the lldb 
> > driver. I was imagining the unit tests would be most useful for the lower 
> > level functionality -- roughly corresponding to the `Lua` class. Right now 
> > that class is pretty simple and doesn't really need a unit test, but I 
> > expect it will become more complex over time...
> Yes, I just wanted to show that it's possible :-) I've also included a test 
> for the Lua class. 
Ok, that's cool.


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

https://reviews.llvm.org/D71234



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D71234: [lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

2019-12-19 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp:53
+TEST_F(ScriptInterpreterTest, ExecuteOneLine) {
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);

labath wrote:
> I'm not actually opposed to this (and it's nice to know that this could work 
> if we need it), but why do this as a unit test? It seems like this could be 
> tested equally well with a `script` command through the lldb driver. I was 
> imagining the unit tests would be most useful for the lower level 
> functionality -- roughly corresponding to the `Lua` class. Right now that 
> class is pretty simple and doesn't really need a unit test, but I expect it 
> will become more complex over time...
Yes, I just wanted to show that it's possible :-) I've also included a test for 
the Lua class. 


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

https://reviews.llvm.org/D71234



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D71234: [lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

2019-12-19 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 234798.
JDevlieghere marked 12 inline comments as done.
JDevlieghere added a comment.

- Rebase
- Feedback Pavel


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

https://reviews.llvm.org/D71234

Files:
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Core/IOHandler.h
  lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/ScriptInterpreter/Lua/lua.test
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in
  lldb/unittests/ScriptInterpreter/CMakeLists.txt
  lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
  lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
===
--- /dev/null
+++ lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
@@ -0,0 +1,62 @@
+//===-- LuaTests.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Utility/Reproducer.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace lldb_private::repro;
+using namespace lldb;
+
+namespace {
+class ScriptInterpreterTest : public ::testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+
+// Pretend Linux is the host platform.
+platform_linux::PlatformLinux::Initialize();
+ArchSpec arch("powerpc64-pc-linux");
+Platform::SetHostPlatform(
+platform_linux::PlatformLinux::CreateInstance(true, ));
+  }
+  void TearDown() override {
+platform_linux::PlatformLinux::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+} // namespace
+
+TEST_F(ScriptInterpreterTest, Plugin) {
+  EXPECT_EQ(ScriptInterpreterLua::GetPluginNameStatic(), "script-lua");
+  EXPECT_EQ(ScriptInterpreterLua::GetPluginDescriptionStatic(),
+"Lua script interpreter");
+}
+
+TEST_F(ScriptInterpreterTest, ExecuteOneLine) {
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  ScriptInterpreterLua script_interpreter(*debugger_sp);
+  CommandReturnObject result;
+  EXPECT_TRUE(script_interpreter.ExecuteOneLine("foo = 1", ));
+  EXPECT_FALSE(script_interpreter.ExecuteOneLine("nil = foo", ));
+  EXPECT_TRUE(result.GetErrorData().startswith(
+  "error: lua failed attempting to evaluate 'nil = foo'"));
+}
Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- /dev/null
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -0,0 +1,26 @@
+//===-- LuaTests.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/ScriptInterpreter/Lua/Lua.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(LuaTest, RunValid) {
+  Lua lua;
+  llvm::Error error = lua.Run("foo = 1");
+  EXPECT_FALSE(static_cast(error));
+}
+
+TEST(LuaTest, RunInvalid) {
+  Lua lua;
+  llvm::Error error = lua.Run("nil = foo");
+  EXPECT_TRUE(static_cast(error));
+  EXPECT_EQ(llvm::toString(std::move(error)),
+"[string \"buffer\"]:1: unexpected symbol near 'nil'\n");
+}
Index: lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
@@ -0,0 +1,15 @@
+add_lldb_unittest(ScriptInterpreterLuaTests
+  LuaTests.cpp
+  ScriptInterpreterTests.cpp
+
+  LINK_LIBS
+lldbHost
+lldbPluginScriptInterpreterLua
+lldbPluginPlatformLinux
+LLVMTestingSupport
+  LINK_COMPONENTS
+Support
+  )
+
+target_include_directories(ScriptInterpreterLuaTests PRIVATE ${LUA_INCLUDE_DIR})

[Lldb-commits] [PATCH] D71234: [lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

2019-12-19 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

This is starting to look pretty good. I think that pretty soon you'll have to 
come up with some way of having a more persistent lua context (instead of 
creating a fresh one for each command), but that doesn't have to happen now.

Does the "multiline" script command do anything already? Is there anything to 
test with respect to that.




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:19-21
+#ifndef LLDB_DISABLE_LIBEDIT
+#include "lldb/Host/Editline.h"
+#endif

No longer needed?



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:25
+
+#define LUA_PROMPT ">>> "
+

it doesn't look like this is used (but if it is, it would be better off as a 
regular variable)



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:59
+private:
+  lua_State *m_lua_state = nullptr;
+};

The constructor initialized this member, and the desctructor asserts it to be 
not null. It doesn't look like this assignment here is actually helping 
anything..



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:74
+if (llvm::Error error = m_lua.Run(data)) {
+  fprintf(GetOutputFILE(), "%s", llvm::toString(std::move(error)).c_str());
+}

*GetOutputStreamFileSP() << llvm::toString(std::move(error))



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:78
+
+  ~IOHandlerLuaInterpreter() override {}
+

delete



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:118
+  IOHandlerSP io_handler_sp(new IOHandlerLuaInterpreter(debugger));
+  if (io_handler_sp) {
+debugger.PushIOHandler(io_handler_sp);

This if is not needed  (new always succeeds)



Comment at: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp:53
+TEST_F(ScriptInterpreterTest, ExecuteOneLine) {
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);

I'm not actually opposed to this (and it's nice to know that this could work if 
we need it), but why do this as a unit test? It seems like this could be tested 
equally well with a `script` command through the lldb driver. I was imagining 
the unit tests would be most useful for the lower level functionality -- 
roughly corresponding to the `Lua` class. Right now that class is pretty simple 
and doesn't really need a unit test, but I expect it will become more complex 
over time...


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

https://reviews.llvm.org/D71234



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D71234: [lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

2019-12-18 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 234637.
JDevlieghere added a comment.

- Rebase
- Add test case


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

https://reviews.llvm.org/D71234

Files:
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Core/IOHandler.h
  lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/ScriptInterpreter/Lua/lua.test
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in
  lldb/unittests/ScriptInterpreter/CMakeLists.txt
  lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- /dev/null
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -0,0 +1,62 @@
+//===-- LuaTests.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Utility/Reproducer.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace lldb_private::repro;
+using namespace lldb;
+
+namespace {
+class ScriptInterpreterTest : public ::testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+
+// Pretend Linux is the host platform.
+platform_linux::PlatformLinux::Initialize();
+ArchSpec arch("powerpc64-pc-linux");
+Platform::SetHostPlatform(
+platform_linux::PlatformLinux::CreateInstance(true, ));
+  }
+  void TearDown() override {
+platform_linux::PlatformLinux::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+} // namespace
+
+TEST_F(ScriptInterpreterTest, Plugin) {
+  EXPECT_EQ(ScriptInterpreterLua::GetPluginNameStatic(), "script-lua");
+  EXPECT_EQ(ScriptInterpreterLua::GetPluginDescriptionStatic(),
+"Lua script interpreter");
+}
+
+TEST_F(ScriptInterpreterTest, ExecuteOneLine) {
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  ScriptInterpreterLua script_interpreter(*debugger_sp);
+  CommandReturnObject result;
+  EXPECT_TRUE(script_interpreter.ExecuteOneLine("foo = 1", ));
+  EXPECT_FALSE(script_interpreter.ExecuteOneLine("nil = foo", ));
+  EXPECT_TRUE(result.GetErrorData().startswith(
+  "error: lua failed attempting to evaluate 'nil = foo'"));
+}
Index: lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
@@ -0,0 +1,11 @@
+add_lldb_unittest(ScriptInterpreterLuaTests
+  LuaTests.cpp
+
+  LINK_LIBS
+lldbHost
+lldbPluginScriptInterpreterLua
+lldbPluginPlatformLinux
+LLVMTestingSupport
+  LINK_COMPONENTS
+Support
+  )
\ No newline at end of file
Index: lldb/unittests/ScriptInterpreter/CMakeLists.txt
===
--- lldb/unittests/ScriptInterpreter/CMakeLists.txt
+++ lldb/unittests/ScriptInterpreter/CMakeLists.txt
@@ -1,3 +1,6 @@
 if (LLDB_ENABLE_PYTHON)
   add_subdirectory(Python)
 endif()
+if (LLDB_ENABLE_LUA)
+  add_subdirectory(Lua)
+endif()
Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -19,6 +19,7 @@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
+config.lldb_enable_lua = @LLDB_ENABLE_LUA@
 config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@"
 # The shell tests use their own module caches.
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-shell")
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -103,6 +103,9 @@
 if config.lldb_enable_python:
 config.available_features.add('python')
 
+if config.lldb_enable_lua:
+config.available_features.add('lua')
+
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
Index: 

[Lldb-commits] [PATCH] D71234: [lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

2019-12-13 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 233877.
JDevlieghere added a comment.

- Rebase
- Hide C calls behind C++ abstraction
- Improve error handling
- Add unit test


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

https://reviews.llvm.org/D71234

Files:
  lldb/include/lldb/Core/IOHandler.h
  lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/unittests/ScriptInterpreter/CMakeLists.txt
  lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- /dev/null
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -0,0 +1,62 @@
+//===-- LuaTests.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Utility/Reproducer.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace lldb_private::repro;
+using namespace lldb;
+
+namespace {
+class ScriptInterpreterTest : public ::testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+
+// Pretend Linux is the host platform.
+platform_linux::PlatformLinux::Initialize();
+ArchSpec arch("powerpc64-pc-linux");
+Platform::SetHostPlatform(
+platform_linux::PlatformLinux::CreateInstance(true, ));
+  }
+  void TearDown() override {
+platform_linux::PlatformLinux::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+} // namespace
+
+TEST_F(ScriptInterpreterTest, Plugin) {
+  EXPECT_EQ(ScriptInterpreterLua::GetPluginNameStatic(), "script-lua");
+  EXPECT_EQ(ScriptInterpreterLua::GetPluginDescriptionStatic(),
+"Lua script interpreter");
+}
+
+TEST_F(ScriptInterpreterTest, ExecuteOneLine) {
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  ScriptInterpreterLua script_interpreter(*debugger_sp);
+  CommandReturnObject result;
+  EXPECT_TRUE(script_interpreter.ExecuteOneLine("foo = 1", ));
+  EXPECT_FALSE(script_interpreter.ExecuteOneLine("nil = foo", ));
+  EXPECT_TRUE(result.GetErrorData().startswith(
+  "error: lua failed attempting to evaluate 'nil = foo'"));
+}
Index: lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
@@ -0,0 +1,11 @@
+add_lldb_unittest(ScriptInterpreterLuaTests
+  LuaTests.cpp
+
+  LINK_LIBS
+lldbHost
+lldbPluginScriptInterpreterLua
+lldbPluginPlatformLinux
+LLVMTestingSupport
+  LINK_COMPONENTS
+Support
+  )
\ No newline at end of file
Index: lldb/unittests/ScriptInterpreter/CMakeLists.txt
===
--- lldb/unittests/ScriptInterpreter/CMakeLists.txt
+++ lldb/unittests/ScriptInterpreter/CMakeLists.txt
@@ -1,3 +1,6 @@
 if (LLDB_ENABLE_PYTHON)
   add_subdirectory(Python)
 endif()
+if (LLDB_ENABLE_LUA)
+  add_subdirectory(Lua)
+endif()
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -10,32 +10,114 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
-
+#include "lldb/Utility/Timer.h"
 #include "llvm/Support/Threading.h"
 
+#ifndef LLDB_DISABLE_LIBEDIT
+#include "lldb/Host/Editline.h"
+#endif
+
+#include "lua.hpp"
+
+#define LUA_PROMPT ">>> "
+
 #include 
 
 using namespace lldb;
 using namespace lldb_private;
 
+class Lua {
+public:
+  Lua() : m_lua_state(luaL_newstate()) {
+assert(m_lua_state);
+luaL_openlibs(m_lua_state);
+  }
+  ~Lua() {
+assert(m_lua_state);
+luaL_openlibs(m_lua_state);
+  }
+
+  llvm::Error Run(llvm::StringRef buffer) {
+int error =
+luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||
+

[Lldb-commits] [PATCH] D71234: [lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

2019-12-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 233220.
JDevlieghere added a comment.

Reuse IOHandlerEditline


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

https://reviews.llvm.org/D71234

Files:
  lldb/include/lldb/Core/IOHandler.h
  lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -10,32 +10,110 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
-
+#include "lldb/Utility/Timer.h"
 #include "llvm/Support/Threading.h"
 
+#ifndef LLDB_DISABLE_LIBEDIT
+#include "lldb/Host/Editline.h"
+#endif
+
+#include "lua.hpp"
+
+#define LUA_PROMPT ">>> "
+
 #include 
 
 using namespace lldb;
 using namespace lldb_private;
 
+class Lua {
+public:
+  Lua() : m_lua_state(luaL_newstate()) {
+assert(m_lua_state);
+luaL_openlibs(m_lua_state);
+  }
+  ~Lua() {
+assert(m_lua_state);
+luaL_openlibs(m_lua_state);
+  }
+
+  lua_State *State() { return m_lua_state; }
+
+private:
+  lua_State *m_lua_state = nullptr;
+};
+
+class IOHandlerLuaInterpreter : public IOHandlerDelegate,
+public IOHandlerEditline {
+public:
+  IOHandlerLuaInterpreter(Debugger )
+  : IOHandlerEditline(debugger, IOHandler::Type::LuaInterpreter, "lua",
+  ">>> ", "..> ", true, debugger.GetUseColor(), 0,
+  *this, nullptr),
+m_lua() {}
+
+  void IOHandlerInputComplete(IOHandler _handler,
+  std::string ) override {
+int error =
+luaL_loadbuffer(m_lua.State(), data.c_str(), data.size(), "line") ||
+lua_pcall(m_lua.State(), 0, 0, 0);
+if (error) {
+  fprintf(GetOutputFILE(), "%s\n", lua_tostring(m_lua.State(), -1));
+  // Pop error message from the stack.
+  lua_pop(m_lua.State(), 1);
+}
+  }
+
+  ~IOHandlerLuaInterpreter() override {}
+
+private:
+  Lua m_lua;
+};
+
 ScriptInterpreterLua::ScriptInterpreterLua(Debugger )
 : ScriptInterpreter(debugger, eScriptLanguageLua) {}
 
 ScriptInterpreterLua::~ScriptInterpreterLua() {}
 
 bool ScriptInterpreterLua::ExecuteOneLine(llvm::StringRef command,
-  CommandReturnObject *,
-  const ExecuteScriptOptions &) {
-  m_debugger.GetErrorStream().PutCString(
-  "error: the lua script interpreter is not yet implemented.\n");
+  CommandReturnObject *result,
+  const ExecuteScriptOptions ) {
+  Lua l;
+
+  // FIXME: Redirecting stdin, stdout and stderr.
+  int error =
+  luaL_loadbuffer(l.State(), command.data(), command.size(), "line") ||
+  lua_pcall(l.State(), 0, 0, 0);
+  if (error == 0)
+return true;
+
+  result->AppendErrorWithFormatv("lua failed attempting to evaluate '{0}'\n",
+ command);
   return false;
 }
 
 void ScriptInterpreterLua::ExecuteInterpreterLoop() {
-  m_debugger.GetErrorStream().PutCString(
-  "error: the lua script interpreter is not yet implemented.\n");
+  static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+  Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION);
+
+  Debugger  = m_debugger;
+
+  // At the moment, the only time the debugger does not have an input file
+  // handle is when this is called directly from lua, in which case it is
+  // both dangerous and unnecessary (not to mention confusing) to try to embed
+  // a running interpreter loop inside the already running lua interpreter
+  // loop, so we won't do it.
+
+  if (!debugger.GetInputFile().IsValid())
+return;
+
+  IOHandlerSP io_handler_sp(new IOHandlerLuaInterpreter(debugger));
+  if (io_handler_sp) {
+debugger.PushIOHandler(io_handler_sp);
+  }
 }
 
 void ScriptInterpreterLua::Initialize() {
Index: lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
+++ lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
@@ -4,4 +4,8 @@
   LINK_LIBS
 lldbCore
 lldbInterpreter
-  )
\ No newline at end of file
+  )
+
+find_package(Lua REQUIRED)
+target_include_directories(lldbPluginScriptInterpreterLua PRIVATE ${LUA_INCLUDE_DIR})
+target_link_libraries(lldbPluginScriptInterpreterLua PRIVATE ${LUA_LIBRARIES})
Index: lldb/include/lldb/Core/IOHandler.h
===
--- lldb/include/lldb/Core/IOHandler.h
+++ 

[Lldb-commits] [PATCH] D71234: [lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

2019-12-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

It looks like this patch already includes some non-trivial functionality, so I 
think this is a good time to start figuring out how to test this. Another 
thing, which may not be needed straight away, but which I think we should 
consider pretty early is creating some sort of c++ wrappers, similar to the 
PythonDataObject we have for python. This is so that we don't have C error 
handling code everywhere, but have things wrapped in a nicer interface with 
`llvm::Expected`s and everything.




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:47
+
+class IOHandlerLuaInterpreter : public IOHandler {
+public:

How much of this class is boiler plate? Can it be shared with something?



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:159
+  const ExecuteScriptOptions ) 
{
+  if (command.empty()) {
+result->AppendError("empty command passed to lua\n");

TBH, I am not sure how useful this error really is. Presumably you will get 
some sort of an error from the lua interpreter if you just let this pass..



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:167
+  // FIXME: Redirecting stdin, stdout and stderr.
+  int success = luaL_dostring(l.State(), command.data());
+  if (success == 0)

So what happens when `command` is not null terminated? I guess you ought to 
split this into `luaL_loadbuffer && lua_pcall`..



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:171
+
+  result->AppendErrorWithFormat("lua failed attempting to evaluate '%s'\n",
+command.data());

same here. You can just use `AppendErrorWithFormatv`..


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D71234



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D71234: [lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

2019-12-09 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added inline comments.



Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt:10
+find_package(Lua REQUIRED)
+include_directories(${LUA_INCLUDE_DIR})
+

Nit: I would use `target_include_directories` here.

https://cmake.org/cmake/help/latest/command/target_include_directories.html


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D71234



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits