Refactored keeper implementation of linda:receive()

6 files changed, 61 insertions, 53 deletions

diff --git a/docs/index.html b/docs/index.html
index 0cad4a0..1b24422 100644
--- a/docs/index.html
+++ b/docs/index.html
@@ -1224,7 +1224,8 @@
         The <tt>send()</tt> and <tt>receive()</tt> methods use Linda keys as FIFO stacks (first in, first out).<br/>
         By default, stack sizes are unlimited but limits can be enforced using the <tt>limit()</tt> method. This can be useful to balance execution speeds in a producer/consumer scenario. <tt>nil</tt> removes the limit.<br/>
         A limit of 0 is allowed to block everything.<br/>
-        If the key was full but the limit change added some room, <tt>limit()</tt> returns <tt>true</tt> and the linda is signalled so that <tt>send()</tt>-blocked threads are awakened.
+        If the key was full but the limit change added some room, <tt>limit()</tt> returns <tt>true</tt> and the linda is signalled so that <tt>send()</tt>-blocked threads are awakened.<br/>
+        In batched mode, <tt>linda:receive()</tt> will raise an error if <tt>min_count < 1</tt> or <tt>max_count < min_count</tt>.
 </p>
 
 <p>
diff --git a/src/keeper.cpp b/src/keeper.cpp
index a546868..b73533a 100644
--- a/src/keeper.cpp
+++ b/src/keeper.cpp
@@ -81,7 +81,7 @@ class KeyUD
     [[nodiscard]] static KeyUD* Create(KeeperState K_);
     [[nodiscard]] static KeyUD* GetPtr(KeeperState K_, int idx_);
     void peek(KeeperState K_, int count_);
-    void pop(KeeperState K_, int count_);
+    [[nodiscard]] int pop(KeeperState K_, int minCount_, int maxCount_);
     void prepareAccess(KeeperState K_);
     void push(KeeperState K_, int count_);
     [[nodiscard]] bool reset(KeeperState K_);
@@ -150,14 +150,21 @@ void KeyUD::peek(KeeperState const K_, int const count_)
 
 // in: fifo
 // out: remove the fifo table from the stack, push as many items as required on the stack (function assumes they exist in sufficient number)
