Skip to content

Commit

Permalink
Enable performance-move-const-arg clang-tidy check (#6319)
Browse files Browse the repository at this point in the history
  • Loading branch information
SiarheiFedartsou committed Aug 23, 2022
1 parent 869b3fa commit 8e74b7a
Show file tree
Hide file tree
Showing 19 changed files with 55 additions and 63 deletions.
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},
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};
}

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

0 comments on commit 8e74b7a

Please sign in to comment.