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 performance-move-const-arg clang-tidy check #6319

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
1 change: 0 additions & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ Checks: >
-misc-unused-parameters,
performance-*,
-performance-noexcept-move-constructor,
-performance-move-const-arg,
-performance-no-int-to-ptr,
readability-*,
-readability-avoid-const-params-in-decls,
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
- FIXED: Bug in bicycle profile that caused exceptions if there is a highway=bicycle in the data. [#6296](https://github.com/Project-OSRM/osrm-backend/pull/6296)
- FIXED: Internal refactoring of identifier types used in data facade [#6044](https://github.com/Project-OSRM/osrm-backend/pull/6044)
- Build:
- CHANGED: Enable performance-move-const-arg clang-tidy check. [#6319](https://github.com/Project-OSRM/osrm-backend/pull/6319)
- CHANGED: Use the latest node on CI. [#6317](https://github.com/Project-OSRM/osrm-backend/pull/6317)
- CHANGED: Migrate Windows CI to GitHub Actions. [#6312](https://github.com/Project-OSRM/osrm-backend/pull/6312)
- ADDED: Add smoke test for Docker image. [#6313](https://github.com/Project-OSRM/osrm-backend/pull/6313)
Expand Down
4 changes: 2 additions & 2 deletions include/contractor/contract_excludable_graph.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ inline auto contractFullGraph(ContractorGraph contractor_graph,
auto edges = toEdges<QueryEdge>(std::move(contractor_graph));
std::vector<bool> edge_filter(edges.size(), true);

return GraphAndFilter{QueryGraph{num_nodes, std::move(edges)}, {std::move(edge_filter)}};
return GraphAndFilter{QueryGraph{num_nodes, edges}, {std::move(edge_filter)}};
}

inline auto contractExcludableGraph(ContractorGraph contractor_graph_,
Expand Down Expand Up @@ -91,7 +91,7 @@ inline auto contractExcludableGraph(ContractorGraph contractor_graph_,
edge_container.Merge(toEdges<QueryEdge>(std::move(filtered_core_graph)));
}

return GraphAndFilter{QueryGraph{num_nodes, std::move(edge_container.edges)},
return GraphAndFilter{QueryGraph{num_nodes, edge_container.edges},
Copy link
Member Author

Choose a reason for hiding this comment

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

Here we could probably change the signature of constructor to accept rvalues, but I decided to just keep things "as is" for now.

edge_container.MakeEdgeFilters()};
}
} // namespace contractor
Expand Down
4 changes: 2 additions & 2 deletions include/contractor/query_edge.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ struct QueryEdge

QueryEdge() : source(SPECIAL_NODEID), target(SPECIAL_NODEID) {}

QueryEdge(NodeID source, NodeID target, EdgeData data)
: source(source), target(target), data(std::move(data))
QueryEdge(NodeID source, NodeID target, const EdgeData &data)
: source(source), target(target), data(data)
{
}

Expand Down
2 changes: 1 addition & 1 deletion include/engine/guidance/assemble_geometry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ inline LegGeometry assembleGeometry(const datafacade::BaseDataFacade &facade,
(path_point.weight_until_turn - path_point.weight_of_turn) /
facade.GetWeightMultiplier(),
path_point.datasource_id});
geometry.locations.push_back(std::move(coordinate));
geometry.locations.push_back(coordinate);
geometry.node_ids.push_back(node_id);
}
}
Expand Down
6 changes: 3 additions & 3 deletions include/extractor/internal_extractor_edge.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,16 @@ struct InternalExtractorEdge
WeightData weight_data,
DurationData duration_data,
util::Coordinate source_coordinate)
: result(source, target, 0, 0, 0, {}, -1, {}), weight_data(std::move(weight_data)),
duration_data(std::move(duration_data)), source_coordinate(std::move(source_coordinate))
: result(source, target, 0, 0, 0, {}, -1, {}), weight_data(weight_data),
duration_data(duration_data), source_coordinate(source_coordinate)
{
}

explicit InternalExtractorEdge(NodeBasedEdgeWithOSM edge,
WeightData weight_data,
DurationData duration_data,
util::Coordinate source_coordinate)
: result(std::move(edge)), weight_data(weight_data), duration_data(duration_data),
: result(edge), weight_data(weight_data), duration_data(duration_data),
source_coordinate(source_coordinate)
{
}
Expand Down
5 changes: 3 additions & 2 deletions include/partitioner/bisection_graph.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ namespace partitioner
// located at for use in the inertial flow sorting by slope.
struct BisectionNode
{
BisectionNode(util::Coordinate coordinate_ = {util::FloatLongitude{0}, util::FloatLatitude{0}},
BisectionNode(const util::Coordinate &coordinate_ = {util::FloatLongitude{0},
util::FloatLatitude{0}},
const NodeID original_id_ = SPECIAL_NODEID)
: coordinate(std::move(coordinate_)), original_id(original_id_)
: coordinate(coordinate_), original_id(original_id_)
{
}

Expand Down
2 changes: 1 addition & 1 deletion include/partitioner/edge_based_graph_reader.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ inline DynamicEdgeBasedGraph LoadEdgeBasedGraph(const boost::filesystem::path &p
auto directed = splitBidirectionalEdges(edges);
auto tidied = prepareEdgesForUsageInGraph<DynamicEdgeBasedGraphEdge>(std::move(directed));

return DynamicEdgeBasedGraph(number_of_edge_based_nodes, std::move(tidied), checksum);
return DynamicEdgeBasedGraph(number_of_edge_based_nodes, tidied, checksum);
}

} // namespace partitioner
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 @@ -59,7 +59,7 @@ class BaseDataLayout
public:
virtual ~BaseDataLayout() = default;

inline void SetBlock(const std::string &name, Block block) { blocks[name] = std::move(block); }
inline void SetBlock(const std::string &name, const Block &block) { blocks[name] = block; }

inline std::uint64_t GetBlockEntries(const std::string &name) const
{
Expand Down
60 changes: 26 additions & 34 deletions include/storage/view_factory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ inline auto make_name_table_view(const SharedDataIndex &index, const std::string
auto values =
make_vector_view<extractor::NameTableView::IndexedData::ValueType>(index, name + "/values");

extractor::NameTableView::IndexedData index_data_view{std::move(blocks), std::move(values)};
extractor::NameTableView::IndexedData index_data_view{blocks, values};
return extractor::NameTableView{index_data_view};
}

Expand Down Expand Up @@ -156,14 +156,14 @@ inline auto make_segment_data_view(const SharedDataIndex &index, const std::stri
auto rev_datasources_list =
make_vector_view<DatasourceID>(index, name + "/reverse_data_sources");

return extractor::SegmentDataView{std::move(geometry_begin_indices),
std::move(node_list),
std::move(fwd_weight_list),
std::move(rev_weight_list),
std::move(fwd_duration_list),
std::move(rev_duration_list),
std::move(fwd_datasources_list),
std::move(rev_datasources_list)};
return extractor::SegmentDataView{geometry_begin_indices,
node_list,
fwd_weight_list,
rev_weight_list,
fwd_duration_list,
rev_duration_list,
fwd_datasources_list,
rev_datasources_list};
Copy link
Member

Choose a reason for hiding this comment

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

👍

}

inline auto make_coordinates_view(const SharedDataIndex &index, const std::string &name)
Expand Down Expand Up @@ -214,7 +214,7 @@ inline auto make_search_tree_view(const SharedDataIndex &index, const std::strin
}

return util::StaticRTree<RTreeLeaf, storage::Ownership::View>{
std::move(search_tree), std::move(rtree_level_starts), path, std::move(coordinates)};
search_tree, rtree_level_starts, path, coordinates};
}

inline auto make_intersection_bearings_view(const SharedDataIndex &index, const std::string &name)
Expand All @@ -225,13 +225,11 @@ inline auto make_intersection_bearings_view(const SharedDataIndex &index, const
index, name + "/class_id_to_ranges/diff_blocks");
auto bearing_values = make_vector_view<DiscreteBearing>(index, name + "/bearing_values");
util::RangeTable<16, storage::Ownership::View> bearing_range_table(
std::move(bearing_offsets),
std::move(bearing_blocks),
static_cast<unsigned>(bearing_values.size()));
bearing_offsets, bearing_blocks, static_cast<unsigned>(bearing_values.size()));

auto bearing_class_id = make_vector_view<BearingClassID>(index, name + "/node_to_class_id");
return extractor::IntersectionBearingsView{
std::move(bearing_values), std::move(bearing_class_id), std::move(bearing_range_table)};
bearing_values, bearing_class_id, bearing_range_table};
}

