Various cleanups and fixes based on clang-tidy reports

joto · joto · commit afe33df8d7cc · 2020-08-11T09:09:21.000+02:00
Fiddle with the clang-tidy config and add fixes.
diff --git a/.clang-tidy b/.clang-tidy
@@ -1,5 +1,5 @@
 ---
-Checks: '*,-android-cloexec-*,-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-no-array-decay,-hicpp-vararg,-llvm-include-order,-modernize-make-unique,-readability-implicit-bool-conversion,-readability-named-parameter'
+Checks: '*,-android-cloexec-*,-cppcoreguidelines-avoid-magic-numbers,-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-no-array-decay,-hicpp-vararg,-llvm-include-order,-llvm-header-guard,-modernize-make-unique,-modernize-use-trailing-return-type,-readability-implicit-bool-conversion,-readability-named-parameter,-readability-magic-numbers'
 #
 #  cppcoreguidelines-pro-type-cstyle-cast
 #  google-build-using-namespace
@@ -11,6 +11,12 @@ Checks: '*,-android-cloexec-*,-cppcoreguidelines-owning-memory,-cppcoreguideline
 #  android-cloexec-*
 #    O_CLOEXEC isn't available on Windows
 #
+#  cppcoreguidelines-avoid-magic-numbers
+#  readability-magic-numbers
+#    We have a lot of these and should probably at least fix some. But remove
+#    it for the time being because with it we can't see the forest for the
+#    trees. (TODO)
+#
 #  cppcoreguidelines-owning-memory
 #  cppcoreguidelines-pro-bounds-array-to-pointer-decay
 #  cppcoreguidelines-pro-bounds-pointer-arithmetic
@@ -26,11 +32,18 @@ Checks: '*,-android-cloexec-*,-cppcoreguidelines-owning-memory,-cppcoreguideline
 #  google-readability-todo
 #    We are not that organized
 #
+#  llvm-header-guard
+#    Not our style
+#
 #  modernize-make-unique
 #    Not available in C++11
 #
+#  modernize-use-trailing-return-type
+#    We are not that modern...
+#
 #  readability-implicit-bool-conversion
 #    Readability is a matter of opinion here
 #
 #WarningsAsErrors: '*'
+HeaderFilterRegex: '\/src\/'
 ...
diff --git a/src/flex-table.hpp b/src/flex-table.hpp
@@ -31,7 +31,8 @@ class flex_table_t
         permanent
     };
 
-    flex_table_t(std::string const &name, int srid) : m_name(name), m_srid(srid)
+    flex_table_t(std::string name, int srid)
+    : m_name(std::move(name)), m_srid(srid)
     {}
 
     std::string const &name() const noexcept { return m_name; }
diff --git a/src/gazetteer-style.hpp b/src/gazetteer-style.hpp
@@ -73,7 +73,8 @@ class db_deleter_place_t
 class gazetteer_copy_mgr_t : public db_copy_mgr_t<db_deleter_place_t>
 {
 public:
-    gazetteer_copy_mgr_t(std::shared_ptr<db_copy_thread_t> const &processor)
+    explicit gazetteer_copy_mgr_t(
+        std::shared_ptr<db_copy_thread_t> const &processor)
     : db_copy_mgr_t<db_deleter_place_t>(processor),
       m_table(std::make_shared<db_target_descr_t>("place", "place_id"))
     {}
@@ -167,7 +168,7 @@ class gazetteer_style_t
     /// addresstags to include
     std::vector<ptag_t> m_address;
     /// value of operator tag
-    char const *m_operator;
+    char const *m_operator = nullptr;
     /// admin level
     int m_admin_level;
 
diff --git a/src/middle-ram.cpp b/src/middle-ram.cpp
@@ -118,8 +118,7 @@ void middle_ram_t::stop(thread_pool_t &)
 }
 
 middle_ram_t::middle_ram_t(options_t const *options)
-: m_ways(), m_rels(),
-  m_cache(new node_ram_cache{options->alloc_chunkwise, options->cache}),
+: m_cache(new node_ram_cache{options->alloc_chunkwise, options->cache}),
   m_extra_attributes(options->extra_attributes)
 {}
 
diff --git a/src/middle-ram.hpp b/src/middle-ram.hpp
@@ -75,7 +75,7 @@ class elem_cache_t
         assert(block < m_blocks.size());
 
         if (!m_blocks[block]) {
-            return 0;
+            return nullptr;
         }
 
         return m_blocks[block]->get(id2offset(id));
