Introduce new Lua helper fuctions for working with Lua arrays

New functions luaX_is_empty_table(), luaX_is_array() and luaX_for_each()
take some of the boilerplate out of working with Lua arrays.

Author: joto

src/flex-lua-index.cpp

@@ -32,19 +32,18 @@ static void check_and_add_columns(flex_table_t const &table,
                                   std::vector<std::string> *columns,
                                   lua_State *lua_state)
 {
-    lua_pushnil(lua_state);
-    while (lua_next(lua_state, -2) != 0) {
-        if (!lua_isnumber(lua_state, -2)) {
-            throw std::runtime_error{
-                "The 'column' field must contain a string or an array."};
-        }
+    if (!luaX_is_array(lua_state)) {
+        throw std::runtime_error{
+            "The 'column' field must contain a string or an array."};
+    }
+
+    luaX_for_each(lua_state, [&]() {
         if (!lua_isstring(lua_state, -1)) {
             throw std::runtime_error{
                 "The entries in the 'column' array must be strings."};
         }
         check_and_add_column(table, columns, lua_tostring(lua_state, -1));
-        lua_pop(lua_state, 1); // table
-    }
+    });
 }
 
 void flex_lua_setup_index(lua_State *lua_state, flex_table_t *table)

src/lua-utils.cpp

@@ -202,3 +202,34 @@ int luaX_pcall(lua_State *lua_state, int narg, int nres)
 }
 
 #endif
+
+bool luaX_is_empty_table(lua_State *lua_state)
+{
+    assert(lua_istable(lua_state, -1));
+    lua_pushnil(lua_state);
+    if (lua_next(lua_state, -2) == 0) {
+        return true;
+    }
+    lua_pop(lua_state, 2);
+    return false;
+}
+
+bool luaX_is_array(lua_State *lua_state)
+{
+    // Checking that a Lua table is an array is surprisingly difficult.
+    // This code is based on:
+    // https://web.archive.org/web/20140227143701/http://ericjmritz.name/2014/02/26/lua-is_array/
+    assert(lua_istable(lua_state, -1));
+    int i = 0;
+    lua_pushnil(lua_state);
+    while (lua_next(lua_state, -2) != 0) {
+        ++i;
+        lua_rawgeti(lua_state, -3, i);
+        if (lua_isnil(lua_state, -1)) {
+            lua_pop(lua_state, 3);
+            return false;
+        }
+        lua_pop(lua_state, 2);
+    }
+    return true;
+}

src/lua-utils.hpp

@@ -18,6 +18,7 @@ extern "C"
 #include <lua.h>
 }
 
+#include <cassert>
 #include <cstdint>
 #include <utility>
 
@@ -64,4 +65,47 @@ bool luaX_get_table_bool(lua_State *lua_state, char const *key, int table_index,
 
 int luaX_pcall(lua_State *lua_state, int narg, int nres);
 
+/**
+ * Returns true if the value on top of the stack is an empty Lua table.
+ *
+ * \pre Value on top of the Lua stack must be a Lua table.
+ * \post Stack is unchanged.
+ */
+bool luaX_is_empty_table(lua_State *lua_state);
+
+/**
+ * Check that the value on the top of the Lua stack is a simple array.
+ * This means that all keys must be consecutive integers starting from 1.
+ *
+ * \returns True if this is an array (also for Lua tables without any items)
+ *
+ * \pre Value on top of the Lua stack must be a Lua table.
+ * \post Stack is unchanged.
+ */
+bool luaX_is_array(lua_State *lua_state);
+
+/**
+ * Call a function for each item in a Lua array table. The item value will
+ * be on the top of the stack inside that function.
+ *
+ * \pre Value on top of the Lua stack must be a Lua array table.
+ * \pre The function must leave the Lua stack in the same condition it found
+ *      it in.
+ * \post Stack is unchanged.
+ */
+template <typename FUNC>
+void luaX_for_each(lua_State *lua_state, FUNC &&func)
+{
+    assert(lua_istable(lua_state, -1));
+    lua_pushnil(lua_state);
+    while (lua_next(lua_state, -2) != 0) {
+#ifndef NDEBUG
+        int const top = lua_gettop(lua_state);
+#endif
+        std::forward<FUNC>(func)();
+        assert(top == lua_gettop(lua_state));
+        lua_pop(lua_state, 1);
+    }
+}
+
 #endif // OSM2PGSQL_LUA_UTILS_HPP

src/output-flex.cpp

@@ -577,13 +577,11 @@ void output_flex_t::setup_flex_table_columns(flex_table_t *table)
                         table->name());
     }
 