inline auto make_entry_classes_view(const SharedDataIndex &index, const std::string &name)
Expand All @@ -252,8 +250,7 @@ inline auto make_contracted_metric_view(const SharedDataIndex &index, const std:
edge_filter.push_back(make_vector_view<bool>(index, filter_name));
}));

return contractor::ContractedMetricView{{std::move(node_list), std::move(edge_list)},
std::move(edge_filter)};
return contractor::ContractedMetricView{{node_list, edge_list}, std::move(edge_filter)};
}

inline auto make_partition_view(const SharedDataIndex &index, const std::string &name)
Expand All @@ -264,8 +261,7 @@ inline auto make_partition_view(const SharedDataIndex &index, const std::string
auto partition = make_vector_view<PartitionID>(index, name + "/partition");
auto cell_to_children = make_vector_view<CellID>(index, name + "/cell_to_children");

return partitioner::MultiLevelPartitionView{
level_data_ptr, std::move(partition), std::move(cell_to_children)};
return partitioner::MultiLevelPartitionView{level_data_ptr, partition, cell_to_children};
}

inline auto make_timestamp_view(const SharedDataIndex &index, const std::string &name)
Expand All @@ -280,10 +276,8 @@ inline auto make_cell_storage_view(const SharedDataIndex &index, const std::stri
auto cells = make_vector_view<partitioner::CellStorageView::CellData>(index, name + "/cells");
auto level_offsets = make_vector_view<std::uint64_t>(index, name + "/level_to_cell_offset");

return partitioner::CellStorageView{std::move(source_boundary),
std::move(destination_boundary),
std::move(cells),
std::move(level_offsets)};
return partitioner::CellStorageView{
source_boundary, destination_boundary, cells, level_offsets};
}

