Enable even more clang-tidy checks (#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)