Enable more clang-tidy checks. (#6270)

* Enable more clang-tidy checks

.clang-tidy

@@ -9,14 +9,12 @@ 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,
+    -clang-diagnostic-constant-conversion,
     google-*,
     -google-build-explicit-make-pair,
     -google-build-using-namespace,
@@ -29,7 +27,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 +39,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
 

unit_tests/common/range_tools.hpp

@@ -3,7 +3,7 @@
 
 #include <boost/test/unit_test.hpp>
 
-#define REQUIRE_SIZE_RANGE(range, ref) BOOST_REQUIRE_EQUAL(range.size(), ref)
+#define REQUIRE_SIZE_RANGE(range, ref) BOOST_REQUIRE_EQUAL((range).size(), ref)
 #define CHECK_EQUAL_RANGE(range, ...)                                                              \
     do                                                                                             \
     {                                                                                              \

unit_tests/engine/geometry_string.cpp

@@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(incorrect_polylines)
     };
     util::Coordinate coord{util::FloatLongitude{0}, util::FloatLatitude{0}};
 
-    for (auto polyline : polylines)
+    for (const auto &polyline : polylines)
     {
         const auto result = decodePolyline(polyline);
         BOOST_CHECK(result.size() == 1);

unit_tests/library/match.cpp

@@ -12,10 +12,12 @@
 #include "osrm/status.hpp"
 
 osrm::Status run_match_json(const osrm::OSRM &osrm,
-                            const MatchParameters &params,
+                            const osrm::MatchParameters &params,
-                            json::Object &json_result,
+                            osrm::json::Object &json_result,
                             bool use_json_only_api)
 {
+    using namespace osrm;
+
     if (use_json_only_api)
     {
         return osrm.Match(params, json_result);

unit_tests/library/tile.cpp

@@ -34,7 +34,7 @@ osrm::Status run_tile(const osrm::OSRM &osrm,
 }
 
 #define CHECK_EQUAL_RANGE(R1, R2)                                                                  \
-    BOOST_CHECK_EQUAL_COLLECTIONS(R1.begin(), R1.end(), R2.begin(), R2.end());
+    BOOST_CHECK_EQUAL_COLLECTIONS((R1).begin(), (R1).end(), (R2).begin(), (R2).end());
 
 BOOST_AUTO_TEST_SUITE(tile)
 

unit_tests/library/waypoint_check.hpp

@@ -6,10 +6,10 @@
 #include "osrm/json_container.hpp"
 #include "util/exception.hpp"
 
-using namespace osrm;
+inline bool waypoint_check(osrm::json::Value waypoint)
-
-inline bool waypoint_check(json::Value waypoint)
 {
+    using namespace osrm;
+
     if (!waypoint.is<mapbox::util::recursive_wrapper<util::json::Object>>())
     {
         throw util::exception("Must pass in a waypoint object");
@@ -24,6 +24,8 @@ inline bool waypoint_check(json::Value waypoint)
 
 inline bool waypoint_check(const osrm::engine::api::fbresult::Waypoint *const waypoint)
 {
+    using namespace osrm;
+
     util::FloatLongitude lon{waypoint->location()->longitude()};
     util::FloatLatitude lat{waypoint->location()->latitude()};
     util::Coordinate location_coordinate(lon, lat);

unit_tests/partitioner/bisection_to_partition.cpp

@@ -3,7 +3,7 @@
 
 #include "partitioner/bisection_to_partition.hpp"
 
-#define CHECK_SIZE_RANGE(range, ref) BOOST_CHECK_EQUAL(range.end() - range.begin(), ref)
+#define CHECK_SIZE_RANGE(range, ref) BOOST_CHECK_EQUAL((range).end() - (range).begin(), ref)
 #define CHECK_EQUAL_RANGE(range, ref)                                                              \
     do                                                                                             \
     {                                                                                              \

unit_tests/partitioner/graph_generator.hpp

@@ -7,8 +7,6 @@
 #include <algorithm>
 #include <vector>
 
-using namespace osrm::util;
-
 struct EdgeWithSomeAdditionalData
 {
     NodeID source;
@@ -16,16 +14,17 @@ struct EdgeWithSomeAdditionalData
     unsigned important_data;
 };
 
-inline Coordinate
+inline osrm::util::Coordinate
 makeCoordinate(int x, int y, double step_size, double offset_x = 0, double offset_y = 0)
 {
-    return {FloatLongitude{offset_x + x * step_size}, FloatLatitude{offset_y + y * step_size}};
+    return {osrm::util::FloatLongitude{offset_x + x * step_size},
+            osrm::util::FloatLatitude{offset_y + y * step_size}};
 }
 
-std::vector<Coordinate> inline makeGridCoordinates(
+std::vector<osrm::util::Coordinate> inline makeGridCoordinates(
     int rows, int columns, double step_size, double lon_base, double lat_base)
 {
-    std::vector<Coordinate> result;
+    std::vector<osrm::util::Coordinate> result;
 
     for (int r = 0; r < rows; ++r)
         for (int c = 0; c < columns; ++c)

unit_tests/partitioner/multi_level_partition.cpp

@@ -8,7 +8,7 @@
 
 #include "partitioner/multi_level_partition.hpp"
 
-#define CHECK_SIZE_RANGE(range, ref) BOOST_CHECK_EQUAL(range.second - range.first, ref)
+#define CHECK_SIZE_RANGE(range, ref) BOOST_CHECK_EQUAL((range).second - (range).first, ref)
 #define CHECK_EQUAL_RANGE(range, ref)                                                              \
     do                                                                                             \
     {                                                                                              \

unit_tests/partitioner/recursive_bisection.cpp

@@ -72,7 +72,7 @@ BOOST_AUTO_TEST_CASE(dividing_four_grid_cells)
 
     RecursiveBisection bisection(graph, 120, 1.1, 0.25, 10, 1);
 
-    const auto result = bisection.BisectionIDs();
+    const auto &result = bisection.BisectionIDs();
     // all same IDs withing a group
     for (int i = 0; i < 4; ++i)
         for (int j = 0; j < rows * cols; ++j)

unit_tests/partitioner/remove_unconnected.cpp

@@ -5,7 +5,7 @@
 
 #include "util/static_graph.hpp"
 
-#define CHECK_SIZE_RANGE(range, ref) BOOST_CHECK_EQUAL(range.end() - range.begin(), ref)
+#define CHECK_SIZE_RANGE(range, ref) BOOST_CHECK_EQUAL((range).end() - (range).begin(), ref)
 #define CHECK_EQUAL_RANGE(range, ref)                                                              \
     do                                                                                             \
     {                                                                                              \

unit_tests/server/parameters_parser.cpp

@@ -17,7 +17,7 @@
 #include <boost/test/unit_test.hpp>
 
 #define CHECK_EQUAL_RANGE(R1, R2)                                                                  \
-    BOOST_CHECK_EQUAL_COLLECTIONS(R1.begin(), R1.end(), R2.begin(), R2.end());
+    BOOST_CHECK_EQUAL_COLLECTIONS((R1).begin(), (R1).end(), (R2).begin(), (R2).end());
 
 BOOST_AUTO_TEST_SUITE(api_parameters_parser)
 

unit_tests/server/url_parser.cpp

@@ -23,7 +23,7 @@ std::ostream &operator<<(std::ostream &out, const osrm::server::api::ParsedURL &
 #include <boost/test/unit_test.hpp>
 
 #define CHECK_EQUAL_RANGE(R1, R2)                                                                  \
-    BOOST_CHECK_EQUAL_COLLECTIONS(R1.begin(), R1.end(), R2.begin(), R2.end());
+    BOOST_CHECK_EQUAL_COLLECTIONS((R1).begin(), (R1).end(), (R2).begin(), (R2).end());
 
 BOOST_AUTO_TEST_SUITE(api_url_parser)