From 3c5d105e5252c40d47c8d427f323fb9a101e0489 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Fri, 30 May 2025 15:05:54 +0200 Subject: [PATCH 1/5] Don't use __tostring as function name Because names with two underscores are reserved in C++. --- src/flex-lua-expire-output.cpp | 6 +++--- src/flex-lua-expire-output.hpp | 2 +- src/flex-lua-locator.cpp | 6 +++--- src/flex-lua-locator.hpp | 2 +- src/flex-lua-table.cpp | 6 +++--- src/flex-lua-table.hpp | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/flex-lua-expire-output.cpp b/src/flex-lua-expire-output.cpp index 1ec5f23d3..59cc0c222 100644 --- a/src/flex-lua-expire-output.cpp +++ b/src/flex-lua-expire-output.cpp @@ -65,7 +65,7 @@ create_expire_output(lua_State *lua_state, std::string const &default_schema, return new_expire_output; } -TRAMPOLINE_WRAPPED_OBJECT(expire_output, __tostring) +TRAMPOLINE_WRAPPED_OBJECT(expire_output, tostring) TRAMPOLINE_WRAPPED_OBJECT(expire_output, filename) TRAMPOLINE_WRAPPED_OBJECT(expire_output, maxzoom) TRAMPOLINE_WRAPPED_OBJECT(expire_output, minzoom) @@ -112,7 +112,7 @@ void lua_wrapper_expire_output::init(lua_State *lua_state) lua_pushvalue(lua_state, -1); lua_setfield(lua_state, -2, "__index"); luaX_add_table_func(lua_state, "__tostring", - lua_trampoline_expire_output___tostring); + lua_trampoline_expire_output_tostring); luaX_add_table_func(lua_state, "filename", lua_trampoline_expire_output_filename); luaX_add_table_func(lua_state, "maxzoom", @@ -126,7 +126,7 @@ void lua_wrapper_expire_output::init(lua_State *lua_state) lua_pop(lua_state, 2); } -int lua_wrapper_expire_output::__tostring() const +int lua_wrapper_expire_output::tostring() const { std::string const str = fmt::format("osm2pgsql.ExpireOutput[minzoom={},maxzoom={},filename={}," diff --git a/src/flex-lua-expire-output.hpp b/src/flex-lua-expire-output.hpp index aa3bfac76..809a8ebc4 100644 --- a/src/flex-lua-expire-output.hpp +++ b/src/flex-lua-expire-output.hpp @@ -36,7 +36,7 @@ class lua_wrapper_expire_output : public lua_wrapper_base { } - int __tostring() const; + int tostring() const; int filename() const; int maxzoom() const; int minzoom() const; diff --git a/src/flex-lua-locator.cpp b/src/flex-lua-locator.cpp index 536fdf1cc..f5bf027e6 100644 --- a/src/flex-lua-locator.cpp +++ b/src/flex-lua-locator.cpp @@ -36,7 +36,7 @@ locator_t &create_locator(lua_State *lua_state, return new_locator; } -TRAMPOLINE_WRAPPED_OBJECT(locator, __tostring) +TRAMPOLINE_WRAPPED_OBJECT(locator, tostring) TRAMPOLINE_WRAPPED_OBJECT(locator, name) TRAMPOLINE_WRAPPED_OBJECT(locator, add_bbox) TRAMPOLINE_WRAPPED_OBJECT(locator, add_from_db) @@ -83,7 +83,7 @@ void lua_wrapper_locator::init(lua_State *lua_state, lua_pushvalue(lua_state, -1); lua_setfield(lua_state, -2, "__index"); luaX_add_table_func(lua_state, "__tostring", - lua_trampoline_locator___tostring); + lua_trampoline_locator_tostring); luaX_add_table_func(lua_state, "name", lua_trampoline_locator_name); luaX_add_table_func(lua_state, "add_bbox", lua_trampoline_locator_add_bbox); luaX_add_table_func(lua_state, "add_from_db", @@ -96,7 +96,7 @@ void lua_wrapper_locator::init(lua_State *lua_state, lua_pop(lua_state, 2); } -int lua_wrapper_locator::__tostring() const +int lua_wrapper_locator::tostring() const { std::string const str{fmt::format("osm2pgsql.Locator[name={},size={}]", self().name(), self().size())}; diff --git a/src/flex-lua-locator.hpp b/src/flex-lua-locator.hpp index cf560702a..80f07714d 100644 --- a/src/flex-lua-locator.hpp +++ b/src/flex-lua-locator.hpp @@ -38,7 +38,7 @@ class lua_wrapper_locator : public lua_wrapper_base { } - int __tostring() const; + int tostring() const; int name() const; int add_bbox(); int add_from_db(); diff --git a/src/flex-lua-table.cpp b/src/flex-lua-table.cpp index 3fca95e6f..9932b3c2c 100644 --- a/src/flex-lua-table.cpp +++ b/src/flex-lua-table.cpp @@ -418,7 +418,7 @@ void setup_flex_table_indexes(lua_State *lua_state, flex_table_t *table, lua_pop(lua_state, 1); // "indexes" } -TRAMPOLINE_WRAPPED_OBJECT(table, __tostring) +TRAMPOLINE_WRAPPED_OBJECT(table, tostring) TRAMPOLINE_WRAPPED_OBJECT(table, cluster) TRAMPOLINE_WRAPPED_OBJECT(table, columns) TRAMPOLINE_WRAPPED_OBJECT(table, name) @@ -469,7 +469,7 @@ void lua_wrapper_table::init(lua_State *lua_state) lua_pushvalue(lua_state, -1); lua_setfield(lua_state, -2, "__index"); luaX_add_table_func(lua_state, "__tostring", - lua_trampoline_table___tostring); + lua_trampoline_table_tostring); luaX_add_table_func(lua_state, "insert", lua_trampoline_table_insert); luaX_add_table_func(lua_state, "name", lua_trampoline_table_name); luaX_add_table_func(lua_state, "schema", lua_trampoline_table_schema); @@ -479,7 +479,7 @@ void lua_wrapper_table::init(lua_State *lua_state) lua_pop(lua_state, 2); } -int lua_wrapper_table::__tostring() const +int lua_wrapper_table::tostring() const { std::string const str{fmt::format("osm2pgsql.Table[{}]", self().name())}; luaX_pushstring(lua_state(), str); diff --git a/src/flex-lua-table.hpp b/src/flex-lua-table.hpp index 6e83634ea..32b17d564 100644 --- a/src/flex-lua-table.hpp +++ b/src/flex-lua-table.hpp @@ -36,7 +36,7 @@ class lua_wrapper_table : public lua_wrapper_base { } - int __tostring() const; + int tostring() const; int cluster() const; int columns() const; int name() const; From 570ed8c88b38f898b44e4da6d2ae642b1571f6dc Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Fri, 30 May 2025 15:14:48 +0200 Subject: [PATCH 2/5] Supress unhelpful clang-tidy warning --- src/flex-lua-wrapper.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/flex-lua-wrapper.hpp b/src/flex-lua-wrapper.hpp index 6f590d532..0b63fd704 100644 --- a/src/flex-lua-wrapper.hpp +++ b/src/flex-lua-wrapper.hpp @@ -15,6 +15,7 @@ #include #include +// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) #define TRAMPOLINE_WRAPPED_OBJECT(obj_name, func_name) \ int lua_trampoline_##obj_name##_##func_name(lua_State *lua_state) \ { \ From 2e8448c8f35f0ef55810cbd579185ed0d2fcc275 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Fri, 30 May 2025 17:00:43 +0200 Subject: [PATCH 3/5] Use const ref instead of forwarding ref Because we are not forwarding. --- src/params.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/params.hpp b/src/params.hpp index 8337303d7..c5bff5975 100644 --- a/src/params.hpp +++ b/src/params.hpp @@ -67,7 +67,7 @@ class params_t typename K, typename V, std::enable_if_t>, bool> = true> - void set(K &&key, V &&value) + void set(K &&key, V const &value) { m_map.insert_or_assign(std::forward(key), std::string(value)); } From 734f2ad67ed473c91e2b4434649b1fb417380a02 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Fri, 30 May 2025 19:45:00 +0200 Subject: [PATCH 4/5] Use anonymous namespace for local linkage --- src/logging.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/logging.cpp b/src/logging.cpp index 3ebb0306a..e5e149490 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -11,11 +11,15 @@ #include +namespace { + thread_local unsigned int this_thread_num = 0; /// Global logger singleton logger the_logger{}; +} // anonymous namespace + /// Access the global logger singleton logger &get_logger() noexcept { return the_logger; } From fcf1ddd86d5f42f0234514960479706b09374e20 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Fri, 30 May 2025 17:54:36 +0200 Subject: [PATCH 5/5] Use std::min/max instead of hand-rolling the check clang-tidy check readability-use-std-min-max Apparently C++ compilers are good enough to optimize this properly https://stackoverflow.com/questions/15540250/in-c-is-it-better-to-cap-a-value-using-stdmin-or-an-if-branch --- src/gen/gen-discrete-isolation.cpp | 4 +--- src/geom-box.cpp | 34 ++++++++-------------------- src/geom-pole-of-inaccessibility.cpp | 8 ++----- 3 files changed, 12 insertions(+), 34 deletions(-) diff --git a/src/gen/gen-discrete-isolation.cpp b/src/gen/gen-discrete-isolation.cpp index 39a6b06ef..2ee6d2cf9 100644 --- a/src/gen/gen-discrete-isolation.cpp +++ b/src/gen/gen-discrete-isolation.cpp @@ -113,9 +113,7 @@ FROM {src} WHERE {importance_column} > 0 double const dx = coords[m].first - coords[n].first; double const dy = coords[m].second - coords[n].second; double const dist = dx * dx + dy * dy; - if (dist < min) { - min = dist; - } + min = std::min(dist, min); } data[n].di = std::sqrt(min); } diff --git a/src/geom-box.cpp b/src/geom-box.cpp index 9e80d957e..09812380d 100644 --- a/src/geom-box.cpp +++ b/src/geom-box.cpp @@ -9,25 +9,17 @@ #include "geom-box.hpp" +#include #include namespace geom { box_t &box_t::extend(point_t const &point) noexcept { - if (point.x() < m_min_x) { - m_min_x = point.x(); - } - if (point.y() < m_min_y) { - m_min_y = point.y(); - } - if (point.x() > m_max_x) { - m_max_x = point.x(); - } - if (point.y() > m_max_y) { - m_max_y = point.y(); - } - + m_min_x = std::min(point.x(), m_min_x); + m_min_y = std::min(point.y(), m_min_y); + m_max_x = std::max(point.x(), m_max_x); + m_max_y = std::max(point.y(), m_max_y); return *this; } @@ -40,18 +32,10 @@ void box_t::extend(point_list_t const &list) noexcept void box_t::extend(box_t const &box) noexcept { - if (box.min_x() < m_min_x) { - m_min_x = box.min_x(); - } - if (box.min_y() < m_min_y) { - m_min_y = box.min_y(); - } - if (box.max_x() > m_max_x) { - m_max_x = box.max_x(); - } - if (box.max_y() > m_max_y) { - m_max_y = box.max_y(); - } + m_min_x = std::min(box.min_x(), m_min_x); + m_min_y = std::min(box.min_y(), m_min_y); + m_max_x = std::max(box.max_x(), m_max_x); + m_max_y = std::max(box.max_y(), m_max_y); } box_t envelope(geom::nullgeom_t const & /*geom*/) { return box_t{}; } diff --git a/src/geom-pole-of-inaccessibility.cpp b/src/geom-pole-of-inaccessibility.cpp index 452b84d74..1573828f5 100644 --- a/src/geom-pole-of-inaccessibility.cpp +++ b/src/geom-pole-of-inaccessibility.cpp @@ -90,9 +90,7 @@ bool point_to_ring_distance_squared(point_t point, ring_t const &ring, double const d = point_to_segment_distance_squared(point, a, b, stretch); - if (d < *min_dist_squared) { - *min_dist_squared = d; - } + *min_dist_squared = std::min(d, *min_dist_squared); } return inside; @@ -158,9 +156,7 @@ point_t pole_of_inaccessibility(const polygon_t &polygon, double precision, double const min_precision = std::max(envelope.width(), envelope.height()) / 1000.0; - if (min_precision > precision) { - precision = min_precision; - } + precision = std::max(min_precision, precision); box_t const stretched_envelope{envelope.min_x(), envelope.min_y() * stretch, envelope.max_x(),