fix "ipairs() called on enum attr tables never end"

docs/changelog.txt

@@ -121,6 +121,7 @@ Template for new versions:
 
 ## Lua
 - Added ``dfhack.maps.forEachTile`` to scan a cuboid of tiles with a declarative filter and apply actions (count, set tiletype or per-tiletype replacements, set designation/occupancy fields, spawn constructions, Lua callback) in a single native call
+- Fixed ``ipairs()`` on enum attribute tables (e.g. ``df.item_type.attrs``) never terminating (issue #1860)
 
 ## Removed
 

library/LuaWrapper.cpp

@@ -1055,20 +1055,12 @@ static int meta_ptr_tostring(lua_State *state)
 }
 
 /**
- * Metamethod: __index for enum.attrs
+ * Push the attr entry for the given enum item value.
+ * Out-of-range values resolve to the default entry.
  */
-static int meta_enum_attr_index(lua_State *state)
+static void push_enum_attr(lua_State *state, enum_identity *id, int64_t idx)
 {
-    if (!lua_isnumber(state, 2))
-        lua_rawget(state, UPVAL_FIELDTABLE);
-    if (!lua_isnumber(state, 2))
-        luaL_error(state, "Invalid index in enum.attrs[]");
-
-    auto id = (enum_identity*)lua_touserdata(state, lua_upvalueindex(2));
-    auto *complex = id->getComplex();
-
-    int64_t idx = lua_tonumber(state, 2);
-    if (complex)
+    if (auto *complex = id->getComplex())
     {
         auto it = complex->value_index_map.find(idx);
         if (it != complex->value_index_map.end())
@@ -1087,6 +1079,20 @@ static int meta_enum_attr_index(lua_State *state)
     auto atype = id->getAttrType();
 
     push_object_internal(state, atype, ptr + unsigned(atype->byte_size()*idx));
+}
+
+/**
+ * Metamethod: __index for enum.attrs
+ */
+static int meta_enum_attr_index(lua_State *state)
+{
+    if (!lua_isnumber(state, 2))
+        lua_rawget(state, UPVAL_FIELDTABLE);
+    if (!lua_isnumber(state, 2))
+        luaL_error(state, "Invalid index in enum.attrs[]");
+
+    auto id = (enum_identity*)lua_touserdata(state, lua_upvalueindex(2));
+    push_enum_attr(state, id, lua_tonumber(state, 2));
     return 1;
 }
 
@@ -1462,6 +1468,83 @@ static int complex_enum_ipairs(lua_State *L)
     return 3;
 }
 
+/*
+ * enum.attrs ipairs() support
+ *
+ * enum.attrs[] returns a default entry for out-of-range indexes, so a plain
+ * ipairs() would never terminate. These iterators stop at the same place as
+ * ipairs() on the enum itself (DFHack/dfhack#1860).
+ *
+ * upvalues for the *_ipairs functions:
+ *  1: enum_identity
+ *  2: enum_identity::ComplexData (complex enums only)
+ *
+ * upvalues for the *_inext iterators (UPVAL_TYPETABLE is required by
+ * push_object_internal):
+ *  1: DFHACK_TYPETABLE
+ *  2: enum_identity (plain) or enum_identity::ComplexData (complex)
+ *  3: enum_identity (complex only)
+ */
+
+static int meta_enum_attr_inext(lua_State *L)
+{
+    auto id = (enum_identity*)lua_touserdata(L, lua_upvalueindex(2));
+    int64_t i = luaL_checkint(L, 2) + 1;
+    if (i <= id->getLastItem())
+    {
+        lua_pushinteger(L, i);
+        push_enum_attr(L, id, i);
+        return 2;
+    }
+    else
+    {
+        lua_pushnil(L);
+        return 1;
+    }
+}
+
+static int meta_enum_attr_ipairs(lua_State *L)
+{
+    auto id = (enum_identity*)lua_touserdata(L, lua_upvalueindex(1));
+    lua_rawgetp(L, LUA_REGISTRYINDEX, &DFHACK_TYPETABLE_TOKEN);
+    lua_pushvalue(L, lua_upvalueindex(1));
+    lua_pushcclosure(L, meta_enum_attr_inext, 2);
+    lua_pushnil(L);
+    lua_pushinteger(L, id->getFirstItem() - 1);
+    return 3;
+}
+
+static int complex_enum_attr_inext(lua_State *L)
+{
+    bool is_first = lua_isuserdata(L, 2);
+    int64_t i = (is_first)
+        ? ((enum_identity::ComplexData*)lua_touserdata(L, lua_upvalueindex(2)))->index_value_map[0]
+        : luaL_checkint(L, 2);
+    if (is_first || complex_enum_next_item_helper(L, i))
+    {
+        auto id = (enum_identity*)lua_touserdata(L, lua_upvalueindex(3));
+        lua_pushinteger(L, i);
+        push_enum_attr(L, id, i);
+        return 2;
+    }
+    else
+    {
+        lua_pushnil(L);
+        return 1;
+    }
+}
+
+static int complex_enum_attr_ipairs(lua_State *L)
+{
+    lua_rawgetp(L, LUA_REGISTRYINDEX, &DFHACK_TYPETABLE_TOKEN);
+    lua_pushvalue(L, lua_upvalueindex(2));
+    lua_pushvalue(L, lua_upvalueindex(1));
+    lua_pushcclosure(L, complex_enum_attr_inext, 3);
+    lua_pushnil(L);
+    lua_pushlightuserdata(L, (void*)1);
+    return 3;
+}
+
 
 static void RenderTypeChildren(lua_State *state, const std::vector<const compound_identity*> &children);
 