+    if (!luaX_is_array(lua_state())) {
+        throw std::runtime_error{"The 'columns' field must contain an array."};
+    }
     std::size_t num_columns = 0;
-    lua_pushnil(lua_state());
-    while (lua_next(lua_state(), -2) != 0) {
-        if (!lua_isnumber(lua_state(), -2)) {
-            throw std::runtime_error{
-                "The 'columns' field must contain an array."};
-        }
+    luaX_for_each(lua_state(), [&]() {
         if (!lua_istable(lua_state(), -1)) {
             throw std::runtime_error{
                 "The entries in the 'columns' array must be tables."};
@@ -611,16 +609,15 @@ void output_flex_t::setup_flex_table_columns(flex_table_t *table)
                 column.type() == table_column_type::area) {
                 column.set_projection(lua_tostring(lua_state(), -1));
             } else {
-                throw std::runtime_error{
-                    "Projection can only be set on geometry and area columns."};
+                throw std::runtime_error{"Projection can only be set on "
+                                         "geometry and area columns."};
             }
         }
 
-        // stack has: projection, create_only, not_null, sql_type, column,
-        //            type, table
-        lua_pop(lua_state(), 7);
+        // stack has: projection, create_only, not_null, sql_type, column, type
+        lua_pop(lua_state(), 6);
         ++num_columns;
-    }
+    });
 
     if (num_columns == 0 && !table->has_id_column()) {
         throw fmt_error("No columns defined for table '{}'.", table->name());
@@ -655,21 +652,18 @@ void output_flex_t::setup_indexes(flex_table_t *table)
                         table->name());
     }
 
-    lua_pushnil(lua_state());
-    while (lua_next(lua_state(), -2) != 0) {
-        if (!lua_isnumber(lua_state(), -2)) {
-            throw std::runtime_error{
-                "The 'indexes' field must contain an array."};
-        }
+    if (!luaX_is_array(lua_state())) {
+        throw std::runtime_error{"The 'indexes' field must contain an array."};
+    }
+
+    luaX_for_each(lua_state(), [&]() {
         if (!lua_istable(lua_state(), -1)) {
             throw std::runtime_error{
                 "The entries in the 'indexes' array must be Lua tables."};
         }
 
         flex_lua_setup_index(lua_state(), table);
-
-        lua_pop(lua_state(), 1);
-    }
+    });
 
     lua_pop(lua_state(), 1); // "indexes"
 }
@@ -1179,25 +1173,24 @@ void output_flex_t::select_relation_members()
     }
 
     // Iterate over the 'ways' table to get all ids...
-    lua_pushnil(lua_state());
-    while (lua_next(lua_state(), -2) != 0) {
-        if (!lua_isnumber(lua_state(), -2)) {
-            throw std::runtime_error{
-                "Table returned from select_relation_members() contains 'ways' "
-                "field, but it isn't an array table."};
-        }
+    if (!luaX_is_array(lua_state())) {
+        throw std::runtime_error{
+            "Table returned from select_relation_members() contains 'ways' "
+            "field, but it isn't an array table."};
+    }
 
-        osmid_t const id = lua_tointeger(lua_state(), -1);
-        if (id == 0) {
-            throw std::runtime_error{
-                "Table returned from select_relation_members() contains 'ways' "
-                "field, which must contain an array of non-zero integer way "
-                "ids."};
-        }
+    luaX_for_each(
+        lua_state(), [&]() {
+            osmid_t const id = lua_tointeger(lua_state(), -1);
+            if (id == 0) {
+                throw std::runtime_error{
+                    "Table returned from select_relation_members() contains "
+                    "'ways' field, which must contain an array of non-zero "
+                    "integer way ids."};
+            }
 
-        m_stage2_way_ids->set(id);
-        lua_pop(lua_state(), 1); // value pushed by lua_next()
-    }
+            m_stage2_way_ids->set(id);
+        });
 
     lua_pop(lua_state(), 2); // return value (a table), ways field (a table)
 }

tests/CMakeLists.txt

@@ -85,6 +85,7 @@ set_test(test-wkb LABELS NoDB)
 # these tests require LUA support
 if (WITH_LUA)
     set_test(test-flex-indexes)
