From 706313c5468cb6ace2debed6ac7056e63ee1dfbd Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Sat, 19 Apr 2025 17:18:00 +0200 Subject: [PATCH 1/3] Disable some clang-tidy warnings --- .clang-tidy | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index 1bae03965..b4e660b85 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,5 +1,5 @@ --- -Checks: '*,-abseil-*,-altera-*,-android-cloexec-*,-bugprone-easily-swappable-parameters,-cert-err58-cpp,-cppcoreguidelines-avoid-do-while,-cppcoreguidelines-avoid-magic-numbers,-cppcoreguidelines-avoid-non-const-global-variables,-cppcoreguidelines-owning-memory,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-type-cstyle-cast,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-type-static-cast-downcast,-cppcoreguidelines-pro-type-vararg,-fuchsia-*,-google-readability-casting,-google-readability-todo,-hicpp-named-parameter,-hicpp-no-array-decay,-hicpp-vararg,-llvm-include-order,-llvm-header-guard,-llvmlibc-*,-misc-include-cleaner,-misc-no-recursion,-modernize-use-nodiscard,-modernize-use-trailing-return-type,-readability-identifier-length,-readability-implicit-bool-conversion,-readability-named-parameter,-readability-magic-numbers' +Checks: '*,-abseil-*,-altera-*,-android-cloexec-*,-boost-use-ranges,-bugprone-chained-comparison,-bugprone-easily-swappable-parameters,-cert-err58-cpp,-cppcoreguidelines-avoid-do-while,-cppcoreguidelines-avoid-magic-numbers,-cppcoreguidelines-avoid-non-const-global-variables,-cppcoreguidelines-owning-memory,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-type-cstyle-cast,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-type-static-cast-downcast,-cppcoreguidelines-pro-type-vararg,-fuchsia-*,-google-readability-casting,-google-readability-todo,-hicpp-named-parameter,-hicpp-no-array-decay,-hicpp-vararg,-llvm-include-order,-llvm-header-guard,-llvmlibc-*,-misc-include-cleaner,-misc-no-recursion,-modernize-use-nodiscard,-modernize-use-trailing-return-type,-readability-identifier-length,-readability-implicit-bool-conversion,-readability-math-missing-parentheses,-readability-named-parameter,-readability-magic-numbers' # # cppcoreguidelines-pro-type-cstyle-cast # google-build-using-namespace @@ -7,6 +7,7 @@ Checks: '*,-abseil-*,-altera-*,-android-cloexec-*,-bugprone-easily-swappable-par # llvm-header-guard # llvm-include-order # hicpp-named-parameter +# readability-math-missing-parentheses # readability-named-parameter # Differ from our style guidelines # @@ -19,6 +20,12 @@ Checks: '*,-abseil-*,-altera-*,-android-cloexec-*,-bugprone-easily-swappable-par # android-cloexec-* # O_CLOEXEC isn't available on Windows # +# boost-use-ranges +# Not applicable, we don't want to rely on more code from boost. +# +# bugprone-chained-comparison +# Triggerd by the Catch library in many places. +# # bugprone-easily-swappable-parameters # Can't always be avoided. # From 7215cfa7531936618d849f3d23b068780c8f5f7b Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Sat, 19 Apr 2025 17:26:58 +0200 Subject: [PATCH 2/3] Move internal linkage code into anonymous namespace --- src/flex-lua-geom.cpp | 4 +- src/gen/gen-rivers.cpp | 4 +- src/gen/raster.cpp | 4 + src/output-flex.cpp | 96 +++++++++++----------- tests/test-lua-utils.cpp | 4 + tests/test-output-flex-example-configs.cpp | 4 +- tests/test-output-flex-update.cpp | 18 ++-- 7 files changed, 71 insertions(+), 63 deletions(-) diff --git a/src/flex-lua-geom.cpp b/src/flex-lua-geom.cpp index 03184b1b3..dc4772905 100644 --- a/src/flex-lua-geom.cpp +++ b/src/flex-lua-geom.cpp @@ -36,6 +36,8 @@ geom::geometry_t *unpack_geometry(lua_State *lua_state, int n) noexcept return static_cast(user_data); } +namespace { + /** * This function is called by Lua garbage collection when a geometry object * needs cleaning up. It calls the destructor of the C++ object. After that @@ -51,8 +53,6 @@ int geom_gc(lua_State *lua_state) noexcept return 0; } -namespace { - // The following functions are called when their counterparts in Lua are // called on geometry objects. diff --git a/src/gen/gen-rivers.cpp b/src/gen/gen-rivers.cpp index 66d028a20..8c94510fc 100644 --- a/src/gen/gen-rivers.cpp +++ b/src/gen/gen-rivers.cpp @@ -41,6 +41,8 @@ gen_rivers_t::gen_rivers_t(pg_conn_t *connection, bool append, params_t *params) get_params().get_string("src_areas"))); } +namespace { + /// The data for a graph edge in the waterway network. struct edge_t { @@ -76,8 +78,6 @@ bool operator<(geom::point_t a, edge_t const &b) noexcept return a < b.points[0]; } -namespace { - void follow_chain_and_set_width( edge_t const &edge, std::vector *edges, std::map const &node_order, diff --git a/src/gen/raster.cpp b/src/gen/raster.cpp index 9bbb4667a..26eddf5ff 100644 --- a/src/gen/raster.cpp +++ b/src/gen/raster.cpp @@ -16,12 +16,16 @@ #include +namespace { + template void append(std::string *str, T value) { str->append(reinterpret_cast(&value), sizeof(T)); } +} // anonymous namespace + void add_raster_header(std::string *wkb, wkb_raster_header const &data) { append(wkb, data.endianness); diff --git a/src/output-flex.cpp b/src/output-flex.cpp index 22e8aa2e6..e1f92215e 100644 --- a/src/output-flex.cpp +++ b/src/output-flex.cpp @@ -334,6 +334,54 @@ void check_for_object(lua_State *lua_state, char const *const function_name) function_name); } +/** + * Expects a Lua (hash) table on the stack, reads the field with name of the + * 'type' parameter which must be either nil or a Lua (array) table, in which + * case all (integer) ids in that table are reads into the 'ids' out + * parameter. + */ +void get_object_ids(lua_State *lua_state, char const *const type, idlist_t *ids) +{ + lua_getfield(lua_state, -1, type); + int const ltype = lua_type(lua_state, -1); + + if (ltype == LUA_TNIL) { + lua_pop(lua_state, 1); + return; + } + + if (ltype != LUA_TTABLE) { + lua_pop(lua_state, 1); + throw fmt_error( + "Table returned from select_relation_members() contains '{}' " + "field, but it isn't an array table.", + type); + } + + if (!luaX_is_array(lua_state)) { + lua_pop(lua_state, 1); + throw fmt_error( + "Table returned from select_relation_members() contains '{}' " + "field, but it isn't an array table.", + type); + } + + luaX_for_each(lua_state, [&]() { + osmid_t const id = lua_tointeger(lua_state, -1); + if (id == 0) { + throw fmt_error( + "Table returned from select_relation_members() contains " + "'{}' field, which must contain an array of non-zero " + "integer node ids.", + type); + } + + ids->push_back(id); + }); + + lua_pop(lua_state, 1); +} + } // anonymous namespace /** @@ -899,54 +947,6 @@ void output_flex_t::pending_way(osmid_t id) get_mutex_and_call_lua_function(m_process_way, m_way_cache.get()); } -/** - * Expects a Lua (hash) table on the stack, reads the field with name of the - * 'type' parameter which must be either nil or a Lua (array) table, in which - * case all (integer) ids in that table are reads into the 'ids' out - * parameter. - */ -void get_object_ids(lua_State *lua_state, char const *const type, idlist_t *ids) -{ - lua_getfield(lua_state, -1, type); - int const ltype = lua_type(lua_state, -1); - - if (ltype == LUA_TNIL) { - lua_pop(lua_state, 1); - return; - } - - if (ltype != LUA_TTABLE) { - lua_pop(lua_state, 1); - throw fmt_error( - "Table returned from select_relation_members() contains '{}' " - "field, but it isn't an array table.", - type); - } - - if (!luaX_is_array(lua_state)) { - lua_pop(lua_state, 1); - throw fmt_error( - "Table returned from select_relation_members() contains '{}' " - "field, but it isn't an array table.", - type); - } - - luaX_for_each(lua_state, [&]() { - osmid_t const id = lua_tointeger(lua_state, -1); - if (id == 0) { - throw fmt_error( - "Table returned from select_relation_members() contains " - "'{}' field, which must contain an array of non-zero " - "integer node ids.", - type); - } - - ids->push_back(id); - }); - - lua_pop(lua_state, 1); -} - void output_flex_t::select_relation_members() { if (!m_select_relation_members) { diff --git a/tests/test-lua-utils.cpp b/tests/test-lua-utils.cpp index 6da752042..49af80dcc 100644 --- a/tests/test-lua-utils.cpp +++ b/tests/test-lua-utils.cpp @@ -13,6 +13,8 @@ #include +namespace { + // Run the Lua code in "code" and then execute the function "func". template void test_lua(lua_State *lua_state, char const *code, FUNC&& func) { @@ -25,6 +27,8 @@ void test_lua(lua_State *lua_state, char const *code, FUNC&& func) { REQUIRE(lua_gettop(lua_state) == 0); } +} // anonymous namespace + TEST_CASE("check luaX_is_empty_table", "[NoDB]") { std::shared_ptr lua_state{ diff --git a/tests/test-output-flex-example-configs.cpp b/tests/test-output-flex-example-configs.cpp index e49c45542..3d1bcfba3 100644 --- a/tests/test-output-flex-example-configs.cpp +++ b/tests/test-output-flex-example-configs.cpp @@ -25,8 +25,6 @@ testing::db::import_t db; char const *const data_file = "liechtenstein-2013-08-03.osm.pbf"; -} // anonymous namespace - std::vector get_files() { // NOLINTNEXTLINE(concurrency-mt-unsafe) char const *env = std::getenv("EXAMPLE_FILES"); @@ -34,6 +32,8 @@ std::vector get_files() { return osmium::split_string(env, ',', true); } +} // anonymous namespace + TEST_CASE("minimal test for flex example configs") { auto const files = get_files(); diff --git a/tests/test-output-flex-update.cpp b/tests/test-output-flex-update.cpp index 4c31b31d1..a6a6b0d4f 100644 --- a/tests/test-output-flex-update.cpp +++ b/tests/test-output-flex-update.cpp @@ -19,6 +19,15 @@ testing::db::import_t db; char const *const conf_file = "test_output_flex.lua"; +// Return a string with the schema name prepended to the table name. +std::string with_schema(char const *table_name, options_t const &options) +{ + if (options.dbschema.empty()) { + return {table_name}; + } + return options.dbschema + "." + table_name; +} + } // anonymous namespace struct options_slim_default @@ -72,15 +81,6 @@ END } }; -// Return a string with the schema name prepended to the table name. -std::string with_schema(char const *table_name, options_t const &options) -{ - if (options.dbschema.empty()) { - return {table_name}; - } - return options.dbschema + "." + table_name; -} - TEMPLATE_TEST_CASE("updating a node", "", options_slim_default, options_slim_expire, options_slim_schema) { From 6a9e32562490dce2b57fa018822ef024ba5b1b3a Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Sat, 19 Apr 2025 17:44:53 +0200 Subject: [PATCH 3/3] Remove return with parameter in void function --- src/gen/gen-base.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gen/gen-base.cpp b/src/gen/gen-base.cpp index b14259f97..cdf553041 100644 --- a/src/gen/gen-base.cpp +++ b/src/gen/gen-base.cpp @@ -103,7 +103,7 @@ void gen_base_t::dbprepare(std::string_view stmt, std::string const &templ) { template_t sql_template{templ}; sql_template.set_params(get_params()); - return connection().prepare(stmt, fmt::runtime(sql_template.render())); + connection().prepare(stmt, fmt::runtime(sql_template.render())); } void gen_base_t::dbprepare(std::string_view stmt, params_t const &tmp_params, @@ -112,7 +112,7 @@ void gen_base_t::dbprepare(std::string_view stmt, params_t const &tmp_params, template_t sql_template{templ}; sql_template.set_params(get_params()); sql_template.set_params(tmp_params); - return connection().prepare(stmt, fmt::runtime(sql_template.render())); + connection().prepare(stmt, fmt::runtime(sql_template.render())); } void gen_base_t::raster_table_preprocess(std::string const &table)