inline auto make_filtered_cell_metric_view(const SharedDataIndex &index,
Expand All @@ -301,8 +295,7 @@ inline auto make_filtered_cell_metric_view(const SharedDataIndex &index,
auto durations = make_vector_view<EdgeDuration>(index, durations_block_id);
auto distances = make_vector_view<EdgeDistance>(index, distances_block_id);

return customizer::CellMetricView{
std::move(weights), std::move(durations), std::move(distances)};
return customizer::CellMetricView{weights, durations, distances};
}

inline auto make_cell_metric_view(const SharedDataIndex &index, const std::string &name)
Expand All @@ -321,8 +314,7 @@ inline auto make_cell_metric_view(const SharedDataIndex &index, const std::strin
auto durations = make_vector_view<EdgeDuration>(index, durations_block_id);
auto distances = make_vector_view<EdgeDistance>(index, distances_block_id);

cell_metric_excludes.push_back(customizer::CellMetricView{
std::move(weights), std::move(durations), std::move(distances)});
cell_metric_excludes.push_back(customizer::CellMetricView{weights, durations, distances});
}

return cell_metric_excludes;
Expand All @@ -342,14 +334,14 @@ inline auto make_multi_level_graph_view(const SharedDataIndex &index, const std:
auto is_forward_edge = make_vector_view<bool>(index, name + "/is_forward_edge");
auto is_backward_edge = make_vector_view<bool>(index, name + "/is_backward_edge");

return customizer::MultiLevelEdgeBasedGraphView(std::move(node_list),
std::move(edge_list),
std::move(node_to_offset),
std::move(node_weights),
std::move(node_durations),
std::move(node_distances),
std::move(is_forward_edge),
std::move(is_backward_edge));
return customizer::MultiLevelEdgeBasedGraphView(node_list,
edge_list,
node_to_offset,
node_weights,
node_durations,
node_distances,
is_forward_edge,
is_backward_edge);
}

inline auto make_maneuver_overrides_views(const SharedDataIndex &index, const std::string &name)
Expand Down
2 changes: 1 addition & 1 deletion src/contractor/contractor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ int Contractor::Run()
std::tie(query_graph, edge_filters) = contractExcludableGraph(
toContractorGraph(number_of_edge_based_nodes, std::move(edge_based_edge_list)),
std::move(node_weights),
std::move(node_filters));
node_filters);
TIMER_STOP(contraction);
util::Log() << "Contracted graph has " << query_graph.GetNumberOfEdges() << " edges.";
util::Log() << "Contraction took " << TIMER_SEC(contraction) << " sec";
Expand Down
3 changes: 1 addition & 2 deletions src/customize/customizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@ auto LoadAndUpdateEdgeExpandedGraph(const CustomizationConfig &config,
auto tidied = partitioner::prepareEdgesForUsageInGraph<
typename partitioner::MultiLevelEdgeBasedGraph::InputEdge>(std::move(directed));

auto edge_based_graph =
partitioner::MultiLevelEdgeBasedGraph(mlp, num_nodes, std::move(tidied));
auto edge_based_graph = partitioner::MultiLevelEdgeBasedGraph(mlp, num_nodes, tidied);

return edge_based_graph;
}
Expand Down
2 changes: 1 addition & 1 deletion src/engine/routing_algorithms/alternative_path_mld.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,7 @@ InternalManyRoutesResult alternativePathSearch(SearchEngineData<Algorithm> &sear
const auto extract_packed_path_from_heaps = [&](WeightedViaNode via) {
auto packed_path = retrievePackedPathFromHeap(forward_heap, reverse_heap, via.node);

return WeightedViaNodePackedPath{std::move(via), std::move(packed_path)};
return WeightedViaNodePackedPath{via, std::move(packed_path)};
};

std::vector<WeightedViaNodePackedPath> weighted_packed_paths;
Expand Down
2 changes: 1 addition & 1 deletion src/extractor/extraction_containers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ void ExtractionContainers::PrepareNodes()
continue;
}
BOOST_ASSERT(node_iter->node_id == *ref_iter);
*used_nodes_iter = std::move(*ref_iter);
*used_nodes_iter = *ref_iter;
used_nodes_iter++;
node_iter++;
ref_iter++;
Expand Down
10 changes: 5 additions & 5 deletions src/extractor/extractor_callbacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti
parsed_way.pronunciation,
parsed_way.exits};
auto v = MapVal{name_id};
string_map.emplace(std::move(k), std::move(v));
string_map.emplace(std::move(k), v);
}
else
{
Expand Down Expand Up @@ -441,8 +441,8 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti
parsed_way.highway_turn_classification,
parsed_way.access_turn_classification}};