+    set_test(test-lua-utils)
     set_test(test-output-flex)
     set_test(test-output-flex-multi-input)
     set_test(test-output-flex-nodes)

tests/test-lua-utils.cpp

@@ -0,0 +1,108 @@
+/**
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This file is part of osm2pgsql (https://osm2pgsql.org/).
+ *
+ * Copyright (C) 2006-2022 by the osm2pgsql developer community.
+ * For a full list of authors see the git log.
+ */
+
+#include <catch.hpp>
+
+#include "lua-utils.hpp"
+
+extern "C"
+{
+#include <lauxlib.h>
+}
+
+// Run the Lua code in "code" and then execute the function "func".
+template <typename FUNC>
+void test_lua(lua_State *lua_state, char const *code, FUNC&& func) {
+    REQUIRE(lua_gettop(lua_state) == 0);
+    REQUIRE(luaL_dostring(lua_state, code) == 0);
+    REQUIRE(lua_gettop(lua_state) == 1);
+    std::forward<FUNC>(func)();
+    REQUIRE(lua_gettop(lua_state) == 1);
+    lua_pop(lua_state, 1); // result from executing the Lua code
+    REQUIRE(lua_gettop(lua_state) == 0);
+}
+
+TEST_CASE("check luaX_is_empty_table", "[NoDB]")
+{
+    std::shared_ptr<lua_State> lua_state{
+        luaL_newstate(), [](lua_State *state) { lua_close(state); }};
+
+    test_lua(lua_state.get(), "return {}", [&](){
+        REQUIRE(luaX_is_empty_table(lua_state.get()));
+    });
+
+    test_lua(lua_state.get(), "return { 1 }", [&](){
+        REQUIRE_FALSE(luaX_is_empty_table(lua_state.get()));
+    });
+
+    test_lua(lua_state.get(), "return { a = 'b' }", [&](){
+        REQUIRE_FALSE(luaX_is_empty_table(lua_state.get()));
+    });
+}
+
+TEST_CASE("check luaX_is_array", "[NoDB]")
+{
+    std::shared_ptr<lua_State> lua_state{
+        luaL_newstate(), [](lua_State *state) { lua_close(state); }};
+
+    test_lua(lua_state.get(), "return { 1, 2, 3 }", [&](){
+        REQUIRE(luaX_is_array(lua_state.get()));
+    });
+
+    test_lua(lua_state.get(), "return { }", [&](){
+        REQUIRE(luaX_is_array(lua_state.get()));
+    });
+
+    test_lua(lua_state.get(), "return { 1 }", [&](){
+        REQUIRE(luaX_is_array(lua_state.get()));
+    });
+
+    test_lua(lua_state.get(), "return { [1] = 1, [2] = 2 }", [&](){
+        REQUIRE(luaX_is_array(lua_state.get()));
+    });
+
+    test_lua(lua_state.get(), "return { 1, nil, 3 }", [&](){
+        REQUIRE_FALSE(luaX_is_array(lua_state.get()));
+    });
+
+    test_lua(lua_state.get(), "return { a = 'foo' }", [&](){
+        REQUIRE_FALSE(luaX_is_array(lua_state.get()));
+    });
+
+    test_lua(lua_state.get(), "return { [1] = 'foo', ['bar'] = 2 }", [&](){
+        REQUIRE_FALSE(luaX_is_array(lua_state.get()));
+    });
+}
+
+TEST_CASE("luaX_for_each should call function n times", "[NoDB]")
+{
+    std::shared_ptr<lua_State> lua_state{
+        luaL_newstate(), [](lua_State *state) { lua_close(state); }};
+
+    test_lua(lua_state.get(), "return { 3, 4, 5 }", [&](){
+        int sum = 0;
+        luaX_for_each(lua_state.get(), [&]() {
+            sum += lua_tonumber(lua_state.get(), -1);
+        });
+        REQUIRE(sum == 12);
+    });
+}
+
+TEST_CASE("luaX_for_each should not call the function for empty arrays",
+          "[NoDB]")
+{
+    std::shared_ptr<lua_State> lua_state{
+        luaL_newstate(), [](lua_State *state) { lua_close(state); }};
+
+    bool called = false;
+    test_lua(lua_state.get(), "return {}", [&]() {
+        luaX_for_each(lua_state.get(), [&]() { called = true; });
+    });
+    REQUIRE_FALSE(called);
+}