@@ -91,8 +91,8 @@ class elem_cache_t
 
 struct middle_ram_t : public middle_t, public middle_query_t
 {
-    middle_ram_t(options_t const *options);
-    virtual ~middle_ram_t() noexcept = default;
+    explicit middle_ram_t(options_t const *options);
+    ~middle_ram_t() noexcept override = default;
 
     void start() override {}
     void stop(thread_pool_t &pool) override;
diff --git a/src/middle.hpp b/src/middle.hpp
@@ -87,8 +87,8 @@ struct middle_t
 
     virtual void start() = 0;
     virtual void stop(thread_pool_t &pool) = 0;
-    virtual void analyze(void) = 0;
-    virtual void commit(void) = 0;
+    virtual void analyze() = 0;
+    virtual void commit() = 0;
 
     /**
      * Add a node to data storage. The node must not already be in the
@@ -135,7 +135,7 @@ inline middle_t::~middle_t() = default;
  */
 struct slim_middle_t : public middle_t
 {
-    virtual ~slim_middle_t() = 0;
+    ~slim_middle_t() override = 0;
 
     /**
      * Delete a node from data storage. Either because you want it removed
diff --git a/src/options.cpp b/src/options.cpp
@@ -297,7 +297,11 @@ options_t::options_t()
 
 static osmium::Box parse_bbox(char const *bbox)
 {
-    double minx, maxx, miny, maxy;
+    double minx;
+    double maxx;
+    double miny;
+    double maxy;
+
     int const n = sscanf(bbox, "%lf,%lf,%lf,%lf", &minx, &miny, &maxx, &maxy);
     if (n != 4) {
         throw std::runtime_error{"Bounding box must be specified like: "
diff --git a/src/output-flex.cpp b/src/output-flex.cpp
@@ -799,7 +799,7 @@ int output_flex_t::table_add_row()
 
     auto &table_connection =
         m_table_connections.at(table_idx_from_param(lua_state()));
-    auto &table = table_connection.table();
+    auto const &table = table_connection.table();
     luaL_checktype(lua_state(), 2, LUA_TTABLE);
     lua_remove(lua_state(), 1);
 
diff --git a/src/output-flex.hpp b/src/output-flex.hpp
@@ -76,7 +76,7 @@ class prepared_lua_function_t
     calling_context context() const noexcept { return m_calling_context; }
 
     /// Is this function defined in the users Lua code?
-    operator bool() const noexcept { return m_index != 0; }
+    explicit operator bool() const noexcept { return m_index != 0; }
 
 private:
     char const *m_name = nullptr;
@@ -107,7 +107,7 @@ class output_flex_t : public output_t
     output_flex_t(output_flex_t &&) = delete;
     output_flex_t &operator=(output_flex_t &&) = delete;
 
-    virtual ~output_flex_t() noexcept = default;
+    ~output_flex_t() noexcept override = default;
 
     std::shared_ptr<output_t>
     clone(std::shared_ptr<middle_query_t> const &mid,
diff --git a/src/output-null.hpp b/src/output-null.hpp
@@ -11,7 +11,8 @@ class output_null_t : public output_t
 public:
     output_null_t(std::shared_ptr<middle_query_t> const &mid,
                   options_t const &options);
-    virtual ~output_null_t();
+
+    ~output_null_t() override;
 
     std::shared_ptr<output_t>
     clone(std::shared_ptr<middle_query_t> const &mid,
diff --git a/src/output-pgsql.hpp b/src/output-pgsql.hpp
@@ -36,7 +36,8 @@ class output_pgsql_t : public output_t
     output_pgsql_t(std::shared_ptr<middle_query_t> const &mid,
                    options_t const &options,
                    std::shared_ptr<db_copy_thread_t> const &copy_thread);
-    virtual ~output_pgsql_t();
+
+    ~output_pgsql_t() override;
 
     std::shared_ptr<output_t>
     clone(std::shared_ptr<middle_query_t> const &mid,
diff --git a/src/progress-display.hpp b/src/progress-display.hpp
@@ -26,7 +26,7 @@ class progress_display_t
         std::time_t start = 0;
         int m_frac;
 
-        Counter(int frac) noexcept : m_frac(frac) {}
+        explicit Counter(int frac) noexcept : m_frac(frac) {}
 
         osmid_t count_k() const noexcept { return count / 1000; }
 
diff --git a/src/table.hpp b/src/table.hpp
@@ -50,7 +50,7 @@ class table_t
         int get_count() const noexcept { return m_count; }
 
     private:
-        wkb_reader(pg_result_t &&result)
+        explicit wkb_reader(pg_result_t &&result)
         : m_result(std::move(result)), m_count(m_result.num_tuples()),
           m_current(0)
         {}
diff --git a/src/tagtransform-c.cpp b/src/tagtransform-c.cpp
@@ -32,8 +32,6 @@ static const struct
               {37, "primary", 1},       {38, "trunk", 1},
               {39, "motorway", 1}};
 
-static const unsigned int nLayers = (sizeof(layers) / sizeof(*layers));
-
 void add_z_order(taglist_t &tags, int *roads)
 {
     std::string const *const layer = tags.get("layer");
@@ -50,10 +48,10 @@ void add_z_order(taglist_t &tags, int *roads)
     *roads = 0;
 
     if (highway) {
-        for (unsigned i = 0; i < nLayers; ++i) {
-            if (!std::strcmp(layers[i].highway, highway->c_str())) {
-                z_order += layers[i].offset;
-                *roads = layers[i].roads;
+        for (const auto &layer : layers) {
+            if (*highway == layer.highway) {
+                z_order += layer.offset;
+                *roads = layer.roads;
                 break;
             }
         }
@@ -220,7 +218,9 @@ bool c_tagtransform_t::filter_rel_member_tags(
 {
     //if it has a relation figure out what kind it is
     std::string const *type = rel_tags.get("type");
-    bool is_route = false, is_boundary = false, is_multipolygon = false;
+    bool is_route = false;
+    bool is_boundary = false;
+    bool is_multipolygon = false;
     if (type) {
         //what kind of relation is it
         if (*type == "route") {
diff --git a/src/tagtransform-lua.hpp b/src/tagtransform-lua.hpp
@@ -15,7 +15,7 @@ class lua_tagtransform_t : public tagtransform_t
     lua_tagtransform_t(lua_tagtransform_t const &other) = default;
 
 public:
-    lua_tagtransform_t(options_t const *options);
+    explicit lua_tagtransform_t(options_t const *options);
     ~lua_tagtransform_t();
 
     std::unique_ptr<tagtransform_t> clone() const override;
diff --git a/tests/test-db-copy-mgr.cpp b/tests/test-db-copy-mgr.cpp
@@ -26,7 +26,7 @@ static std::shared_ptr<db_target_descr_t> setup_table(std::string const &cols)
 }
 
 template <typename... ARGS>
-void add_row(copy_mgr_t &mgr, std::shared_ptr<db_target_descr_t> t,
+void add_row(copy_mgr_t &mgr, std::shared_ptr<db_target_descr_t> const &t,
              ARGS &&... args)
 {
     mgr.new_line(t);
@@ -37,8 +37,8 @@ void add_row(copy_mgr_t &mgr, std::shared_ptr<db_target_descr_t> t,
 }
 
 template <typename T>
-void add_array(copy_mgr_t &mgr, std::shared_ptr<db_target_descr_t> t, int id,
-               std::vector<T> const &values)
+void add_array(copy_mgr_t &mgr, std::shared_ptr<db_target_descr_t> const &t,
+               int id, std::vector<T> const &values)
 {
     mgr.new_line(t);
     mgr.add_column(id);
@@ -53,7 +53,7 @@ void add_array(copy_mgr_t &mgr, std::shared_ptr<db_target_descr_t> t, int id,
 }
 
 static void
-add_hash(copy_mgr_t &mgr, std::shared_ptr<db_target_descr_t> t, int id,
+add_hash(copy_mgr_t &mgr, std::shared_ptr<db_target_descr_t> const &t, int id,
          std::vector<std::pair<std::string, std::string>> const &values)
 {
     mgr.new_line(t);
@@ -74,7 +74,7 @@ static void check_row(std::vector<std::string> const &row)
     auto conn = db.connect();
     auto res = conn.require_row("SELECT * FROM test_copy_mgr");
 
-    for (unsigned i = 0; i < row.size(); ++i) {
+    for (std::size_t i = 0; i < row.size(); ++i) {
         CHECK(res.get_value(0, (int)i) == row[i]);
     }
 }
diff --git a/tests/test-middle.cpp b/tests/test-middle.cpp
@@ -507,7 +507,7 @@ static void check_way(std::shared_ptr<middle_pgsql_t> const &mid,
  */
 static void check_way_nodes(std::shared_ptr<middle_pgsql_t> const &mid,
                             osmid_t way_id,
-                            std::vector<osmium::Node const *> nodes)
+                            std::vector<osmium::Node const *> const &nodes)
 {
     auto const mid_q = mid->get_query_instance();
 

Original file line number	Diff line number	Diff line change
`@@ -118,8 +118,7 @@ void middle_ram_t::stop(thread_pool_t &)`
`118`	`118`	`}`
`119`	`119`
`120`	`120`	`middle_ram_t::middle_ram_t(options_t const *options)`
`121`		`-: m_ways(), m_rels(),`
`122`		`- m_cache(new node_ram_cache{options->alloc_chunkwise, options->cache}),`
	`121`	`+: m_cache(new node_ram_cache{options->alloc_chunkwise, options->cache}),`
`123`	`122`	`m_extra_attributes(options->extra_attributes)`
`124`	`123`	`{}`
`125`	`124`