@@ -1562,7 +1645,21 @@ static void FillEnumKeys(lua_State *state, int ix_meta, int ftable, enum_identit
         lua_pushvalue(state, base+1);
         lua_pushcclosure(state, meta_enum_attr_index, 3);
 
-        freeze_table(state, false, (eid->getFullName()+".attrs").c_str());
+        freeze_table(state, true, (eid->getFullName()+".attrs").c_str());
+
+        lua_pushlightuserdata(state, eid);
+        if (complex)
+        {
+            lua_pushlightuserdata(state, (void*)complex);
+            lua_pushcclosure(state, complex_enum_attr_ipairs, 2);
+        }
+        else
+        {
+            lua_pushcclosure(state, meta_enum_attr_ipairs, 1);
+        }
+        lua_setfield(state, -2, "__ipairs");
+        lua_pop(state, 1);
+
         lua_setfield(state, ftable, "attrs");
     }
 

test/structures/enum_attrs.lua

@@ -0,0 +1,54 @@
+config.target = 'core'
+
+-- https://github.com/DFHack/dfhack/issues/1860
+-- enum.attrs indexes never return nil (out-of-range indexes return a default
+-- entry), so ipairs() on an attrs table needs a bounded __ipairs that stops
+-- where ipairs() on the enum itself stops
+
+local function enum_keys(enum)
+    local keys = {}
+    for i in ipairs(enum) do
+        table.insert(keys, i)
+    end
+    return keys
+end
+
+-- iterate attrs with a hard cap so a regression fails instead of hanging
+local function attrs_keys(enum, limit)
+    local keys, values = {}, {}
+    for i, attrs in ipairs(enum.attrs) do
+        table.insert(keys, i)
+        values[i] = attrs
+        if #keys > limit then
+            return keys, values, false
+        end
+    end
+    return keys, values, true
+end
+
+local function check_attrs_ipairs(enum, name)
+    local expected = enum_keys(enum)
+    local keys, values, terminated = attrs_keys(enum, #expected + 1)
+    expect.true_(terminated, 'ipairs(' .. name .. '.attrs) did not terminate')
+    expect.table_eq(expected, keys)
+    for _, i in ipairs(keys) do
+        expect.eq(values[i], enum.attrs[i],
+                  'wrong attrs entry yielded for ' .. name .. '[' .. i .. ']')
+    end
+end
+
+function test.plain_enum()
+    check_attrs_ipairs(df.item_type, 'df.item_type')
+end
+
+function test.complex_enum()
+    expect.true_(df.pronoun_type._complex)
+    check_attrs_ipairs(df.pronoun_type, 'df.pronoun_type')
+end
+
+function test.out_of_range_index()
+    -- invalid indexes must keep returning the default entry
+    expect.true_(df.item_type.attrs[df.item_type._last_item + 1])
+    expect.eq(df.item_type.attrs[df.item_type._last_item + 1],
+              df.item_type.attrs[df.item_type._last_item + 100])
+end