lua-users home
lua-l archive

[Date Prev][Date Next][Thread Prev][Thread Next] [Date Index] [Thread Index]

I don't think that table.remove works correctly for tables which have
nil at their start. Sometimes it works, sometimes it doesn't. Here's
an example I could find which fails:

> a = {}; for i=1,100 do a[i] = i end
> -- add some nils to the front
> for i=1,75 do a[i] = nil end
> table.remove(a, 97)
> print(a[97]) -- should be 98, right?

Okay, looking at the code for table.remove, I think I see what's happening:

static int tremove (lua_State *L) {
  int e = aux_getn(L, 1);
  int pos = luaL_optint(L, 2, e);
  if (e == 0) return 0;  /* table is `empty' */
  luaL_setn(L, 1, e - 1);  /* t.n = n-1 */
  lua_rawgeti(L, 1, pos);  /* result = t[pos] */
  for ( ;pos<e; pos++) {     /* aha! this loop may not execute,
depending on whether
                                                the getn function returns. */
    lua_rawgeti(L, 1, pos+1);
    lua_rawseti(L, 1, pos);  /* t[pos] = t[pos+1] */
  lua_rawseti(L, 1, e);  /* t[e] = nil */
  return 1;

What about using the first encountered nil as the halting condition
for the loop? That would always work. Here it is in Lua:

function table.remove(t, ind)
  local next
     next = t[ind+1]
     t[ind] = next
     pos = pos+1
   until next==nil

Even if the behavior is not changed, I'd like to see a stronger
warning that table.remove relies on the length of the table being
tracked correctly.

Here's what the ref manual says currently:
Removes from table the element at position pos, shifting down other
elements to close the space, if necessary. Returns the value of the
removed element. The default value for pos is n, where n is the length
of the table, so that a call table.remove(t) removes the last element
of table t.

What about adding something like: 'table.remove relies on the length
of the table being tracked correctly, even when an explicit index is
specified. Thus, it may not work as expected if a table contains any
nil values.'