Enable even more clang-tidy checks (#6273)

This commit is contained in:
Siarhei Fedartsou 2022-07-04 22:46:59 +02:00 committed by GitHub
parent 19d2ec56b8
commit 06b1b980bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
39 changed files with 84 additions and 76 deletions

View File

@ -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,

View File

@ -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)

View File

@ -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>;

View File

@ -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);

View File

@ -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),

View File

@ -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(

View File

@ -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)
{

View File

@ -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

View File

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

View File

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

View File

@ -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)
{
}

View File

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

View File

@ -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>;

View File

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

View File

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

View File

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

View File

@ -30,11 +30,11 @@ 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>
@ -42,11 +42,11 @@ 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

View File

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

View File

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

View File

@ -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,

View File

@ -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>;

View File

@ -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};
}

View File

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

View File

@ -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)

View File

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

View File

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

View File

@ -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);
}
/**

View File

@ -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.

View File

@ -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)

View File

@ -192,8 +192,8 @@ util::json::Object makeRouteStep(guidance::RouteStep step, util::json::Value geo
}
}
route_step.values["mode"] = extractor::travelModeToString(std::move(step.mode));
route_step.values["maneuver"] = makeStepManeuver(std::move(step.maneuver));
route_step.values["mode"] = extractor::travelModeToString(step.mode);
route_step.values["maneuver"] = makeStepManeuver(step.maneuver);
route_step.values["geometry"] = std::move(geometry);
route_step.values["driving_side"] = step.is_left_hand_driving ? "left" : "right";

View File

@ -484,7 +484,9 @@ std::
// OSM elements Lua parser
tbb::filter_t<SharedBuffer, ParsedBuffer> buffer_transformer(
tbb::filter::parallel, [&](const SharedBuffer buffer) {
tbb::filter::parallel,
// NOLINTNEXTLINE(performance-unnecessary-value-param)
[&](const SharedBuffer buffer) {
ParsedBuffer parsed_buffer;
parsed_buffer.buffer = buffer;
scripting_environment.ProcessElements(*buffer,
@ -531,7 +533,9 @@ std::
});
tbb::filter_t<SharedBuffer, std::shared_ptr<ExtractionRelationContainer>> buffer_relation_cache(
tbb::filter::parallel, [&](const SharedBuffer buffer) {
tbb::filter::parallel,
// NOLINTNEXTLINE(performance-unnecessary-value-param)
[&](const SharedBuffer buffer) {
if (!buffer)
return std::shared_ptr<ExtractionRelationContainer>{};
@ -568,6 +572,7 @@ std::
unsigned number_of_relations = 0;
tbb::filter_t<std::shared_ptr<ExtractionRelationContainer>, void> buffer_storage_relation(
tbb::filter::serial_in_order,
// NOLINTNEXTLINE(performance-unnecessary-value-param)
[&](const std::shared_ptr<ExtractionRelationContainer> parsed_relations) {
number_of_relations += parsed_relations->GetRelationsNum();
relations.Merge(std::move(*parsed_relations));

View File

@ -419,10 +419,10 @@ bool MergableRoadDetector::HaveSameDirection(const NodeID intersection_node,
// sampling to correctly weight longer segments in regression calculations
const auto constexpr SAMPLE_INTERVAL = 5;
coordinates_to_the_left = coordinate_extractor.SampleCoordinates(
std::move(coordinates_to_the_left), distance_to_extract, SAMPLE_INTERVAL);
coordinates_to_the_left, distance_to_extract, SAMPLE_INTERVAL);
coordinates_to_the_right = coordinate_extractor.SampleCoordinates(
std::move(coordinates_to_the_right), distance_to_extract, SAMPLE_INTERVAL);
coordinates_to_the_right, distance_to_extract, SAMPLE_INTERVAL);
/* extract the number of lanes for a road
* restricts a vector to the last two thirds of the length

View File

@ -891,7 +891,7 @@ void Sol2ScriptingEnvironment::ProcessElements(
{
local_context.ProcessNode(node, result_node, relations);
}
resulting_nodes.push_back({node, std::move(result_node)});
resulting_nodes.push_back({node, result_node});
}
break;
case osmium::item_type::way:

View File

@ -109,8 +109,8 @@ std::unordered_set<EdgeID> findSegregatedNodes(const extractor::NodeBasedGraphFa
};
auto isSegregated = [&](NodeID node1,
std::vector<EdgeInfo> v1,
std::vector<EdgeInfo> v2,
const std::vector<EdgeInfo> &v1,
const std::vector<EdgeInfo> &v2,
EdgeInfo const &current,
double edge_length) {
// Internal intersection edges must be short and cannot be a roundabout.

View File

@ -64,7 +64,7 @@ bool TurnLaneData::operator<(const TurnLaneData &other) const
std::find(tag_by_modifier, tag_by_modifier + 8, other.tag);
}
LaneDataVector laneDataFromDescription(TurnLaneDescription turn_lane_description)
LaneDataVector laneDataFromDescription(const TurnLaneDescription &turn_lane_description)
{
typedef std::unordered_map<TurnLaneType::Mask, std::pair<LaneID, LaneID>> LaneMap;
// TODO need to handle cases that have none-in between two identical values

View File

@ -70,6 +70,7 @@ namespace
template <typename T> inline bool is_aligned(const void *pointer)
{
// NOLINTNEXTLINE(misc-redundant-expression)
static_assert(sizeof(T) % alignof(T) == 0, "pointer can not be used as an array pointer");
return reinterpret_cast<uintptr_t>(pointer) % alignof(T) == 0;
}

View File

@ -55,7 +55,7 @@ util::json::Object
CoordinateVectorToMultiPoint::operator()(const std::vector<util::Coordinate> &input_coordinates,
const boost::optional<json::Object> &properties) const
{
const auto coordinates = makeJsonArray(input_coordinates);
auto coordinates = makeJsonArray(input_coordinates);
return makeFeature("MultiPoint", std::move(coordinates), properties);
}
@ -64,7 +64,7 @@ util::json::Object
CoordinateVectorToLineString::operator()(const std::vector<util::Coordinate> &input_coordinates,
const boost::optional<json::Object> &properties) const
{
const auto coordinates = makeJsonArray(input_coordinates);
auto coordinates = makeJsonArray(input_coordinates);
return makeFeature("LineString", std::move(coordinates), properties);
}

View File

@ -15,7 +15,7 @@ using namespace osrm::extractor;
TurnRestriction makeWayRestriction(NodeID from, std::vector<NodeID> via, NodeID to, bool is_only)
{
WayRestriction wr{from, via, to};
WayRestriction wr{from, std::move(via), to};
return TurnRestriction(wr, is_only);
}
@ -100,8 +100,8 @@ std::map<NodeID, size_t> checkNode(const RestrictionGraph &graph,
std::vector<instruction> expected_instructions,
std::vector<NodeID> expected_edges)
{
checkInstructions(graph.GetRestrictions(node_id), expected_instructions);
checkEdges(graph.GetEdges(node_id), expected_edges);
checkInstructions(graph.GetRestrictions(node_id), std::move(expected_instructions));
checkEdges(graph.GetEdges(node_id), std::move(expected_edges));
return nextEdges(graph.GetEdges(node_id));
}
@ -117,7 +117,8 @@ validateStartRestrictionNode(RestrictionGraph &graph,
BOOST_REQUIRE_GE(node_id, graph.num_via_nodes);
BOOST_REQUIRE_LT(node_id, graph.nodes.size());
return checkNode(graph, node_id, restriction_instructions, restriction_edges);
return checkNode(
graph, node_id, std::move(restriction_instructions), std::move(restriction_edges));
}
std::map<NodeID, size_t>
@ -139,7 +140,8 @@ validateViaRestrictionNode(RestrictionGraph &graph,
<< to);
BOOST_REQUIRE_LT(via_node_idx, graph.num_via_nodes);
return checkNode(graph, via_node_idx, restriction_instructions, restriction_edges);
return checkNode(
graph, via_node_idx, std::move(restriction_instructions), std::move(restriction_edges));
}
BOOST_AUTO_TEST_CASE(empty_restrictions)

View File

@ -14,7 +14,7 @@ using namespace osrm::util;
constexpr unsigned BLOCK_SIZE = 16;
typedef RangeTable<BLOCK_SIZE, osrm::storage::Ownership::Container> TestRangeTable;
void ConstructionTest(std::vector<unsigned> lengths, std::vector<unsigned> offsets)
void ConstructionTest(const std::vector<unsigned> &lengths, const std::vector<unsigned> &offsets)
{
BOOST_CHECK_EQUAL(lengths.size(), offsets.size() - 1);