Enable even more clang-tidy checks (Project-OSRM#6273)

.clang-tidy

@@ -15,6 +15,9 @@ Checks: >
     -clang-analyzer-*,
     -clang-diagnostic-deprecated-declarations,
     -clang-diagnostic-constant-conversion,
+    cppcoreguidelines-avoid-goto,
+    cppcoreguidelines-no-malloc,
+    cppcoreguidelines-virtual-class-destructor,
     google-*,
     -google-build-explicit-make-pair,
     -google-build-using-namespace,
@@ -40,7 +43,6 @@ Checks: >
     -misc-argument-comment,
     -misc-non-private-member-variables-in-classes,
     -misc-unconventional-assign-operator,
-    -misc-redundant-expression,
     -misc-no-recursion,
     -misc-misplaced-const,
     -misc-definitions-in-headers,
@@ -49,7 +51,6 @@ Checks: >
     -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 even more clang-tidy checks. [#6273](https://github.com/Project-OSRM/osrm-backend/pull/6273)
       - CHANGED: Configure CMake to not build flatbuffers tests and samples. [#6274](https://github.com/Project-OSRM/osrm-backend/pull/6274)
       - CHANGED: Enable more clang-tidy checks. [#6270](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)

include/contractor/query_graph.hpp

@@ -15,7 +15,7 @@ namespace detail
 {
 template <storage::Ownership Ownership>
 using QueryGraph = util::StaticGraph<typename QueryEdge::EdgeData, Ownership>;
-}
+} // namespace detail
 
 using QueryGraph = detail::QueryGraph<storage::Ownership::Container>;
 using QueryGraphView = detail::QueryGraph<storage::Ownership::View>;

include/engine/api/route_api.hpp

@@ -688,7 +688,7 @@ class RouteAPI : public BaseAPI
                     intersection.classes.begin(),
                     intersection.classes.end(),
                     classes.begin(),
-                    [&fb_result](const std::string cls) { return fb_result.CreateString(cls); });
+                    [&fb_result](const std::string &cls) { return fb_result.CreateString(cls); });
                 auto classes_vector = fb_result.CreateVector(classes);
                 auto entry_vector = fb_result.CreateVector(intersection.entry);
 

include/engine/datafacade/contiguous_internalmem_datafacade.hpp

@@ -630,7 +630,7 @@ class ContiguousInternalMemoryDataFacade<CH>
       public ContiguousInternalMemoryAlgorithmDataFacade<CH>
 {
   public:
-    ContiguousInternalMemoryDataFacade(std::shared_ptr<ContiguousBlockAllocator> allocator,
+    ContiguousInternalMemoryDataFacade(const std::shared_ptr<ContiguousBlockAllocator> &allocator,
                                        const std::string &metric_name,
                                        const std::size_t exclude_index)
         : ContiguousInternalMemoryDataFacadeBase(allocator, metric_name, exclude_index),
@@ -752,7 +752,7 @@ class ContiguousInternalMemoryDataFacade<MLD> final
 {
   private:
   public:
-    ContiguousInternalMemoryDataFacade(std::shared_ptr<ContiguousBlockAllocator> allocator,
+    ContiguousInternalMemoryDataFacade(const std::shared_ptr<ContiguousBlockAllocator> &allocator,
                                        const std::string &metric_name,
                                        const std::size_t exclude_index)
         : ContiguousInternalMemoryDataFacadeBase(allocator, metric_name, exclude_index),

include/engine/plugins/plugin_base.hpp

@@ -168,7 +168,7 @@ class BasePlugin
     std::vector<std::vector<PhantomNodeWithDistance>>
     GetPhantomNodesInRange(const datafacade::BaseDataFacade &facade,
                            const api::BaseParameters &parameters,
-                           const std::vector<double> radiuses,
+                           const std::vector<double> &radiuses,
                            bool use_all_edges = false) const
     {
         std::vector<std::vector<PhantomNodeWithDistance>> phantom_nodes(

include/engine/routing_algorithms/routing_base.hpp

@@ -336,7 +336,7 @@ void annotatePath(const FacadeT &facade,
 
 template <typename Algorithm>
 double getPathDistance(const DataFacade<Algorithm> &facade,
-                       const std::vector<PathData> unpacked_path,
+                       const std::vector<PathData> &unpacked_path,
                        const PhantomNode &source_phantom,
                        const PhantomNode &target_phantom)
 {

include/extractor/intersection_bearings_container.hpp

@@ -17,7 +17,7 @@ namespace extractor
 namespace detail
 {
 template <storage::Ownership Ownership> class IntersectionBearingsContainer;
-}
+} // namespace detail
 
 namespace serialization
 {
@@ -46,9 +46,10 @@ template <storage::Ownership Ownership> class IntersectionBearingsContainer
     IntersectionBearingsContainer &operator=(IntersectionBearingsContainer &&) = default;
     IntersectionBearingsContainer &operator=(const IntersectionBearingsContainer &) = default;
 
-    IntersectionBearingsContainer(std::vector<BearingClassID> node_to_class_id_,
+    // NOLINTNEXTLINE(performance-unnecessary-value-param)
+    IntersectionBearingsContainer(std::vector<BearingClassID> node_to_class_id,
                                   const std::vector<util::guidance::BearingClass> &bearing_classes)
-        : node_to_class_id(std::move(node_to_class_id_))
+        : node_to_class_id_(std::move(node_to_class_id))
     {
         std::vector<unsigned> bearing_counts(bearing_classes.size());
         std::transform(bearing_classes.begin(),
@@ -57,32 +58,32 @@ template <storage::Ownership Ownership> class IntersectionBearingsContainer
                        [](const auto &bearings) { return bearings.getAvailableBearings().size(); });
         // NOLINTNEXTLINE(bugprone-fold-init-type)
         auto total_bearings = std::accumulate(bearing_counts.begin(), bearing_counts.end(), 0);
-        class_id_to_ranges_table = RangeTable<16>{bearing_counts};
+        class_id_to_ranges_table_ = RangeTable<16>{bearing_counts};
 
-        values.reserve(total_bearings);
+        values_.reserve(total_bearings);
         for (const auto &bearing_class : bearing_classes)
         {
             const auto &bearings = bearing_class.getAvailableBearings();
-            values.insert(values.end(), bearings.begin(), bearings.end());
+            values_.insert(values_.end(), bearings.begin(), bearings.end());
         }
     }
 
-    IntersectionBearingsContainer(Vector<DiscreteBearing> values_,
-                                  Vector<BearingClassID> node_to_class_id_,
-                                  RangeTable<16> class_id_to_ranges_table_)
-        : values(std::move(values_)), node_to_class_id(std::move(node_to_class_id_)),
-          class_id_to_ranges_table(std::move(class_id_to_ranges_table_))
+    IntersectionBearingsContainer(Vector<DiscreteBearing> values,
+                                  Vector<BearingClassID> node_to_class_id,
+                                  RangeTable<16> class_id_to_ranges_table)
+        : values_(std::move(values)), node_to_class_id_(std::move(node_to_class_id)),
+          class_id_to_ranges_table_(std::move(class_id_to_ranges_table))
     {
     }
 
     // Returns the bearing class for an intersection node
     util::guidance::BearingClass GetBearingClass(const NodeID node) const
     {
-        auto class_id = node_to_class_id[node];
-        auto range = class_id_to_ranges_table.GetRange(class_id);
+        auto class_id = node_to_class_id_[node];
+        auto range = class_id_to_ranges_table_.GetRange(class_id);
         util::guidance::BearingClass result;
-        std::for_each(values.begin() + range.front(),
-                      values.begin() + range.back() + 1,
+        std::for_each(values_.begin() + range.front(),
+                      values_.begin() + range.back() + 1,
                       [&](const DiscreteBearing &bearing) { result.add(bearing); });
         return result;
     }
@@ -96,9 +97,9 @@ template <storage::Ownership Ownership> class IntersectionBearingsContainer
                                     const IntersectionBearingsContainer &turn_data_container);
 
   private:
-    Vector<DiscreteBearing> values;
-    Vector<BearingClassID> node_to_class_id;
-    RangeTable<16> class_id_to_ranges_table;
+    Vector<DiscreteBearing> values_;
+    Vector<BearingClassID> node_to_class_id_;
+    RangeTable<16> class_id_to_ranges_table_;
 };
 } // namespace detail
 

include/extractor/maneuver_override_relation_parser.hpp

@@ -10,7 +10,7 @@
 namespace osmium
 {
 class Relation;
-}
+} // namespace osmium
 
 namespace osrm
 {

include/extractor/name_table.hpp

@@ -15,7 +15,7 @@ namespace extractor
 namespace detail
 {
 template <storage::Ownership Ownership> class NameTableImpl;
-}
+} // namespace detail
 
 namespace serialization
 {

include/extractor/node_based_edge.hpp

@@ -192,7 +192,7 @@ inline NodeBasedEdgeWithOSM::NodeBasedEdgeWithOSM(OSMNodeID source,
                     geometry_id,
                     annotation_data,
                     flags),
-      osm_source_id(std::move(source)), osm_target_id(std::move(target))
+      osm_source_id(source), osm_target_id(target)
 {
 }
 

include/extractor/node_data_container.hpp

@@ -24,7 +24,7 @@ class EdgeBasedGraphFactory;
 namespace detail
 {
 template <storage::Ownership Ownership> class EdgeBasedNodeDataContainerImpl;
-}
+} // namespace detail
 
 namespace serialization
 {

include/extractor/packed_osm_ids.hpp

@@ -12,7 +12,7 @@ namespace detail
 {
 template <storage::Ownership Ownership>
 using PackedOSMIDs = util::detail::PackedVector<OSMNodeID, 34, Ownership>;
-}
+} // namespace detail
 
 using PackedOSMIDsView = detail::PackedOSMIDs<storage::Ownership::View>;
 using PackedOSMIDs = detail::PackedOSMIDs<storage::Ownership::Container>;

include/extractor/restriction_parser.hpp

@@ -11,7 +11,7 @@
 namespace osmium
 {
 class Relation;
-}
+} // namespace osmium
 
 namespace osrm
 {

include/extractor/scripting_environment.hpp

@@ -26,7 +26,7 @@ namespace osrm
 namespace util
 {
 struct Coordinate;
-}
+} // namespace util
 
 namespace extractor
 {

include/extractor/segment_data_container.hpp

@@ -27,7 +27,7 @@ class CompressedEdgeContainer;
 namespace detail
 {
 template <storage::Ownership Ownership> class SegmentDataContainerImpl;
-}
+} // namespace detail
 
 namespace serialization
 {

include/extractor/serialization.hpp

@@ -30,23 +30,23 @@ inline void read(storage::tar::FileReader &reader,
                  const std::string &name,
                  detail::IntersectionBearingsContainer<Ownership> &intersection_bearings)
 {
-    storage::serialization::read(reader, name + "/bearing_values", intersection_bearings.values);
+    storage::serialization::read(reader, name + "/bearing_values", intersection_bearings.values_);
     storage::serialization::read(
-        reader, name + "/node_to_class_id", intersection_bearings.node_to_class_id);
+        reader, name + "/node_to_class_id", intersection_bearings.node_to_class_id_);
     util::serialization::read(
-        reader, name + "/class_id_to_ranges", intersection_bearings.class_id_to_ranges_table);
+        reader, name + "/class_id_to_ranges", intersection_bearings.class_id_to_ranges_table_);
 }
 
 template <storage::Ownership Ownership>
 inline void write(storage::tar::FileWriter &writer,
                   const std::string &name,
                   const detail::IntersectionBearingsContainer<Ownership> &intersection_bearings)
 {
-    storage::serialization::write(writer, name + "/bearing_values", intersection_bearings.values);
+    storage::serialization::write(writer, name + "/bearing_values", intersection_bearings.values_);
     storage::serialization::write(
-        writer, name + "/node_to_class_id", intersection_bearings.node_to_class_id);
+        writer, name + "/node_to_class_id", intersection_bearings.node_to_class_id_);
     util::serialization::write(
-        writer, name + "/class_id_to_ranges", intersection_bearings.class_id_to_ranges_table);
+        writer, name + "/class_id_to_ranges", intersection_bearings.class_id_to_ranges_table_);
 }
 
 // read/write for properties file

include/guidance/segregated_intersection_classification.hpp

@@ -9,7 +9,7 @@ namespace osrm
 namespace extractor
 {
 class NodeBasedGraphFactory;
-}
+} // namespace extractor
 
 namespace guidance
 {

include/guidance/turn_data_container.hpp

@@ -19,7 +19,7 @@ namespace guidance
 namespace detail
 {
 template <storage::Ownership Ownership> class TurnDataContainerImpl;
-}
+} // namespace detail
 
 namespace serialization
 {

include/guidance/turn_lane_data.hpp

@@ -28,7 +28,7 @@ typedef std::vector<TurnLaneData> LaneDataVector;
 
 // convertes a string given in the OSM format into a TurnLaneData vector
 OSRM_ATTR_WARN_UNUSED
-LaneDataVector laneDataFromDescription(extractor::TurnLaneDescription turn_lane_description);
+LaneDataVector laneDataFromDescription(const extractor::TurnLaneDescription &turn_lane_description);
 
 // Locate A Tag in a lane data vector (if multiple tags are set, the first one found is returned)
 LaneDataVector::const_iterator findTag(const extractor::TurnLaneType::Mask tag,

include/partitioner/multi_level_partition.hpp

@@ -27,7 +27,7 @@ namespace partitioner
 namespace detail
 {
 template <storage::Ownership Ownership> class MultiLevelPartitionImpl;
-}
+} // namespace detail
 using MultiLevelPartition = detail::MultiLevelPartitionImpl<storage::Ownership::Container>;
 using MultiLevelPartitionView = detail::MultiLevelPartitionImpl<storage::Ownership::View>;
 

include/storage/block.hpp

@@ -33,6 +33,7 @@ using NamedBlock = std::tuple<std::string, Block>;
 
 template <typename T> Block make_block(uint64_t num_entries)
 {
+    // NOLINTNEXTLINE(misc-redundant-expression)
     static_assert(sizeof(T) % alignof(T) == 0, "aligned T* can't be used as an array pointer");
     return Block{num_entries, sizeof(T) * num_entries, 0};
 }

include/storage/shared_memory_ownership.hpp

@@ -12,7 +12,7 @@ enum class Ownership
     View,
     External
 };
-}
+} // namespace storage
 } // namespace osrm
 
 #endif // SHARED_MEMORY_OWNERSHIP_HPP

