Merge pull request #905 from secondlife/require-tweaks

 Refactor `require()` to make it easier to reason about Lua stack usage.

3 files changed, 77 insertions, 58 deletions

diff --git a/indra/llcommon/lua_function.cpp b/indra/llcommon/lua_function.cpp
index a5f1f582d9..78abb8ba7e 100644
--- a/indra/llcommon/lua_function.cpp
+++ b/indra/llcommon/lua_function.cpp
@@ -789,7 +789,8 @@ std::ostream& operator<<(std::ostream& out, const lua_what& self)
 *****************************************************************************/
 std::ostream& operator<<(std::ostream& out, const lua_stack& self)
 {
-    const char* sep = "stack: [";
+    out << "stack: [";
+    const char* sep = "";
     for (int index = 1; index <= lua_gettop(self.L); ++index)
     {
         out << sep << lua_what(self.L, index);
diff --git a/indra/llcommon/lua_function.h b/indra/llcommon/lua_function.h
index 08a2353d29..07848e38af 100644
--- a/indra/llcommon/lua_function.h
+++ b/indra/llcommon/lua_function.h
@@ -17,6 +17,7 @@
 #include "luau/luaconf.h"
 #include "luau/lualib.h"
 #include "stringize.h"
+#include <exception>                // std::uncaught_exceptions()
 #include <memory>                   // std::shared_ptr
 #include <utility>                  // std::pair
 
@@ -216,4 +217,40 @@ private:
     lua_State* L;
 };
 
+// adapted from indra/test/debug.h
+// can't generalize Debug::operator() target because it's a variadic template
+class LuaLog
+{
+public:
+    template <typename... ARGS>
+    LuaLog(lua_State* L, ARGS&&... args):
+        L(L),
+        mBlock(stringize(std::forward<ARGS>(args)...))
+    {
+        (*this)("entry ", lua_stack(L));
+    }
+
+    // non-copyable
+    LuaLog(const LuaLog&) = delete;
+    LuaLog& operator=(const LuaLog&) = delete;
+
+    ~LuaLog()
+    {
+        auto exceptional{ std::uncaught_exceptions()? "exceptional " : "" };
+        (*this)(exceptional, "exit ", lua_stack(L));
+    }
+
+    template <typename... ARGS>
+    void operator()(ARGS&&... args)
+    {
+        LL_INFOS("Lua") << mBlock << ' ';
+        stream_to(LL_CONT, std::forward<ARGS>(args)...);
+        LL_ENDL;
+    }
+
+private:
+    lua_State* L;
+    const std::string mBlock;
+};
+
 #endif /* ! defined(LL_LUA_FUNCTION_H) */
diff --git a/indra/llcommon/tests/lleventcoro_test.cpp b/indra/llcommon/tests/lleventcoro_test.cpp
index c7a958da49..6ff895c1c1 100644
--- a/indra/llcommon/tests/lleventcoro_test.cpp
+++ b/indra/llcommon/tests/lleventcoro_test.cpp
@@ -113,30 +113,27 @@ namespace tut
 
     void test_data::explicit_wait(boost::shared_ptr<LLCoros::Promise<std::string>>& cbp)
     {
-        DEBUGIN
-        {
-            mSync.bump();
-            // The point of this test is to verify / illustrate suspending a
-            // coroutine for something other than an LLEventPump. In other
-            // words, this shows how to adapt to any async operation that
-            // provides a callback-style notification (and prove that it
-            // works).
+        DEBUG;
+        mSync.bump();
+        // The point of this test is to verify / illustrate suspending a
+        // coroutine for something other than an LLEventPump. In other
+        // words, this shows how to adapt to any async operation that
+        // provides a callback-style notification (and prove that it
+        // works).
 
-            // Perhaps we would send a request to a remote server and arrange
-            // for cbp->set_value() to be called on response.
-            // For test purposes, instead of handing 'callback' (or an
-            // adapter) off to some I/O subsystem, we'll just pass it back to
-            // our caller.
-            cbp = boost::make_shared<LLCoros::Promise<std::string>>();
-            LLCoros::Future<std::string> future = LLCoros::getFuture(*cbp);
+        // Perhaps we would send a request to a remote server and arrange
+        // for cbp->set_value() to be called on response.
+        // For test purposes, instead of handing 'callback' (or an
+        // adapter) off to some I/O subsystem, we'll just pass it back to
+        // our caller.
+        cbp = boost::make_shared<LLCoros::Promise<std::string>>();
+        LLCoros::Future<std::string> future = LLCoros::getFuture(*cbp);
 
-            // calling get() on the future causes us to suspend
-            debug("about to suspend");
-            stringdata = future.get();
-            mSync.bump();
-            ensure_equals("Got it", stringdata, "received");
-        }
-        DEBUGEND
+        // calling get() on the future causes us to suspend
+        debug("about to suspend");
+        stringdata = future.get();
+        mSync.bump();
+        ensure_equals("Got it", stringdata, "received");
     }
 
     template<> template<>
@@ -163,13 +160,9 @@ namespace tut
 
     void test_data::waitForEventOn1()
     {
-        DEBUGIN
-        {
-            mSync.bump();
-            result = suspendUntilEventOn("source");
-            mSync.bump();
-        }
-        DEBUGEND
+        mSync.bump();
+        result = suspendUntilEventOn("source");
+        mSync.bump();
     }
 
     template<> template<>
@@ -189,15 +182,11 @@ namespace tut
 
     void test_data::coroPump()
     {
-        DEBUGIN
-        {
-            mSync.bump();
-            LLCoroEventPump waiter;
-            replyName = waiter.getName();
-            result = waiter.suspend();
-            mSync.bump();
-        }
-        DEBUGEND
+        mSync.bump();
+        LLCoroEventPump waiter;
+        replyName = waiter.getName();
+        result = waiter.suspend();
+        mSync.bump();
     }
 
     template<> template<>
@@ -217,16 +206,12 @@ namespace tut
 
     void test_data::postAndWait1()
     {
-        DEBUGIN
-        {
-            mSync.bump();
-            result = postAndSuspend(LLSDMap("value", 17),       // request event
-                                 immediateAPI.getPump(),     // requestPump
-                                 "reply1",                   // replyPump
-                                 "reply");                   // request["reply"] = name
-            mSync.bump();
-        }
-        DEBUGEND
+        mSync.bump();
+        result = postAndSuspend(LLSDMap("value", 17),       // request event
+                                immediateAPI.getPump(),     // requestPump
+                                "reply1",                   // replyPump
+                                "reply");                   // request["reply"] = name
+        mSync.bump();
     }
 
     template<> template<>
@@ -240,15 +225,11 @@ namespace tut
 
     void test_data::coroPumpPost()
     {
-        DEBUGIN
-        {
-            mSync.bump();
-            LLCoroEventPump waiter;
-            result = waiter.postAndSuspend(LLSDMap("value", 17),
-                                        immediateAPI.getPump(), "reply");
-            mSync.bump();
-        }
-        DEBUGEND
+        mSync.bump();
+        LLCoroEventPump waiter;
+        result = waiter.postAndSuspend(LLSDMap("value", 17),
+                                       immediateAPI.getPump(), "reply");
+        mSync.bump();
     }
 
     template<> template<>