-void KeyUD::pop(KeeperState const K_, int const count_)
+int KeyUD::pop(KeeperState const K_, int const minCount_, int const maxCount_)
 {
-    LUA_ASSERT(K_, lua_istable(K_, -1));
-    int const _fifo_idx{ lua_gettop(K_) };                                                         // K_: ... fifo
+    if (count < minCount_) {
+        // pop ourselves, return nothing
+        lua_pop(K_, 1);                                                                            // K_: ... this
+        return 0;
+    }
+    int const _popCount{ std::min(count, maxCount_) };
+    LUA_ASSERT(K_, KeyUD::GetPtr(K_, -1) == this);                                                 // K_: ... this
+    prepareAccess(K_);                                                                             // K_: ... fifo
+    int const _fifo_idx{ lua_gettop(K_) };
     // each iteration pushes a value on the stack!
-    STACK_GROW(K_, count_ + 2);
+    STACK_GROW(K_, _popCount + 2);
     // skip first item, we will push it last
-    for (int const _i : std::ranges::iota_view{ 1, count_ }) {
+    for (int const _i : std::ranges::iota_view{ 1, _popCount }) {
         int const _at{ first + _i };
         // push item on the stack
         lua_rawgeti(K_, _fifo_idx, _at);                                                           // K_: ... fifo val
@@ -166,20 +173,16 @@ void KeyUD::pop(KeeperState const K_, int const count_)
         lua_rawseti(K_, _fifo_idx, _at);                                                           // K_: ... fifo val
     }
     // now process first item
-    {
-        int const _at{ first };
-        lua_rawgeti(K_, _fifo_idx, _at);                                                           // K_: ... fifo vals val
-        lua_pushnil(K_);                                                                           // K_: ... fifo vals val nil
-        lua_rawseti(K_, _fifo_idx, _at);                                                           // K_: ... fifo vals val
-        lua_replace(K_, _fifo_idx);                                                                // K_: ... vals
-    }
+    lua_rawgeti(K_, _fifo_idx, first);                                                             // K_: ... fifo vals val
+    lua_pushnil(K_);                                                                               // K_: ... fifo vals val nil
+    lua_rawseti(K_, _fifo_idx, first);                                                             // K_: ... fifo vals val
+    lua_replace(K_, _fifo_idx);                                                                    // K_: ... vals
 
     // avoid ever-growing indexes by resetting each time we detect the fifo is empty
-    {
-        int const _new_count{ count - count_ };
-        first = (_new_count == 0) ? 1 : (first + count_);
-        count = _new_count;
-    }
+    int const _new_count{ count - _popCount };
+    first = (_new_count == 0) ? 1 : (first + _popCount);
+    count = _new_count;
+    return _popCount;
 }
 
 // #################################################################################################
@@ -466,21 +469,20 @@ int keepercall_receive(lua_State* const L_)
     PushKeysDB(_K, 1);                                                                             // _K: linda keys... KeysDB
     lua_replace(_K, 1);                                                                            // _K: KeysDB keys...
     
-    for (int const _i : std::ranges::iota_view{ 2, _top + 1 }) {
-        lua_pushvalue(_K, _i);                                                                     // _K: KeysDB keys... key[i]
+    for (int const _keyIdx : std::ranges::iota_view{ 2, _top + 1 }) {
+        lua_pushvalue(_K, _keyIdx);                                                                // _K: KeysDB keys... key[i]
         lua_rawget(_K, 1);                                                                         // _K: KeysDB keys... KeyUD
         KeyUD* const _key{ KeyUD::GetPtr(_K, -1) };
-        if (_key != nullptr && _key->count > 0) {
-            _key->prepareAccess(_K);                                                               // _K: KeysDB keys... fifo
-            _key->pop(_K, 1);                                                                      // _K: KeysDB keys... val
-            if (!lua_isnil(_K, -1)) {
+        if (_key != nullptr) { // it's fine to attempt a read on a key that wasn't yet written to
+            int const _popped{ _key->pop(_K, 1, 1) };                                              // _K: KeysDB keys... val
+            if (_popped > 0) {
                 lua_replace(_K, 1);                                                                // _K: val keys...
-                lua_settop(_K, _i);                                                                // _K: val keys... key[i]
-                if (_i != 2) {
-                    lua_replace(_K, 2);                                                            // _K: val key keys...
-                    lua_settop(_K, 2);                                                             // _K: val key
+                lua_settop(_K, _keyIdx);                                                           // _K: val keys... key[i]
+                if (_keyIdx != 2) {
+                    lua_replace(_K, 2);                                                            // _K: val key[i] keys...
+                    lua_settop(_K, 2);                                                             // _K: val key[i]
                 }
-                lua_insert(_K, 1);                                                                 // _K: key, val
+                lua_insert(_K, 1);                                                                 // _K: key val
                 return 2;
             }
         }
@@ -496,27 +498,23 @@ int keepercall_receive(lua_State* const L_)
 int keepercall_receive_batched(lua_State* const L_)
 {
     KeeperState const _K{ L_ };
+    // linda:receive() made sure that _min_count > 0 and _max_count > _min_count
     int const _min_count{ static_cast<int>(lua_tointeger(_K, 3)) };
-    if (_min_count > 0) {
-        int const _max_count{ static_cast<int>(luaL_optinteger(_K, 4, _min_count)) };
-        lua_settop(_K, 2);                                                                         // _K: linda key
-        lua_insert(_K, 1);                                                                         // _K: key linda
-        PushKeysDB(_K, 2);                                                                         // _K: key linda KeysDB
-        lua_remove(_K, 2);                                                                         // _K: key KeysDB
-        lua_pushvalue(_K, 1);                                                                      // _K: key KeysDB key
-        lua_rawget(_K, 2);                                                                         // _K: key KeysDB KeyUD
-        lua_remove(_K, 2);                                                                         // _K: key KeyUD
-        KeyUD* const _key{ KeyUD::GetPtr(_K, -1) };
-        if (_key != nullptr && _key->count >= _min_count) {
-            _key->prepareAccess(_K);                                                               // _K: key fifo
-            _key->pop(_K, std::min(_max_count, _key->count));                                      // _K: key val...
-        } else {
-            lua_settop(_K, 0);                                                                     // _K:
-        }
-        // return whatever remains on the stack at that point
-        return lua_gettop(_K);
-    } else {
+    int const _max_count{ static_cast<int>(luaL_optinteger(_K, 4, _min_count)) };
+    lua_settop(_K, 2);                                                                             // _K: linda key
+    lua_insert(_K, 1);                                                                             // _K: key linda
+    PushKeysDB(_K, 2);                                                                             // _K: key linda KeysDB
+    lua_remove(_K, 2);                                                                             // _K: key KeysDB
+    lua_pushvalue(_K, 1);                                                                          // _K: key KeysDB key
+    lua_rawget(_K, 2);                                                                             // _K: key KeysDB KeyUD
+    lua_remove(_K, 2);                                                                             // _K: key KeyUD
+    KeyUD* const _key{ KeyUD::GetPtr(_K, -1) };
+    if (_key == nullptr || _key->pop(_K, _min_count, _max_count) == 0) {                           // _K: [key val...]|crap
+        // Lua will adjust the stack for us when we return
         return 0;
+    } else {
+        // return whatever remains on the stack at that point: the key and the values we pulled from the fifo
+        return lua_gettop(_K);
     }
 }
 
diff --git a/src/linda.cpp b/src/linda.cpp
index 12b5e8a..b623d29 100644
--- a/src/linda.cpp
+++ b/src/linda.cpp
@@ -470,12 +470,15 @@ LUAG_FUNC(linda_receive)
             _selected_keeper_receive = KEEPER_API(receive_batched);
             // we expect a user-defined amount of return value
             _expected_pushed_min = (int) luaL_checkinteger(L_, _key_i + 1);
+            if (_expected_pushed_min < 1) {
+                raise_luaL_argerror(L_, _key_i + 1, "bad min count");
+            }
             _expected_pushed_max = (int) luaL_optinteger(L_, _key_i + 2, _expected_pushed_min);
             // don't forget to count the key in addition to the values
             ++_expected_pushed_min;
             ++_expected_pushed_max;
             if (_expected_pushed_min > _expected_pushed_max) {
-                raise_luaL_error(L_, "batched min/max error");
+                raise_luaL_argerror(L_, _key_i + 2, "batched min/max error");
             }
         } else {
             // make sure the keys are of a valid type
diff --git a/tests/basic.lua b/tests/basic.lua
index ecd93fc..8522895 100644
--- a/tests/basic.lua
+++ b/tests/basic.lua
@@ -433,7 +433,7 @@ local function chunk2(linda)
     assert(config.strip_functions and info.short_src=="?" or string.match(info.short_src, "^.*basic.lua$"), "bad info.short_src")
     -- These vary so let's not be picky (they're there..)
     --
-    assert(info.linedefined == 400, "bad linedefined")   -- start of 'chunk2'
+    assert(info.linedefined == 419, "bad linedefined")   -- start of 'chunk2'
     assert(config.strip_functions and info.currentline==-1 or info.currentline > info.linedefined, "bad currentline")   -- line of 'debug.getinfo'
     assert(info.lastlinedefined > info.currentline, "bad lastlinedefined")   -- end of 'chunk2'
     local k,func= linda:receive("down")
diff --git a/tests/errhangtest.lua b/tests/errhangtest.lua
index 99f44b2..d0ffcc4 100644
--- a/tests/errhangtest.lua
+++ b/tests/errhangtest.lua
@@ -1,4 +1,8 @@
-local lanes = require "lanes".configure{with_timers=false}
+local lanes = require "lanes".configure{with_timers=false,strip_functions=false}
+
+local require_assert_result_1, require_assert_result_2 = require "assert"    -- assert.fails()
+print("require_assert_result:", require_assert_result_1, require_assert_result_2)
+
 local linda = lanes.linda()
 
 -- we are not allowed to send coroutines through a lane
@@ -22,6 +26,9 @@ end
 -- get/set a few values
 if true then
         print "\n#### set 3 -> receive batched"
+        assert.fails(function() linda:receive(linda.batched, "some key", -1, 1) end)
+        assert.fails(function() linda:receive(linda.batched, "some key", 2, 1) end)
+        assert.failsnot(function() linda:receive(0, linda.batched, "some key", 1, 3) end)
         local fun = function() print "function test ok" end
         print(pcall(linda.set, linda, 'test', true, nil, fun))
         -- read back the contents
diff --git a/tests/linda_perf.lua b/tests/linda_perf.lua
index 4d35380..b173d12 100644
--- a/tests/linda_perf.lua
+++ b/tests/linda_perf.lua
@@ -124,7 +124,6 @@ do
         end
 end
 
-do return end
 -- #################################################################################################
 
 TEST1 = TEST1 or 1000