Enable more clang-tidy checks

.clang-tidy

@@ -9,13 +9,10 @@ Checks: >
     -bugprone-implicit-widening-of-multiplication-result,
     -bugprone-integer-division,
     -bugprone-reserved-identifier,
-    -bugprone-macro-parentheses,
     -bugprone-unhandled-self-assignment,
-    -bugprone-suspicious-missing-comma,
     -bugprone-forward-declaration-namespace,
     -bugprone-sizeof-expression,
     -clang-analyzer-*,
-    -clang-diagnostic-unused-local-typedef,
     -clang-diagnostic-deprecated-declarations,
     google-*,
     -google-build-explicit-make-pair,
@@ -29,7 +26,6 @@ Checks: >
     -google-readability-todo,
     -google-runtime-int,
     -google-build-namespaces,
-    -google-global-names-in-headers,
     -google-runtime-references,
     -google-readability-function-size,
     llvm-*,
@@ -42,14 +38,17 @@ Checks: >
     misc-*,
     -misc-argument-comment,
     -misc-non-private-member-variables-in-classes,
-    -misc-unused-using-decls,
     -misc-unconventional-assign-operator,
     -misc-redundant-expression,
     -misc-no-recursion,
     -misc-misplaced-const,
     -misc-definitions-in-headers,
-    -misc-unused-alias-decls,
     -misc-unused-parameters,
+    performance-*,
+    -performance-noexcept-move-constructor,
+    -performance-move-const-arg,
+    -performance-no-int-to-ptr,
+    -performance-unnecessary-value-param,
     readability-*,
     -readability-avoid-const-params-in-decls,
     -readability-braces-around-statements,

CHANGELOG.md

