From e792264b4cc0f493a1961338236533bbfee99014 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Sat, 5 Jul 2025 17:05:19 +0200 Subject: [PATCH 1/6] Use smaller enums Found by clang-tidy In src/taginfo-impl.hpp we can't use a smaller type, because we use bool operators on those values which promotes them to ints which leads to other problems. --- src/output-pgsql.hpp | 3 ++- src/taginfo-impl.hpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/output-pgsql.hpp b/src/output-pgsql.hpp index 82aab120e..39cde4c5d 100644 --- a/src/output-pgsql.hpp +++ b/src/output-pgsql.hpp @@ -23,12 +23,13 @@ #include "tagtransform.hpp" #include +#include #include class output_pgsql_t : public output_t { public: - enum table_id + enum table_id : std::uint8_t { t_point = 0, t_line, diff --git a/src/taginfo-impl.hpp b/src/taginfo-impl.hpp index e93296549..8534e9086 100644 --- a/src/taginfo-impl.hpp +++ b/src/taginfo-impl.hpp @@ -19,7 +19,7 @@ #include #include -enum column_flags : unsigned int +enum column_flags : unsigned int // NOLINT(performance-enum-size) { FLAG_POLYGON = 1, /* For polygon table */ FLAG_LINEAR = 2, /* For lines table */ From 6518353ade764bef09c9c1bc179a13f6f38f6331 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Sat, 5 Jul 2025 17:05:19 +0200 Subject: [PATCH 2/6] Disable clang-tidy warnings in legacy code This code will be removed with the pgsql output at some point and we don't want to fiddle with it any more than necessary. --- src/table.cpp | 4 ++++ src/taginfo.cpp | 3 +++ 2 files changed, 7 insertions(+) diff --git a/src/table.cpp b/src/table.cpp index e6a40c010..4d4dc511f 100644 --- a/src/table.cpp +++ b/src/table.cpp @@ -355,6 +355,9 @@ void table_t::task_wait() util::human_readable_duration(run_time)); } +// NOLINTBEGIN(google-runtime-int,cert-err34-c) +// This is legacy code which will be removed anyway. + /* Escape data appropriate to the type */ void table_t::escape_type(std::string const &value, ColumnType flags) { @@ -423,6 +426,7 @@ void table_t::escape_type(std::string const &value, ColumnType flags) break; } } +// NOLINTEND(google-runtime-int,cert-err34-c) pg_result_t table_t::get_wkb(osmid_t id) { diff --git a/src/taginfo.cpp b/src/taginfo.cpp index 8cfa85892..208f8b019 100644 --- a/src/taginfo.cpp +++ b/src/taginfo.cpp @@ -87,6 +87,8 @@ unsigned get_tag_type(std::string const &tag) } // anonymous namespace +// NOLINTBEGIN(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) +// This is legacy code which will be removed anyway. bool read_style_file(std::string const &filename, export_list *exlist) { bool enable_way_area = true; @@ -195,3 +197,4 @@ bool read_style_file(std::string const &filename, export_list *exlist) return enable_way_area; } +// NOLINTEND(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) From 7ab133eaf46f22c78a964f5b55bc09aa17528b1e Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Sat, 5 Jul 2025 17:05:19 +0200 Subject: [PATCH 3/6] Remove implied inlines --- src/geom-boost-adaptor.hpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/geom-boost-adaptor.hpp b/src/geom-boost-adaptor.hpp index 4ca489ce5..7eac5fbf9 100644 --- a/src/geom-boost-adaptor.hpp +++ b/src/geom-boost-adaptor.hpp @@ -85,8 +85,8 @@ BOOST_GEOMETRY_DETAIL_SPECIALIZE_BOX_TRAITS(::geom::box_t, ::geom::point_t) template <> struct indexed_access<::geom::box_t, min_corner, 0> { - static inline double get(::geom::box_t const &b) { return b.min_x(); } - static inline void set(::geom::box_t &b, double value) + static double get(::geom::box_t const &b) { return b.min_x(); } + static void set(::geom::box_t &b, double value) { b.set_min_x(value); } @@ -95,8 +95,8 @@ struct indexed_access<::geom::box_t, min_corner, 0> template <> struct indexed_access<::geom::box_t, min_corner, 1> { - static inline double get(::geom::box_t const &b) { return b.min_y(); } - static inline void set(::geom::box_t &b, double value) + static double get(::geom::box_t const &b) { return b.min_y(); } + static void set(::geom::box_t &b, double value) { b.set_min_y(value); } @@ -105,8 +105,8 @@ struct indexed_access<::geom::box_t, min_corner, 1> template <> struct indexed_access<::geom::box_t, max_corner, 0> { - static inline double get(::geom::box_t const &b) { return b.max_x(); } - static inline void set(::geom::box_t &b, double value) + static double get(::geom::box_t const &b) { return b.max_x(); } + static void set(::geom::box_t &b, double value) { b.set_max_x(value); } @@ -115,8 +115,8 @@ struct indexed_access<::geom::box_t, max_corner, 0> template <> struct indexed_access<::geom::box_t, max_corner, 1> { - static inline double get(::geom::box_t const &b) { return b.max_y(); } - static inline void set(::geom::box_t &b, double value) + static double get(::geom::box_t const &b) { return b.max_y(); } + static void set(::geom::box_t &b, double value) { b.set_max_y(value); } From 34b01a20f520b0adb5180643568d2e3f58e33f9b Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Sat, 5 Jul 2025 17:05:19 +0200 Subject: [PATCH 4/6] Don't use std::forward in a loop This could be a use after move. --- src/geom-functions.hpp | 4 ++-- src/lua-utils.hpp | 8 ++++---- src/tile.hpp | 9 ++++----- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/geom-functions.hpp b/src/geom-functions.hpp index 7c22d6298..56ace587d 100644 --- a/src/geom-functions.hpp +++ b/src/geom-functions.hpp @@ -43,13 +43,13 @@ point_t interpolate(point_t p1, point_t p2, double frac) noexcept; * \pre \code !list.empty() \endcode */ template -void for_each_segment(point_list_t const &list, FUNC &&func) +void for_each_segment(point_list_t const &list, FUNC const &func) { assert(!list.empty()); auto it = list.cbegin(); auto prev = it; for (++it; it != list.cend(); ++it) { - std::forward(func)(*prev, *it); + func(*prev, *it); prev = it; } } diff --git a/src/lua-utils.hpp b/src/lua-utils.hpp index 3ca4ec91b..920d28495 100644 --- a/src/lua-utils.hpp +++ b/src/lua-utils.hpp @@ -40,14 +40,14 @@ void luaX_add_table_func(lua_State *lua_state, char const *key, template void luaX_add_table_array(lua_State *lua_state, char const *key, - COLLECTION const &collection, FUNC &&func) + COLLECTION const &collection, FUNC const &func) { lua_pushstring(lua_state, key); lua_createtable(lua_state, (int)collection.size(), 0); int n = 0; for (auto const &member : collection) { lua_pushinteger(lua_state, ++n); - std::forward(func)(member); + func(member); lua_rawset(lua_state, -3); } lua_rawset(lua_state, -3); @@ -99,7 +99,7 @@ bool luaX_is_array(lua_State *lua_state); * \post Stack is unchanged. */ template -void luaX_for_each(lua_State *lua_state, FUNC &&func) +void luaX_for_each(lua_State *lua_state, FUNC const &func) { assert(lua_istable(lua_state, -1)); lua_pushnil(lua_state); @@ -107,7 +107,7 @@ void luaX_for_each(lua_State *lua_state, FUNC &&func) #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); } diff --git a/src/tile.hpp b/src/tile.hpp index c95a376e9..c76f97d2e 100644 --- a/src/tile.hpp +++ b/src/tile.hpp @@ -281,14 +281,14 @@ class tile_t */ template std::size_t for_each_tile(quadkey_list_t const &tiles_at_maxzoom, - uint32_t minzoom, uint32_t maxzoom, OUTPUT &&output) + uint32_t minzoom, uint32_t maxzoom, + OUTPUT const &output) { assert(minzoom <= maxzoom); if (minzoom == maxzoom) { for (auto const quadkey : tiles_at_maxzoom) { - std::forward(output)( - tile_t::from_quadkey(quadkey, maxzoom)); + output(tile_t::from_quadkey(quadkey, maxzoom)); } return tiles_at_maxzoom.size(); } @@ -309,8 +309,7 @@ std::size_t for_each_tile(quadkey_list_t const &tiles_at_maxzoom, * the first sibling. */ if (qt_current != last_quadkey.down(dz)) { - std::forward(output)( - tile_t::from_quadkey(qt_current, maxzoom - dz)); + output(tile_t::from_quadkey(qt_current, maxzoom - dz)); ++count; } } From f971b7169ff5a46b8db2981f03a0541afd5cdc21 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Sat, 5 Jul 2025 17:05:19 +0200 Subject: [PATCH 5/6] Use universal ref and std::forward --- src/db-copy-mgr.hpp | 4 ++-- src/pgsql.hpp | 11 ++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/db-copy-mgr.hpp b/src/db-copy-mgr.hpp index 7585e87de..971f17d82 100644 --- a/src/db-copy-mgr.hpp +++ b/src/db-copy-mgr.hpp @@ -93,9 +93,9 @@ class db_copy_mgr_t * a column delimiter. */ template - void add_column(T value) + void add_column(T &&value) { - add_value(value); + add_value(std::forward(value)); m_current.buffer += '\t'; } diff --git a/src/pgsql.hpp b/src/pgsql.hpp index be2d6f9ad..f6981b8a4 100644 --- a/src/pgsql.hpp +++ b/src/pgsql.hpp @@ -180,7 +180,7 @@ class pg_conn_t * status code PGRES_COMMAND_OK or PGRES_TUPLES_OK). */ template - pg_result_t exec(fmt::format_string sql, TArgs... params) const + pg_result_t exec(fmt::format_string sql, TArgs &&...params) const { return exec(fmt::format(sql, std::forward(params)...)); } @@ -196,7 +196,7 @@ class pg_conn_t */ template void prepare(std::string const &stmt, fmt::format_string sql, - TArgs... params) const + TArgs &&...params) const { std::string const query = fmt::format(sql, std::forward(params)...); @@ -213,7 +213,7 @@ class pg_conn_t * \throws exception if the command failed. */ template - pg_result_t exec_prepared(char const *stmt, TArgs... params) const + pg_result_t exec_prepared(char const *stmt, TArgs &&...params) const { return exec_prepared_with_result_format(stmt, false, std::forward(params)...); @@ -229,7 +229,8 @@ class pg_conn_t * \throws exception if the command failed. */ template - pg_result_t exec_prepared_as_binary(char const *stmt, TArgs... params) const + pg_result_t exec_prepared_as_binary(char const *stmt, + TArgs &&...params) const { return exec_prepared_with_result_format(stmt, true, std::forward(params)...); @@ -312,7 +313,7 @@ class pg_conn_t template pg_result_t exec_prepared_with_result_format(char const *stmt, bool result_as_binary, - TArgs... params) const + TArgs &&...params) const { // We have to convert all non-string parameters into strings and // store them somewhere. We use the exec_params vector for this. From b3b1055f2b6697cd09cb1ab6b6c4d610d90c0272 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Sat, 5 Jul 2025 17:05:19 +0200 Subject: [PATCH 6/6] Disable unhelpful clang-tidy warning for templated code --- src/params.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/params.hpp b/src/params.hpp index c5bff5975..a5935393e 100644 --- a/src/params.hpp +++ b/src/params.hpp @@ -54,6 +54,7 @@ class params_t m_map.insert_or_assign(std::forward(key), value); } + // NOLINTBEGIN(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) template >, bool> = true> @@ -71,6 +72,7 @@ class params_t { m_map.insert_or_assign(std::forward(key), std::string(value)); } + // NOLINTEND(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) template void remove(K &&key)