external_memory.all_edges_list.push_back(InternalExtractorEdge(
std::move(edge), forward_weight_data, forward_duration_data, {}));
external_memory.all_edges_list.push_back(
InternalExtractorEdge(edge, forward_weight_data, forward_duration_data, {}));
});
}

Expand Down Expand Up @@ -475,8 +475,8 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti
parsed_way.highway_turn_classification,
parsed_way.access_turn_classification}};

external_memory.all_edges_list.push_back(InternalExtractorEdge(
std::move(edge), backward_weight_data, backward_duration_data, {}));
external_memory.all_edges_list.push_back(
InternalExtractorEdge(edge, backward_weight_data, backward_duration_data, {}));
});
}

Expand Down
4 changes: 2 additions & 2 deletions src/partitioner/inertial_flow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ makeSpatialOrder(const BisectionGraphView &view, const double ratio, const doubl
{
struct NodeWithCoordinate
{
NodeWithCoordinate(NodeID nid_, util::Coordinate coordinate_)
: nid{nid_}, coordinate{std::move(coordinate_)}
NodeWithCoordinate(NodeID nid_, const util::Coordinate &coordinate_)
: nid{nid_}, coordinate{coordinate_}
{
}

Expand Down
4 changes: 2 additions & 2 deletions src/partitioner/recursive_bisection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,13 @@ RecursiveBisection::RecursiveBisection(BisectionGraph &bisection_graph_,
TreeNode left_node{std::move(left_graph), node.depth + 1};

if (!terminal(left_node))
feeder.add(std::move(left_node));
feeder.add(left_node);

BisectionGraphView right_graph{bisection_graph, center, node.graph.End()};
TreeNode right_node{std::move(right_graph), node.depth + 1};

if (!terminal(right_node))
feeder.add(std::move(right_node));
feeder.add(right_node);
});

TIMER_STOP(bisection);
Expand Down
2 changes: 1 addition & 1 deletion src/util/coordinate_calculation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ Coordinate interpolateLinear(double factor, const Coordinate from, const Coordin
FixedLatitude interpolated_lat{
static_cast<std::int32_t>(from_lat + factor * (to_lat - from_lat))};

return {std::move(interpolated_lon), std::move(interpolated_lat)};
return {interpolated_lon, interpolated_lat};
}

// compute the signed area of a triangle
Expand Down
2 changes: 1 addition & 1 deletion unit_tests/contractor/helper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ inline contractor::ContractorGraph makeGraph(const std::vector<TestEdge> &edges)
}
std::sort(input_edges.begin(), input_edges.end());

return contractor::ContractorGraph{max_id + 1, std::move(input_edges)};
return contractor::ContractorGraph{max_id + 1, input_edges};
}
} // namespace unit_test
} // namespace osrm
Expand Down