Merge pull request #5561 from peoplestom/pessimistic_move

Removed un-needed calls to std::move

Author: gardster

.travis.yml

@@ -121,6 +121,14 @@ matrix:
           packages: ['libstdc++-4.9-dev']
       env: CLANG_VERSION='5.0.0' BUILD_TYPE='Release' ENABLE_MASON=ON RUN_CLANG_FORMAT=ON ENABLE_LTO=ON
 
+    - os: linux
+      compiler: "gcc-9-release"
+      addons: &gcc9
+        apt:
+          sources: ['ubuntu-toolchain-r-test']
+          packages: ['g++-9', 'libbz2-dev', 'libxml2-dev', 'libzip-dev', 'liblua5.2-dev', 'libtbb-dev', 'libboost-all-dev']
+      env: CCOMPILER='gcc-9' CXXCOMPILER='g++-9' BUILD_TYPE='Release' CXXFLAGS='-Wno-cast-function-type'
+
     - os: linux
       compiler: "gcc-8-release"
       addons: &gcc8

CHANGELOG.md

@@ -2,6 +2,7 @@
   - Changes from 5.21.0
     - Build:
       - ADDED: optionally build Node `lts` and `latest` bindings [#5347](https://github.com/Project-OSRM/osrm-backend/pull/5347)
+      - FIXED: pessimistic calls to std::move [#5560](https://github.com/Project-OSRM/osrm-backend/pull/5561)
     - Features:
       - ADDED: new waypoints parameter to the `route` plugin, enabling silent waypoints [#5345](https://github.com/Project-OSRM/osrm-backend/pull/5345)
       - ADDED: data timestamp information in the response (saved in new file `.osrm.timestamp`). [#5115](https://github.com/Project-OSRM/osrm-backend/issues/5115)

include/engine/api/base_api.hpp

@@ -12,6 +12,7 @@
 #include <boost/assert.hpp>
 #include <boost/range/algorithm/transform.hpp>
 
+#include <memory>
 #include <vector>
 
 namespace osrm
@@ -73,7 +74,7 @@ class BaseAPI
     }
 
     flatbuffers::Offset<flatbuffers::Vector<flatbuffers::Offset<fbresult::Waypoint>>>
-    MakeWaypoints(flatbuffers::FlatBufferBuilder &builder,
+    MakeWaypoints(flatbuffers::FlatBufferBuilder *builder,
                   const std::vector<PhantomNodes> &segment_end_coordinates) const
     {
         BOOST_ASSERT(parameters.coordinates.size() > 0);
@@ -82,43 +83,43 @@ class BaseAPI
         std::vector<flatbuffers::Offset<fbresult::Waypoint>> waypoints;
         waypoints.resize(parameters.coordinates.size());
         waypoints[0] =
-            MakeWaypoint(builder, segment_end_coordinates.front().source_phantom).Finish();
+            MakeWaypoint(builder, segment_end_coordinates.front().source_phantom)->Finish();
 
         std::transform(segment_end_coordinates.begin(),
                        segment_end_coordinates.end(),
                        std::next(waypoints.begin()),
-                       [this, &builder](const PhantomNodes &phantom_pair) {
-                           return MakeWaypoint(builder, phantom_pair.target_phantom).Finish();
+                       [this, builder](const PhantomNodes &phantom_pair) {
+                           return MakeWaypoint(builder, phantom_pair.target_phantom)->Finish();
                        });
-        return builder.CreateVector(waypoints);
+        return builder->CreateVector(waypoints);
     }
 
     // FIXME: gcc 4.9 does not like MakeWaypoints to be protected
     // protected:
-    fbresult::WaypointBuilder MakeWaypoint(flatbuffers::FlatBufferBuilder &builder,
-                                           const PhantomNode &phantom) const
+    std::unique_ptr<fbresult::WaypointBuilder> MakeWaypoint(flatbuffers::FlatBufferBuilder *builder,
+                                                            const PhantomNode &phantom) const
     {
 
         auto location =
             fbresult::Position(static_cast<double>(util::toFloating(phantom.location.lon)),
                                static_cast<double>(util::toFloating(phantom.location.lat)));
-        auto name_string = builder.CreateString(
+        auto name_string = builder->CreateString(
             facade.GetNameForID(facade.GetNameIndex(phantom.forward_segment_id.id)).to_string());
 
         boost::optional<flatbuffers::Offset<flatbuffers::String>> hint_string = boost::none;
         if (parameters.generate_hints)
         {
-            hint_string = builder.CreateString(Hint{phantom, facade.GetCheckSum()}.ToBase64());
+            hint_string = builder->CreateString(Hint{phantom, facade.GetCheckSum()}.ToBase64());
         }
 
-        fbresult::WaypointBuilder waypoint(builder);
-        waypoint.add_location(&location);
-        waypoint.add_distance(util::coordinate_calculation::fccApproximateDistance(
+        auto waypoint = std::make_unique<fbresult::WaypointBuilder>(*builder);
+        waypoint->add_location(&location);
+        waypoint->add_distance(util::coordinate_calculation::fccApproximateDistance(
             phantom.location, phantom.input_location));
-        waypoint.add_name(name_string);
+        waypoint->add_name(name_string);
         if (hint_string)
         {
-            waypoint.add_hint(*hint_string);
+            waypoint->add_hint(*hint_string);
         }
         return waypoint;
     }

include/engine/api/match_api.hpp

@@ -62,10 +62,10 @@ class MatchAPI final : public RouteAPI
 
         if (data_version_string)
         {
-            response.add_data_version(*data_version_string);
+            response->add_data_version(*data_version_string);
         }
 
-        fb_result.Finish(response.Finish());
+        fb_result.Finish(response->Finish());
     }
     void MakeResponse(const std::vector<map_matching::SubMatching> &sub_matchings,
                       const std::vector<InternalRouteResult> &sub_routes,
@@ -141,29 +141,29 @@ class MatchAPI final : public RouteAPI
             }
             const auto &phantom =
                 sub_matchings[matching_index.sub_matching_index].nodes[matching_index.point_index];
-            auto waypoint = BaseAPI::MakeWaypoint(fb_result, phantom);
-            waypoint.add_matchings_index(matching_index.sub_matching_index);
-            waypoint.add_alternatives_count(sub_matchings[matching_index.sub_matching_index]
-                                                .alternatives_count[matching_index.point_index]);
+            auto waypoint = BaseAPI::MakeWaypoint(&fb_result, phantom);
+            waypoint->add_matchings_index(matching_index.sub_matching_index);
+            waypoint->add_alternatives_count(sub_matchings[matching_index.sub_matching_index]
+                                                 .alternatives_count[matching_index.point_index]);
             // waypoint indices need to be adjusted if route legs were collapsed
             // waypoint parameter assumes there is only one match object
             if (!parameters.waypoints.empty())
             {
                 if (tidy_result.was_waypoint[trace_index])
                 {
-                    waypoint.add_waypoint_index(was_waypoint_idx);
+                    waypoint->add_waypoint_index(was_waypoint_idx);
                     was_waypoint_idx++;
                 }
                 else
                 {
-                    waypoint.add_waypoint_index(0);
+                    waypoint->add_waypoint_index(0);
                 }
             }
             else
             {
-                waypoint.add_waypoint_index(matching_index.point_index);
+                waypoint->add_waypoint_index(matching_index.point_index);
             }
-            waypoints.push_back(waypoint.Finish());
+            waypoints.push_back(waypoint->Finish());
         }
 
         return fb_result.CreateVector(waypoints);

include/engine/api/nearest_api.hpp

@@ -71,10 +71,9 @@ class NearestAPI final : public BaseAPI
                     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);

include/engine/api/route_api.hpp

@@ -81,14 +81,14 @@ class RouteAPI : public BaseAPI
 
         auto response =
             MakeFBResponse(raw_routes, fb_result, [this, &all_start_end_points, &fb_result]() {
-                return BaseAPI::MakeWaypoints(fb_result, all_start_end_points);
+                return BaseAPI::MakeWaypoints(&fb_result, all_start_end_points);
             });
 
         if (data_version_string)
         {
-            response.add_data_version(*data_version_string);
+            response->add_data_version(*data_version_string);
         }
-        fb_result.Finish(response.Finish());
+        fb_result.Finish(response->Finish());
     }
 
     void
@@ -125,9 +125,10 @@ class RouteAPI : public BaseAPI
 
   protected:
     template <typename GetWptsFn>
-    fbresult::FBResultBuilder MakeFBResponse(const InternalManyRoutesResult &raw_routes,
-                                             flatbuffers::FlatBufferBuilder &fb_result,
-                                             GetWptsFn getWaypoints) const
+    std::unique_ptr<fbresult::FBResultBuilder>
+    MakeFBResponse(const InternalManyRoutesResult &raw_routes,
+                   flatbuffers::FlatBufferBuilder &fb_result,
+                   GetWptsFn getWaypoints) const
     {
 
         std::vector<flatbuffers::Offset<fbresult::RouteObject>> routes;
@@ -151,9 +152,9 @@ class RouteAPI : public BaseAPI
             waypoints_vector = getWaypoints();
         }
 
-        fbresult::FBResultBuilder response(fb_result);
-        response.add_routes(routes_vector);
-        response.add_waypoints(waypoints_vector);
+        auto response = std::make_unique<fbresult::FBResultBuilder>(fb_result);
+        response->add_routes(routes_vector);
+        response->add_waypoints(waypoints_vector);
 
         return response;
     }
@@ -656,7 +657,6 @@ class RouteAPI : public BaseAPI
             step.intersections.end(),
             intersections.begin(),
             [&fb_result, this](const guidance::IntermediateIntersection &intersection) {
-
                 std::vector<flatbuffers::Offset<fbresult::Lane>> lanes;
                 if (json::detail::hasValidLanes(intersection))
                 {

include/engine/api/table_api.hpp

@@ -236,7 +236,7 @@ class TableAPI final : public BaseAPI
 
         boost::range::transform(
             phantoms, std::back_inserter(waypoints), [this, &builder](const PhantomNode &phantom) {
-                return BaseAPI::MakeWaypoint(builder, phantom).Finish();
+                return BaseAPI::MakeWaypoint(&builder, phantom)->Finish();
             });
         return builder.CreateVector(waypoints);
     }
@@ -252,7 +252,7 @@ class TableAPI final : public BaseAPI
                                 std::back_inserter(waypoints),
                                 [this, &builder, phantoms](const std::size_t idx) {
                                     BOOST_ASSERT(idx < phantoms.size());
-                                    return BaseAPI::MakeWaypoint(builder, phantoms[idx]).Finish();
+                                    return BaseAPI::MakeWaypoint(&builder, phantoms[idx])->Finish();
                                 });
         return builder.CreateVector(waypoints);
     }

include/engine/api/trip_api.hpp

@@ -61,9 +61,9 @@ class TripAPI final : public RouteAPI
 
         if (data_version_string)
         {
-            response.add_data_version(*data_version_string);
+            response->add_data_version(*data_version_string);
         }
-        fb_result.Finish(response.Finish());
+        fb_result.Finish(response->Finish());
     }
     void MakeResponse(const std::vector<std::vector<NodeID>> &sub_trips,
                       const std::vector<InternalRouteResult> &sub_routes,
@@ -127,10 +127,10 @@ class TripAPI final : public RouteAPI
             auto trip_index = input_idx_to_trip_idx[input_index];
             BOOST_ASSERT(!trip_index.NotUsed());
 
-            auto waypoint = BaseAPI::MakeWaypoint(fb_result, phantoms[input_index]);
-            waypoint.add_waypoint_index(trip_index.point_index);
-            waypoint.add_trips_index(trip_index.sub_trip_index);
-            waypoints.push_back(waypoint.Finish());
+            auto waypoint = BaseAPI::MakeWaypoint(&fb_result, phantoms[input_index]);
+            waypoint->add_waypoint_index(trip_index.point_index);
+            waypoint->add_trips_index(trip_index.sub_trip_index);
+            waypoints.push_back(waypoint->Finish());
         }
 
         return fb_result.CreateVector(waypoints);

include/updater/csv_file_parser.hpp

@@ -122,7 +122,7 @@ template <typename Key, typename Value> struct CSVFilesParser
 
             util::Log() << "Loaded " << filename << " with " << result.size() << "values";
 
-            return std::move(result);
+            return result;
         }
         catch (const boost::exception &e)
         {

src/server/api/parameters_parser.cpp

@@ -45,9 +45,8 @@ boost::optional<ParameterT> parseParameters(std::string::iterator &iter,
         const auto ok =
             boost::spirit::qi::parse(iter, end, grammar(boost::phoenix::ref(parameters)));
 
-        // return move(a.b) is needed to move b out of a and then return the rvalue by implicit move
         if (ok && iter == end)
-            return std::move(parameters);
+            return parameters;
     }
     catch (const qi::expectation_failure<It> &failure)
     {