include/storage/view_factory.hpp

@@ -100,8 +100,7 @@ inline auto make_ebn_data_view(const SharedDataIndex &index, const std::string &
     auto annotation_data =
         make_vector_view<extractor::NodeBasedEdgeAnnotation>(index, name + "/annotations");
 
-    return extractor::EdgeBasedNodeDataView(std::move(edge_based_node_data),
-                                            std::move(annotation_data));
+    return extractor::EdgeBasedNodeDataView(edge_based_node_data, annotation_data);
 }
 
 inline auto make_turn_data_view(const SharedDataIndex &index, const std::string &name)
@@ -119,11 +118,8 @@ inline auto make_turn_data_view(const SharedDataIndex &index, const std::string
     const auto post_turn_bearings =
         make_vector_view<guidance::TurnBearing>(index, name + "/post_turn_bearings");
 
-    return guidance::TurnDataView(std::move(turn_instructions),
-                                  std::move(lane_data_ids),
-                                  std::move(entry_class_ids),
-                                  std::move(pre_turn_bearings),
-                                  std::move(post_turn_bearings));
+    return guidance::TurnDataView(
+        turn_instructions, lane_data_ids, entry_class_ids, pre_turn_bearings, post_turn_bearings);
 }
 
 inline auto make_segment_data_view(const SharedDataIndex &index, const std::string &name)

