Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable more clang-tidy checks. #6270

Merged
11 changes: 5 additions & 6 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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-*,
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
- API:
- FIXED: Fix inefficient osrm-routed connection handling [#6113](https:/Project-OSRM/osrm-backend/pull/6113)
- Build:
- CHANGED: Enable more clang-tidy checks. [#6262](https:/Project-OSRM/osrm-backend/pull/6270)
- CHANGED: Configure clang-tidy job on CI. [#6261](https:/Project-OSRM/osrm-backend/pull/6261)
- CHANGED: Use Github Actions for building container images [#6138](https:/Project-OSRM/osrm-backend/pull/6138)
- CHANGED: Upgrade Boost dependency to 1.70 [#6113](https:/Project-OSRM/osrm-backend/pull/6113)
Expand Down
2 changes: 1 addition & 1 deletion include/contractor/contract_excludable_graph.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion include/engine/datafacade_factory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
1 change: 0 additions & 1 deletion include/extractor/raster_source.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include <iterator>
#include <string>
#include <unordered_map>
using namespace std;

namespace osrm
{
Expand Down
2 changes: 1 addition & 1 deletion include/extractor/restriction.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
}
Expand Down
5 changes: 1 addition & 4 deletions include/guidance/turn_bearing.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
12 changes: 0 additions & 12 deletions include/guidance/turn_lane_handler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion include/server/server.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
7 changes: 4 additions & 3 deletions include/storage/io_config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_),
output_files(output_files_)
: required_input_files(std::move(required_input_files_)),
optional_input_files(std::move(optional_input_files_)),
output_files(std::move(output_files_))
{
}

Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion include/storage/shared_datatype.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
27 changes: 14 additions & 13 deletions include/util/exception.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
"Fingerprint did not match the expected value", // InvalidFingerprint
"File is incompatible with this version of OSRM", // IncompatibleFileVersion
"Problem opening file", // FileOpenError
"Problem reading from file", // FileReadError
"Problem writing to file", // FileWriteError
"I/O error occurred", // FileIOError
"Unexpected end of file", // UnexpectedEndOfFile
"The dataset you are trying to load is not " // IncompatibleDataset
"compatible with the routing algorithm you want to use." // ...continued...
"Incompatible algorithm" // IncompatibleAlgorithm
"", // Dummy - ErrorCode values start at 2
"", // Dummy - ErrorCode values start at 2
"Fingerprint did not match the expected value", // InvalidFingerprint
"File is incompatible with this version of OSRM", // IncompatibleFileVersion
"Problem opening file", // FileOpenError
"Problem reading from file", // FileReadError
"Problem writing to file", // FileWriteError
"I/O error occurred", // FileIOError
"Unexpected end of file", // UnexpectedEndOfFile
// NOLINTNEXTLINE(bugprone-suspicious-missing-comma)
"The dataset you are trying to load is not " // IncompatibleDataset
"compatible with the routing algorithm you want to use.", // ...continued...
"Incompatible algorithm" // IncompatibleAlgorithm
}};

#ifndef NDEBUG
Expand Down
2 changes: 1 addition & 1 deletion include/util/exception_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions include/util/guidance/name_announcements.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};

Expand Down
6 changes: 6 additions & 0 deletions include/util/range_table.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locally compiler complains that this block_sum is unused, it happens because it is only used in BOOST_ASSERT below.

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) ||
Expand Down
3 changes: 2 additions & 1 deletion src/customize/customizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions src/engine/api/json_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include <utility>
#include <vector>

namespace TurnType = osrm::guidance::TurnType;
using TurnInstruction = osrm::guidance::TurnInstruction;

namespace osrm
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 0 additions & 2 deletions src/extractor/intersection/intersection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@

#include <boost/range/adaptors.hpp>

using osrm::util::angularDeviation;

namespace osrm
{
namespace extractor
Expand Down
3 changes: 0 additions & 3 deletions src/guidance/driveway_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@

#include "util/assert.hpp"

using osrm::guidance::getTurnDirection;
using osrm::util::angularDeviation;

namespace osrm
{
namespace guidance
Expand Down
2 changes: 1 addition & 1 deletion src/guidance/turn_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (fork)
{
// makes sure that the fork is isolated from other neighbouring streets on the left and
Expand Down
2 changes: 1 addition & 1 deletion src/partitioner/dinic_max_flow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/updater/updater.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion src/util/conditional_restrictions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ namespace detail

namespace
{
namespace ph = boost::phoenix;
namespace qi = boost::spirit::qi;
} // namespace

Expand Down