Merge pull request #1884 from joto/lua-array-helpers

Add helper functions for working with Lua arrays

Author: lonvia

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/flex-write.cpp

@@ -11,6 +11,7 @@
 #include "flex-write.hpp"
 #include "geom-functions.hpp"
 #include "json-writer.hpp"
+#include "lua-utils.hpp"
 #include "wkb.hpp"
 
 #include <algorithm>
@@ -131,47 +132,6 @@ static void write_double(db_copy_mgr_t<db_deleter_by_type_and_id_t> *copy_mgr,
 
 using table_register_type = std::vector<void const *>;
 
-/**
- * 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.
- */
-static bool is_lua_array(lua_State *lua_state)
-{
-    uint32_t n = 1;
-    lua_pushnil(lua_state);
-    while (lua_next(lua_state, -2) != 0) {
-        lua_pop(lua_state, 1); // remove value from stack
-#if LUA_VERSION_NUM >= 503
-        if (!lua_isinteger(lua_state, -1)) {
-            lua_pop(lua_state, 1);
-            return false;
-        }
-        int okay = 0;
-        auto const num = lua_tointegerx(lua_state, -1, &okay);
-        if (!okay || num != n++) {
-            lua_pop(lua_state, 1);
-            return false;
-        }
-#else
-        if (!lua_isnumber(lua_state, -1)) {
-            lua_pop(lua_state, 1);
-            return false;
-        }
-        double const num = lua_tonumber(lua_state, -1);
-        double intpart = 0.0;
-        if (std::modf(num, &intpart) != 0.0 || intpart < 0 ||
-            static_cast<uint32_t>(num) != n++) {
-            lua_pop(lua_state, 1);
-            return false;
-        }
-#endif
-    }
-
-    // An empty lua table could be both, we decide here that it is not stored
-    // as a JSON array but as a JSON object.
-    return n != 1;
-}
-
 static void write_json(json_writer_t *writer, lua_State *lua_state,
                        table_register_type *tables);
 
@@ -186,19 +146,21 @@ static void write_json_table(json_writer_t *writer, lua_State *lua_state,
     }
     tables->push_back(table_ptr);
 
-    if (is_lua_array(lua_state)) {
+    if (luaX_is_empty_table(lua_state)) {
+        // An empty lua table could be both, we decide here that it is not
+        // stored as a JSON array but as a JSON object.
+        writer->start_object();
+        writer->end_object();
+    } else if (luaX_is_array(lua_state)) {
         writer->start_array();
-        lua_pushnil(lua_state);
-        while (lua_next(lua_state, -2) != 0) {
+        luaX_for_each(lua_state, [&]() {
             write_json(writer, lua_state, tables);
             writer->next();
-            lua_pop(lua_state, 1);
-        }
+        });
         writer->end_array();
     } else {
         writer->start_object();
-        lua_pushnil(lua_state);
-        while (lua_next(lua_state, -2) != 0) {
+        luaX_for_each(lua_state, [&]() {
             int const ltype_key = lua_type(lua_state, -2);
             if (ltype_key != LUA_TSTRING) {
                 throw fmt_error("Incorrect data type '{}' as key.",
@@ -208,8 +170,7 @@ static void write_json_table(json_writer_t *writer, lua_State *lua_state,
             writer->key(key);
             write_json(writer, lua_state, tables);
             writer->next();
-            lua_pop(lua_state, 1);
-        }
+        });
         writer->end_object();
     }
 }
@@ -403,8 +364,7 @@ void flex_write_column(lua_State *lua_state,
         if (ltype == LUA_TTABLE) {
             copy_mgr->new_hash();
 
-            lua_pushnil(lua_state);
-            while (lua_next(lua_state, -2) != 0) {
+            luaX_for_each(lua_state, [&]() {
                 char const *const key = lua_tostring(lua_state, -2);
                 char const *const val = lua_tostring(lua_state, -1);
                 if (key == nullptr) {
@@ -422,8 +382,7 @@ void flex_write_column(lua_State *lua_state,
                         lua_typename(lua_state, ltype_value), key);
                 }
                 copy_mgr->add_hash_elem(key, val);
-                lua_pop(lua_state, 1);
-            }
+            });
 
             copy_mgr->finish_hash();
         } else {

src/geom-transform.cpp

@@ -11,6 +11,7 @@
 #include "geom-functions.hpp"
 #include "geom-transform.hpp"
 #include "logging.hpp"
+#include "lua-utils.hpp"
 
 #include <osmium/osm.hpp>
 
@@ -153,8 +154,7 @@ void init_geom_transform(geom_transform_t *transform, lua_State *lua_state)
     assert(transform);
     assert(lua_state);
 
-    lua_pushnil(lua_state);
-    while (lua_next(lua_state, -2) != 0) {
+    luaX_for_each(lua_state, [&]() {
         char const *const field = lua_tostring(lua_state, -2);
         if (field == nullptr) {
             throw std::runtime_error{"All fields in geometry transformation "
@@ -169,9 +169,7 @@ void init_geom_transform(geom_transform_t *transform, lua_State *lua_state)
                 show_warning = false;
             }
         }
-
-        lua_pop(lua_state, 1);
-    }
+    });
 }
 
 std::unique_ptr<geom_transform_t>

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)
 }