From bb3e06599650dfc37d970bf829615b101cd073bc Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Sat, 20 Aug 2022 16:49:12 +0200 Subject: [PATCH 1/3] Enable performance-move-const-arg clang-tidy check --- .clang-tidy | 1 - .../contractor/contract_excludable_graph.hpp | 4 +- include/contractor/query_edge.hpp | 4 +- include/engine/guidance/assemble_geometry.hpp | 2 +- include/engine/guidance/assemble_steps.hpp | 4 +- include/extractor/internal_extractor_edge.hpp | 6 +- include/partitioner/bisection_graph.hpp | 5 +- .../partitioner/edge_based_graph_reader.hpp | 2 +- include/storage/shared_datatype.hpp | 2 +- include/storage/view_factory.hpp | 60 ++++++++----------- src/contractor/contractor.cpp | 2 +- src/customize/customizer.cpp | 3 +- src/extractor/edge_based_graph_factory.cpp | 2 +- src/extractor/extraction_containers.cpp | 2 +- src/extractor/extractor_callbacks.cpp | 10 ++-- src/partitioner/inertial_flow.cpp | 4 +- src/partitioner/recursive_bisection.cpp | 4 +- src/util/coordinate_calculation.cpp | 2 +- unit_tests/contractor/helper.hpp | 2 +- 19 files changed, 56 insertions(+), 65 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index d1b047f3a76..dbaf8536fe1 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -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, diff --git a/include/contractor/contract_excludable_graph.hpp b/include/contractor/contract_excludable_graph.hpp index 9d0cc697924..dbfe87da7d6 100644 --- a/include/contractor/contract_excludable_graph.hpp +++ b/include/contractor/contract_excludable_graph.hpp @@ -23,7 +23,7 @@ inline auto contractFullGraph(ContractorGraph contractor_graph, auto edges = toEdges(std::move(contractor_graph)); std::vector 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_, @@ -91,7 +91,7 @@ inline auto contractExcludableGraph(ContractorGraph contractor_graph_, edge_container.Merge(toEdges(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 diff --git a/include/contractor/query_edge.hpp b/include/contractor/query_edge.hpp index 351ecee2421..1b42faae2cd 100644 --- a/include/contractor/query_edge.hpp +++ b/include/contractor/query_edge.hpp @@ -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) { } diff --git a/include/engine/guidance/assemble_geometry.hpp b/include/engine/guidance/assemble_geometry.hpp index 9e3a0deafc5..0ff819c8afc 100644 --- a/include/engine/guidance/assemble_geometry.hpp +++ b/include/engine/guidance/assemble_geometry.hpp @@ -98,7 +98,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.osm_node_ids.push_back(osm_node_id); } } diff --git a/include/engine/guidance/assemble_steps.hpp b/include/engine/guidance/assemble_steps.hpp index ac65a539f22..59d4ac2b1c3 100644 --- a/include/engine/guidance/assemble_steps.hpp +++ b/include/engine/guidance/assemble_steps.hpp @@ -269,7 +269,7 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa leg_geometry.segment_distances[segment_index], weight / weight_multiplier, source_mode, - std::move(maneuver), + maneuver, leg_geometry.FrontIndex(segment_index), leg_geometry.BackIndex(segment_index) + 1, {intersection}, @@ -312,7 +312,7 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa ZERO_DISTANCE, ZERO_WEIGHT, target_mode, - std::move(maneuver), + maneuver, leg_geometry.locations.size() - 1, leg_geometry.locations.size(), {intersection}, diff --git a/include/extractor/internal_extractor_edge.hpp b/include/extractor/internal_extractor_edge.hpp index c7ee904d937..032786c7313 100644 --- a/include/extractor/internal_extractor_edge.hpp +++ b/include/extractor/internal_extractor_edge.hpp @@ -63,8 +63,8 @@ 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) { } @@ -72,7 +72,7 @@ struct InternalExtractorEdge 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) { } diff --git a/include/partitioner/bisection_graph.hpp b/include/partitioner/bisection_graph.hpp index f0a5d62ffee..e8dc8081744 100644 --- a/include/partitioner/bisection_graph.hpp +++ b/include/partitioner/bisection_graph.hpp @@ -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_) { } diff --git a/include/partitioner/edge_based_graph_reader.hpp b/include/partitioner/edge_based_graph_reader.hpp index 0c5d01b705f..3a92dae5eae 100644 --- a/include/partitioner/edge_based_graph_reader.hpp +++ b/include/partitioner/edge_based_graph_reader.hpp @@ -195,7 +195,7 @@ inline DynamicEdgeBasedGraph LoadEdgeBasedGraph(const boost::filesystem::path &p auto directed = splitBidirectionalEdges(edges); auto tidied = prepareEdgesForUsageInGraph(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 diff --git a/include/storage/shared_datatype.hpp b/include/storage/shared_datatype.hpp index bc4bf4efc68..8ffb9bf24c1 100644 --- a/include/storage/shared_datatype.hpp +++ b/include/storage/shared_datatype.hpp @@ -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 { diff --git a/include/storage/view_factory.hpp b/include/storage/view_factory.hpp index 1153103b97c..41ebf4283f1 100644 --- a/include/storage/view_factory.hpp +++ b/include/storage/view_factory.hpp @@ -77,7 +77,7 @@ inline auto make_name_table_view(const SharedDataIndex &index, const std::string auto values = make_vector_view(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}; } @@ -156,14 +156,14 @@ inline auto make_segment_data_view(const SharedDataIndex &index, const std::stri auto rev_datasources_list = make_vector_view(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) @@ -214,7 +214,7 @@ inline auto make_search_tree_view(const SharedDataIndex &index, const std::strin } return util::StaticRTree{ - 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) @@ -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(index, name + "/bearing_values"); util::RangeTable<16, storage::Ownership::View> bearing_range_table( - std::move(bearing_offsets), - std::move(bearing_blocks), - static_cast(bearing_values.size())); + bearing_offsets, bearing_blocks, static_cast(bearing_values.size())); auto bearing_class_id = make_vector_view(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) @@ -252,8 +250,7 @@ inline auto make_contracted_metric_view(const SharedDataIndex &index, const std: edge_filter.push_back(make_vector_view(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) @@ -264,8 +261,7 @@ inline auto make_partition_view(const SharedDataIndex &index, const std::string auto partition = make_vector_view(index, name + "/partition"); auto cell_to_children = make_vector_view(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) @@ -280,10 +276,8 @@ inline auto make_cell_storage_view(const SharedDataIndex &index, const std::stri auto cells = make_vector_view(index, name + "/cells"); auto level_offsets = make_vector_view(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, @@ -301,8 +295,7 @@ inline auto make_filtered_cell_metric_view(const SharedDataIndex &index, auto durations = make_vector_view(index, durations_block_id); auto distances = make_vector_view(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) @@ -321,8 +314,7 @@ inline auto make_cell_metric_view(const SharedDataIndex &index, const std::strin auto durations = make_vector_view(index, durations_block_id); auto distances = make_vector_view(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; @@ -342,14 +334,14 @@ inline auto make_multi_level_graph_view(const SharedDataIndex &index, const std: auto is_forward_edge = make_vector_view(index, name + "/is_forward_edge"); auto is_backward_edge = make_vector_view(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) diff --git a/src/contractor/contractor.cpp b/src/contractor/contractor.cpp index f151eeb5939..4caea102f03 100644 --- a/src/contractor/contractor.cpp +++ b/src/contractor/contractor.cpp @@ -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"; diff --git a/src/customize/customizer.cpp b/src/customize/customizer.cpp index da95f996797..0f5340f2c98 100644 --- a/src/customize/customizer.cpp +++ b/src/customize/customizer.cpp @@ -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; } diff --git a/src/extractor/edge_based_graph_factory.cpp b/src/extractor/edge_based_graph_factory.cpp index edef3d58951..b1f94a7b63b 100644 --- a/src/extractor/edge_based_graph_factory.cpp +++ b/src/extractor/edge_based_graph_factory.cpp @@ -71,7 +71,7 @@ EdgeBasedGraphFactory::EdgeBasedGraphFactory( const extractor::LaneDescriptionMap &lane_description_map) : m_edge_based_node_container(node_data_container), m_connectivity_checksum(0), m_number_of_edge_based_nodes(0), m_coordinates(coordinates), - m_node_based_graph(std::move(node_based_graph)), m_barrier_nodes(barrier_nodes), + m_node_based_graph(node_based_graph), m_barrier_nodes(barrier_nodes), m_traffic_lights(traffic_lights), m_compressed_edge_container(compressed_edge_container), name_table(name_table), segregated_edges(segregated_edges), lane_description_map(lane_description_map) diff --git a/src/extractor/extraction_containers.cpp b/src/extractor/extraction_containers.cpp index a587c15c40f..33e71ff3aed 100644 --- a/src/extractor/extraction_containers.cpp +++ b/src/extractor/extraction_containers.cpp @@ -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++; diff --git a/src/extractor/extractor_callbacks.cpp b/src/extractor/extractor_callbacks.cpp index 9ac6db0c67e..6ecfe83e5f2 100644 --- a/src/extractor/extractor_callbacks.cpp +++ b/src/extractor/extractor_callbacks.cpp @@ -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 { @@ -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, {})); }); } @@ -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, {})); }); } diff --git a/src/partitioner/inertial_flow.cpp b/src/partitioner/inertial_flow.cpp index 59a4aceb4bd..0d8aa425f13 100644 --- a/src/partitioner/inertial_flow.cpp +++ b/src/partitioner/inertial_flow.cpp @@ -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_} { } diff --git a/src/partitioner/recursive_bisection.cpp b/src/partitioner/recursive_bisection.cpp index 7c3ef98e3d3..77e57bc4039 100644 --- a/src/partitioner/recursive_bisection.cpp +++ b/src/partitioner/recursive_bisection.cpp @@ -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); diff --git a/src/util/coordinate_calculation.cpp b/src/util/coordinate_calculation.cpp index 4aa544b0296..fd055725613 100644 --- a/src/util/coordinate_calculation.cpp +++ b/src/util/coordinate_calculation.cpp @@ -341,7 +341,7 @@ Coordinate interpolateLinear(double factor, const Coordinate from, const Coordin FixedLatitude interpolated_lat{ static_cast(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 diff --git a/unit_tests/contractor/helper.hpp b/unit_tests/contractor/helper.hpp index e0dd159fd95..85adfa1fb23 100644 --- a/unit_tests/contractor/helper.hpp +++ b/unit_tests/contractor/helper.hpp @@ -36,7 +36,7 @@ inline contractor::ContractorGraph makeGraph(const std::vector &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 From 872e56a4e8b5141a024b9d55a33591928fde071c Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Sat, 20 Aug 2022 16:50:55 +0200 Subject: [PATCH 2/3] Enable performance-move-const-arg clang-tidy check --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a639194e61..314f7f9701c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - FIXED: Fix bug with reading Set values from Lua scripts. [#6285](https://github.com/Project-OSRM/osrm-backend/pull/6285) - 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) - Build: + - CHANGED: Enable performance-move-const-arg clang-tidy check. [#6319](https://github.com/Project-OSRM/osrm-backend/pull/6319) - 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) - CHANGED: Update libosmium to version 2.18.0. [#6303](https://github.com/Project-OSRM/osrm-backend/pull/6303) From f799dae7db9879ec7009bf85d041666ca7fa84a6 Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Sat, 20 Aug 2022 18:01:27 +0200 Subject: [PATCH 3/3] Enable performance-move-const-arg clang-tidy check --- src/engine/routing_algorithms/alternative_path_mld.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/engine/routing_algorithms/alternative_path_mld.cpp b/src/engine/routing_algorithms/alternative_path_mld.cpp index 51688c825e1..b8fae7568c1 100644 --- a/src/engine/routing_algorithms/alternative_path_mld.cpp +++ b/src/engine/routing_algorithms/alternative_path_mld.cpp @@ -851,7 +851,7 @@ InternalManyRoutesResult alternativePathSearch(SearchEngineData &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 weighted_packed_paths;