@@ -3,6 +3,7 @@
     - API:
       - FIXED: Fix inefficient osrm-routed connection handling [#6113](https://github.com/Project-OSRM/osrm-backend/pull/6113)
     - Build:
+      - CHANGED: Enable more clang-tidy checks. [#6262](https://github.com/Project-OSRM/osrm-backend/pull/6270)
       - CHANGED: Configure clang-tidy job on CI. [#6261](https://github.com/Project-OSRM/osrm-backend/pull/6261)
       - CHANGED: Use Github Actions for building container images [#6138](https://github.com/Project-OSRM/osrm-backend/pull/6138)
       - CHANGED: Upgrade Boost dependency to 1.70 [#6113](https://github.com/Project-OSRM/osrm-backend/pull/6113)

include/contractor/contract_excludable_graph.hpp

@@ -18,7 +18,7 @@ inline auto contractFullGraph(ContractorGraph contractor_graph,
                               std::vector<EdgeWeight> node_weights)
 {
     auto num_nodes = contractor_graph.GetNumberOfNodes();
-    contractGraph(contractor_graph, node_weights);
+    contractGraph(contractor_graph, std::move(node_weights));
 
     auto edges = toEdges<QueryEdge>(std::move(contractor_graph));
     std::vector<bool> edge_filter(edges.size(), true);

include/engine/datafacade_factory.hpp

@@ -65,7 +65,7 @@ template <template <typename A> class FacadeT, typename AlgorithmT> class DataFa
 
         for (const auto &exclude_prefix : exclude_prefixes)
         {
-            auto index_begin = exclude_prefix.find_last_of("/");
+            auto index_begin = exclude_prefix.find_last_of('/');
             BOOST_ASSERT_MSG(index_begin != std::string::npos,
                              "The exclude prefix needs to be a valid data path.");
             std::size_t index =

include/extractor/raster_source.hpp

@@ -18,7 +18,6 @@
 #include <iterator>
 #include <string>
 #include <unordered_map>
-using namespace std;
 
 namespace osrm
 {

include/extractor/restriction.hpp

@@ -186,7 +186,7 @@ struct TurnRestriction
     }
 
     // construction for WayRestrictions
-    explicit TurnRestriction(WayRestriction way_restriction, bool is_only = false)
+    explicit TurnRestriction(const WayRestriction &way_restriction, bool is_only = false)
         : node_or_way(way_restriction), is_only(is_only)
     {
     }

include/guidance/turn_bearing.hpp

@@ -9,15 +9,12 @@ namespace osrm
 {
 namespace guidance
 {
-namespace
-{
-const double bearing_scale = 360.0 / 256.0;
-}
 
 #pragma pack(push, 1)
 class TurnBearing
 {
   public:
+    static constexpr double bearing_scale = 360.0 / 256.0;
     // discretizes a bearing into distinct units of 1.4 degrees
     TurnBearing(const double value = 0) : bearing(value / bearing_scale)
     {

include/guidance/turn_lane_handler.hpp

@@ -52,15 +52,6 @@ typedef enum TurnLaneScenario
     NUM_SCENARIOS
 } TurnLaneScenario;
 
-const constexpr char *scenario_names[] = {"Simple",
-                                          "Partition Local",
-                                          "Simple Previous",
-                                          "Partition Previous",
-                                          "Sliproad",
-                                          "Merge",
-                                          "None",
-                                          "Invalid",
-                                          "Unknown"};
 } // namespace
 
 class TurnLaneHandler
@@ -150,9 +141,6 @@ class TurnLaneHandler
                          LaneDataVector &lane_data) const;
 };
 
-static_assert(sizeof(scenario_names) / sizeof(*scenario_names) == TurnLaneScenario::NUM_SCENARIOS,
-              "Number of scenarios needs to match the number of scenario names.");
-
 } // namespace lanes
 } // namespace guidance
 } // namespace osrm

include/server/server.hpp

@@ -77,7 +77,7 @@ class Server
                 boost::bind(&boost::asio::io_context::run, &io_context));
             threads.push_back(thread);
         }
-        for (auto thread : threads)
+        for (const auto &thread : threads)
         {
             thread->join();
         }

include/storage/io_config.hpp

@@ -18,8 +18,9 @@ struct IOConfig
     IOConfig(std::vector<boost::filesystem::path> required_input_files_,
              std::vector<boost::filesystem::path> optional_input_files_,
              std::vector<boost::filesystem::path> output_files_)
-        : required_input_files(required_input_files_), optional_input_files(optional_input_files_),
+        : required_input_files(std::move(required_input_files_)),
-          output_files(output_files_)
+          optional_input_files(std::move(optional_input_files_)),
+          output_files(std::move(output_files_))
     {
     }
 
@@ -47,7 +48,7 @@ struct IOConfig
 
         std::array<std::string, 6> known_extensions{
             {".osm.bz2", ".osm.pbf", ".osm.xml", ".pbf", ".osm", ".osrm"}};
-        for (auto ext : known_extensions)
+        for (const auto &ext : known_extensions)
         {
             const auto pos = path.find(ext);
             if (pos != std::string::npos)

include/storage/shared_datatype.hpp

@@ -36,7 +36,7 @@ inline std::string trimName(const std::string &name_prefix, const std::string &n
     // list directory and
     if (!name_prefix.empty() && name_prefix.back() == '/')
     {
-        auto directory_position = name.find_first_of("/", name_prefix.length());
+        auto directory_position = name.find_first_of('/', name_prefix.length());
         // this is a "file" in the directory of name_prefix
         if (directory_position == std::string::npos)
         {

include/util/exception.hpp

@@ -47,7 +47,7 @@ class exception : public std::exception
   public:
     explicit exception(const char *message) : message(message) {}
     explicit exception(std::string message) : message(std::move(message)) {}
-    explicit exception(boost::format message) : message(message.str()) {}
+    explicit exception(const boost::format &message) : message(message.str()) {}
     const char *what() const noexcept override { return message.c_str(); }
 
   private:
@@ -65,18 +65,19 @@ class exception : public std::exception
  */
 
 constexpr const std::array<const char *, 11> ErrorDescriptions = {{
-    "",                                                      // Dummy - ErrorCode values start at 2
+    "",                                               // Dummy - ErrorCode values start at 2
-    "",                                                      // Dummy - ErrorCode values start at 2
+    "",                                               // Dummy - ErrorCode values start at 2
-    "Fingerprint did not match the expected value",          // InvalidFingerprint
+    "Fingerprint did not match the expected value",   // InvalidFingerprint
-    "File is incompatible with this version of OSRM",        // IncompatibleFileVersion
+    "File is incompatible with this version of OSRM", // IncompatibleFileVersion
-    "Problem opening file",                                  // FileOpenError
+    "Problem opening file",                           // FileOpenError
-    "Problem reading from file",                             // FileReadError
+    "Problem reading from file",                      // FileReadError
-    "Problem writing to file",                               // FileWriteError
+    "Problem writing to file",                        // FileWriteError
-    "I/O error occurred",                                    // FileIOError
+    "I/O error occurred",                             // FileIOError
-    "Unexpected end of file",                                // UnexpectedEndOfFile
+    "Unexpected end of file",                         // UnexpectedEndOfFile
-    "The dataset you are trying to load is not "             // IncompatibleDataset
+    // NOLINTNEXTLINE(bugprone-suspicious-missing-comma)
-    "compatible with the routing algorithm you want to use." // ...continued...
+    "The dataset you are trying to load is not "              // IncompatibleDataset
-    "Incompatible algorithm"                                 // IncompatibleAlgorithm
+    "compatible with the routing algorithm you want to use.", // ...continued...
+    "Incompatible algorithm"                                  // IncompatibleAlgorithm
 }};
 
 #ifndef NDEBUG

include/util/exception_utils.hpp

@@ -10,6 +10,6 @@
 #define OSRM_SOURCE_FILE_ PROJECT_RELATIVE_PATH_(__FILE__)
 
 // This is the macro to use
-#define SOURCE_REF OSRM_SOURCE_FILE_ + ":" + std::to_string(__LINE__)
+#define SOURCE_REF (OSRM_SOURCE_FILE_ + ":" + std::to_string(__LINE__))
 
 #endif // SOURCE_MACROS_HPP

include/util/guidance/name_announcements.hpp

@@ -71,12 +71,12 @@ template <typename StringView> inline auto decompose(const StringView &lhs, cons
         // we compare suffixes based on this value, it might break UTF chars, but as long as we are
         // consistent in handling, we do not create bad results
         std::string str = boost::to_lower_copy(view.to_string());
-        auto front = str.find_first_not_of(" ");
+        auto front = str.find_first_not_of(' ');
 
         if (front == std::string::npos)
             return str;
 
-        auto back = str.find_last_not_of(" ");
+        auto back = str.find_last_not_of(' ');
         return str.substr(front, back - front + 1);
     };
 

include/util/range_table.hpp

@@ -83,19 +83,25 @@ template <unsigned BLOCK_SIZE, storage::Ownership Ownership> class RangeTable
         unsigned block_idx = 0;
         unsigned block_counter = 0;
         BlockT block;
+#ifndef BOOST_ASSERT_IS_VOID
         unsigned block_sum = 0;
+#endif
         for (const unsigned l : lengths)
         {
             // first entry of a block: encode absolute offset
             if (block_idx == 0)
             {
                 block_offsets.push_back(lengths_prefix_sum);
+#ifndef BOOST_ASSERT_IS_VOID
                 block_sum = 0;
+#endif
             }
             else
             {
                 block[block_idx - 1] = last_length;
+#ifndef BOOST_ASSERT_IS_VOID
                 block_sum += last_length;
+#endif
             }
 
             BOOST_ASSERT((block_idx == 0 && block_offsets[block_counter] == lengths_prefix_sum) ||

src/customize/customizer.cpp

@@ -108,8 +108,9 @@ std::vector<CellMetric> customizeFilteredMetrics(const partitioner::MultiLevelEd
                                                  const std::vector<std::vector<bool>> &node_filters)
 {
     std::vector<CellMetric> metrics;
+    metrics.reserve(node_filters.size());
 
-    for (auto filter : node_filters)
+    for (const auto &filter : node_filters)
     {
         auto metric = storage.MakeMetric();
         customizer.Customize(graph, storage, filter, metric);

src/engine/api/json_factory.cpp

@@ -19,7 +19,6 @@
 #include <utility>
 #include <vector>
 
-namespace TurnType = osrm::guidance::TurnType;
 using TurnInstruction = osrm::guidance::TurnInstruction;
 
 namespace osrm
@@ -242,7 +241,7 @@ util::json::Object makeWaypoint(const util::Coordinate &location,
                                 std::string name,
                                 const Hint &hint)
 {
-    auto waypoint = makeWaypoint(location, distance, name);
+    auto waypoint = makeWaypoint(location, distance, std::move(name));
     waypoint.values["hint"] = hint.ToBase64();
     return waypoint;
 }

src/extractor/intersection/intersection.cpp

@@ -5,8 +5,6 @@
 
 #include <boost/range/adaptors.hpp>
 
-using osrm::util::angularDeviation;
-
 namespace osrm
 {
 namespace extractor

src/guidance/driveway_handler.cpp

@@ -2,9 +2,6 @@
 
 #include "util/assert.hpp"
 
-using osrm::guidance::getTurnDirection;
-using osrm::util::angularDeviation;
-
 namespace osrm
 {
 namespace guidance

src/guidance/turn_handler.cpp

@@ -684,7 +684,7 @@ bool TurnHandler::isCompatibleByRoadClass(const Intersection &intersection, cons
 boost::optional<TurnHandler::Fork> TurnHandler::findFork(const EdgeID via_edge,
                                                          Intersection &intersection) const
 {
-    const auto fork = findForkCandidatesByGeometry(intersection);
+    auto fork = findForkCandidatesByGeometry(intersection);
     if (fork)
     {
         // makes sure that the fork is isolated from other neighbouring streets on the left and

src/partitioner/dinic_max_flow.cpp

@@ -88,7 +88,7 @@ DinicMaxFlow::MinCut DinicMaxFlow::operator()(const BisectionGraphView &view,
             // heuristic)
             for (auto s : source_nodes)
                 levels[s] = 0;
-            const auto cut = MakeCut(view, levels, flow_value);
+            auto cut = MakeCut(view, levels, flow_value);
             return cut;
         }
     } while (true);

src/updater/updater.cpp

@@ -504,7 +504,7 @@ bool IsRestrictionValid(const Timezoner &tz_handler, const extractor::Conditiona
 std::vector<std::uint64_t>
 updateConditionalTurns(std::vector<TurnPenalty> &turn_weight_penalties,
                        const std::vector<extractor::ConditionalTurnPenalty> &conditional_turns,
-                       Timezoner time_zone_handler)
+                       const Timezoner &time_zone_handler)
 {
     std::vector<std::uint64_t> updated_turns;
     if (conditional_turns.size() == 0)

src/util/conditional_restrictions.cpp

@@ -31,7 +31,6 @@ namespace detail
 
 namespace
 {
-namespace ph = boost::phoenix;
 namespace qi = boost::spirit::qi;
 } // namespace