include/util/indexed_data.hpp

@@ -23,7 +23,7 @@ namespace util
 namespace detail
 {
 template <typename GroupBlockPolicy, storage::Ownership Ownership> struct IndexedDataImpl;
-}
+} // namespace detail
 
 namespace serialization
 {

include/util/packed_vector.hpp

@@ -24,7 +24,7 @@ namespace util
 namespace detail
 {
 template <typename T, std::size_t Bits, storage::Ownership Ownership> class PackedVector;
-}
+} // namespace detail
 
 namespace serialization
 {

include/util/static_rtree.hpp

@@ -461,9 +461,9 @@ class StaticRTree
     template <typename = std::enable_if<Ownership == storage::Ownership::Container>>
     explicit StaticRTree(const boost::filesystem::path &on_disk_file_name,
                          const Vector<Coordinate> &coordinate_list)
-        : m_coordinate_list(coordinate_list.data(), coordinate_list.size())
+        : m_coordinate_list(coordinate_list.data(), coordinate_list.size()),
+          m_objects(mmapFile<EdgeDataT>(on_disk_file_name, m_objects_region))
     {
-        m_objects = mmapFile<EdgeDataT>(on_disk_file_name, m_objects_region);
     }
 
     /**

include/util/timed_histogram.hpp

@@ -16,7 +16,7 @@ namespace util
 namespace detail
 {
 extern std::atomic_uint operation;
-}
+} // namespace detail
 
 /**
  * Captures a histogram with a bin size of `IndexBinSize` every `TimeBinSize` count operations.

include/util/viewport.hpp

@@ -25,7 +25,7 @@ static constexpr unsigned MIN_ZOOM = 1;
 // this is an upper bound to current display sizes
 static constexpr double VIEWPORT_WIDTH = 8 * web_mercator::TILE_SIZE;
 static constexpr double VIEWPORT_HEIGHT = 5 * web_mercator::TILE_SIZE;
-static double INV_LOG_2 = 1. / std::log(2);
+static const double INV_LOG_2 = 1. / std::log(2);
 } // namespace detail
 
 inline unsigned getFittedZoom(util::Coordinate south_west, util::Coordinate north_east)