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

Upgrade clang-format to version 15 #6859

Merged
merged 8 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/osrm-backend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ jobs:
token: ${{ secrets.GITHUB_TOKEN }}

format-taginfo-docs:
runs-on: ubuntu-20.04
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v3
- name: Use Node.js
Expand All @@ -88,7 +88,7 @@ jobs:
- name: Prepare environment
run: |
npm ci --ignore-scripts
clang-format-10 --version
Copy link
Member

Choose a reason for hiding this comment

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

It seems ubuntu-22.04 has clang-format-15. May be worth updating to it as we have to reformat a lot of files anyway? https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md

Btw 24.04 seems to be already available as well…

Copy link
Collaborator Author

@DennisOSRM DennisOSRM May 5, 2024

Choose a reason for hiding this comment

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

Oh great, then I'll amend this PR to go straight to 24.04.

Copy link
Member

Choose a reason for hiding this comment

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

I made this conclusion from existence of https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2404-Readme.md, so not sure it is available in reality :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems that 24.04 based runners are not available yet. Sticking to 22.04 for now.

clang-format-15 --version
- name: Run checks
run: |
./scripts/check_taginfo.py taginfo.json profiles/car.lua
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- REMOVED: Drop support of Node 12 & 14. [#6431](https://github.com/Project-OSRM/osrm-backend/pull/6431)
- ADDED: Add 'load directly' mode to default Cucumber test suite. [#6663](https://github.com/Project-OSRM/osrm-backend/pull/6663)
- CHANGED: Drop support for Node 16 [#6855](https://github.com/Project-OSRM/osrm-backend/pull/6855)
- CHANGED: Upgrade clang-format to version 15 [#6859](https://github.com/Project-OSRM/osrm-backend/pull/6859)
- NodeJS:
- CHANGED: Use node-api instead of NAN. [#6452](https://github.com/Project-OSRM/osrm-backend/pull/6452)
- Misc:
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ You can add a :+1: emoji reaction to the issue if you want to express interest i

# Developer

We use `clang-format` version `3.8` to consistently format the code base. There is a helper script under `scripts/format.sh`.
We use `clang-format` version `15` to consistently format the code base. There is a helper script under `scripts/format.sh`.
The format is automatically checked by the `mason-linux-release` job of a Travis CI build.
To save development time a local hook `.git/hooks/pre-push`
```
Expand Down
14 changes: 8 additions & 6 deletions include/contractor/contract_excludable_graph.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,12 @@ inline auto contractExcludableGraph(ContractorGraph contractor_graph_,
// Add all non-core edges to container
{
auto non_core_edges = toEdges<QueryEdge>(contractor_graph);
auto new_end =
std::remove_if(non_core_edges.begin(), non_core_edges.end(), [&](const auto &edge) {
return is_shared_core[edge.source] && is_shared_core[edge.target];
});
auto new_end = std::remove_if(non_core_edges.begin(),
non_core_edges.end(),
[&](const auto &edge) {
return is_shared_core[edge.source] &&
is_shared_core[edge.target];
});
non_core_edges.resize(new_end - non_core_edges.begin());
edge_container.Insert(std::move(non_core_edges));

Expand All @@ -75,8 +77,8 @@ inline auto contractExcludableGraph(ContractorGraph contractor_graph_,
}

// Extract core graph for further contraction
shared_core_graph = contractor_graph.Filter(
[&is_shared_core](const NodeID node) { return is_shared_core[node]; });
shared_core_graph = contractor_graph.Filter([&is_shared_core](const NodeID node)
{ return is_shared_core[node]; });
}

for (const auto &filter : filters)
Expand Down
73 changes: 38 additions & 35 deletions include/contractor/contracted_edge_container.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,37 +89,40 @@ struct ContractedEdgeContainer

// Remove all edges that are contained in the old set of edges and set the appropriate flag.
auto new_end =
std::remove_if(new_edges.begin(), new_edges.end(), [&](const QueryEdge &edge) {
// check if the new edge would be sorted before the currend old edge
// if so it is not contained yet in the set of old edges
if (edge_iter == edge_end || mergeCompare(edge, *edge_iter))
{
return false;
}

// find the first old edge that is equal or greater then the new edge
while (edge_iter != edge_end && mergeCompare(*edge_iter, edge))
{
BOOST_ASSERT(flags_iter != flags.end());
edge_iter++;
flags_iter++;
}

// all new edges will be sorted after the old edges
if (edge_iter == edge_end)
{
return false;
}

BOOST_ASSERT(edge_iter != edge_end);
if (mergable(edge, *edge_iter))
{
*flags_iter = *flags_iter | flag;
return true;
}
BOOST_ASSERT(mergeCompare(edge, *edge_iter));
return false;
});
std::remove_if(new_edges.begin(),
new_edges.end(),
[&](const QueryEdge &edge)
{
// check if the new edge would be sorted before the currend old edge
// if so it is not contained yet in the set of old edges
if (edge_iter == edge_end || mergeCompare(edge, *edge_iter))
{
return false;
}

// find the first old edge that is equal or greater then the new edge
while (edge_iter != edge_end && mergeCompare(*edge_iter, edge))
{
BOOST_ASSERT(flags_iter != flags.end());
edge_iter++;
flags_iter++;
}

// all new edges will be sorted after the old edges
if (edge_iter == edge_end)
{
return false;
}

BOOST_ASSERT(edge_iter != edge_end);
if (mergable(edge, *edge_iter))
{
*flags_iter = *flags_iter | flag;
return true;
}
BOOST_ASSERT(mergeCompare(edge, *edge_iter));
return false;
});

// append new edges
edges.insert(edges.end(), new_edges.begin(), new_end);
Expand All @@ -132,10 +135,10 @@ struct ContractedEdgeContainer
// enforce sorting for next merge step
std::vector<unsigned> ordering(edges_size);
std::iota(ordering.begin(), ordering.end(), 0);
tbb::parallel_sort(
ordering.begin(), ordering.end(), [&](const auto lhs_idx, const auto rhs_idx) {
return mergeCompare(edges[lhs_idx], edges[rhs_idx]);
});
tbb::parallel_sort(ordering.begin(),
ordering.end(),
[&](const auto lhs_idx, const auto rhs_idx)
{ return mergeCompare(edges[lhs_idx], edges[rhs_idx]); });
auto permutation = util::orderingToPermutation(ordering);

util::inplacePermutation(edges.begin(), edges.end(), permutation);
Expand Down
3 changes: 2 additions & 1 deletion include/customizer/cell_customizer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ class CellCustomizer
for (std::size_t level = 1; level < partition.GetNumberOfLevels(); ++level)
{
tbb::parallel_for(tbb::blocked_range<std::size_t>(0, partition.GetNumberOfCells(level)),
[&](const tbb::blocked_range<std::size_t> &range) {
[&](const tbb::blocked_range<std::size_t> &range)
{
auto &heap = heaps.local();
for (auto id = range.begin(), end = range.end(); id != end; ++id)
{
Expand Down
13 changes: 6 additions & 7 deletions include/engine/api/base_api.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ class BaseAPI
util::json::Array waypoints;
waypoints.values.resize(parameters.coordinates.size());

boost::range::transform(
waypoint_candidates,
waypoints.values.begin(),
[this](const PhantomNodeCandidates &candidates) { return MakeWaypoint(candidates); });
boost::range::transform(waypoint_candidates,
waypoints.values.begin(),
[this](const PhantomNodeCandidates &candidates)
{ return MakeWaypoint(candidates); });
return waypoints;
}

Expand Down Expand Up @@ -104,9 +104,8 @@ class BaseAPI
std::transform(waypoint_candidates.begin(),
waypoint_candidates.end(),
waypoints.begin(),
[this, builder](const PhantomNodeCandidates &candidates) {
return MakeWaypoint(builder, candidates)->Finish();
});
[this, builder](const PhantomNodeCandidates &candidates)
{ return MakeWaypoint(builder, candidates)->Finish(); });
return builder->CreateVector(waypoints);
}

Expand Down
3 changes: 2 additions & 1 deletion include/engine/api/base_parameters.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ struct BaseParameters
(approaches.empty() || approaches.size() == coordinates.size()) &&
std::all_of(bearings.begin(),
bearings.end(),
[](const boost::optional<Bearing> &bearing_and_range) {
[](const boost::optional<Bearing> &bearing_and_range)
{
if (bearing_and_range)
{
return bearing_and_range->IsValid();
Expand Down
7 changes: 4 additions & 3 deletions include/engine/api/match_api.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,10 @@ class MatchAPI final : public RouteAPI
data_version_string = fb_result.CreateString(data_timestamp);
}

auto response = MakeFBResponse(sub_routes, fb_result, [this, &fb_result, &sub_matchings]() {
return MakeTracepoints(fb_result, sub_matchings);
});
auto response = MakeFBResponse(sub_routes,
fb_result,
[this, &fb_result, &sub_matchings]()
{ return MakeTracepoints(fb_result, sub_matchings); });

if (!data_timestamp.empty())
{
Expand Down
4 changes: 2 additions & 2 deletions include/engine/api/match_parameters.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ struct MatchParameters : public RouteParameters
MatchParameters(const std::vector<unsigned> &timestamps_,
GapsType gaps_,
bool tidy_,
Args &&... args_)
Args &&...args_)
: MatchParameters(timestamps_, gaps_, tidy_, {}, std::forward<Args>(args_)...)
{
}
Expand All @@ -77,7 +77,7 @@ struct MatchParameters : public RouteParameters
GapsType gaps_,
bool tidy_,
const std::vector<std::size_t> &waypoints_,
Args &&... args_)
Args &&...args_)
: RouteParameters{std::forward<Args>(args_)..., waypoints_}, timestamps{std::move(
timestamps_)},
gaps(gaps_), tidy(tidy_)
Expand Down
27 changes: 14 additions & 13 deletions include/engine/api/nearest_api.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,20 +57,20 @@ class NearestAPI final : public BaseAPI
{
std::vector<flatbuffers::Offset<fbresult::Waypoint>> waypoints;
waypoints.resize(phantom_nodes.front().size());
std::transform(
phantom_nodes.front().begin(),
phantom_nodes.front().end(),
waypoints.begin(),
[this, &fb_result](const PhantomNodeWithDistance &phantom_with_distance) {
auto &phantom_node = phantom_with_distance.phantom_node;
std::transform(phantom_nodes.front().begin(),
phantom_nodes.front().end(),
waypoints.begin(),
[this, &fb_result](const PhantomNodeWithDistance &phantom_with_distance)
{
auto &phantom_node = phantom_with_distance.phantom_node;

auto node_values = MakeNodes(phantom_node);
fbresult::Uint64Pair nodes{node_values.first, node_values.second};
auto node_values = MakeNodes(phantom_node);
fbresult::Uint64Pair nodes{node_values.first, node_values.second};

auto waypoint = MakeWaypoint(&fb_result, {phantom_node});
waypoint->add_nodes(&nodes);
return waypoint->Finish();
});
auto waypoint = MakeWaypoint(&fb_result, {phantom_node});
waypoint->add_nodes(&nodes);
return waypoint->Finish();
});

waypoints_vector = fb_result.CreateVector(waypoints);
}
Expand All @@ -94,7 +94,8 @@ class NearestAPI final : public BaseAPI
std::transform(phantom_nodes.front().begin(),
phantom_nodes.front().end(),
waypoints.values.begin(),
[this](const PhantomNodeWithDistance &phantom_with_distance) {
[this](const PhantomNodeWithDistance &phantom_with_distance)
{
auto &phantom_node = phantom_with_distance.phantom_node;
auto waypoint = MakeWaypoint({phantom_node});

Expand Down
Loading
Loading