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

Refactor: Replace parts of boost::optional with std::optional (#6551,… #6593

Closed
wants to merge 13 commits into from
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- NodeJS:
- CHANGED: Use node-api instead of NAN. [#6452](https://github.com/Project-OSRM/osrm-backend/pull/6452)
- Misc:
- FIXED: Partial fix migration from boost::optional to std::optional [#6551](https://github.com/Project-OSRM/osrm-backend/issues/6551), see also [#6592](https://github.com/Project-OSRM/osrm-backend/issues/6592)
- CHANGED: keep libosrm* in the docker image for downstream linking [#6602](https://github.com/Project-OSRM/osrm-backend/pull/6602)
- CHANGED: Move vector in CSVFilesParser instead copying it. [#6470](https://github.com/Project-OSRM/osrm-backend/pull/6470)
- REMOVED: Get rid of unused functions in util/json_util.hpp. [#6446](https://github.com/Project-OSRM/osrm-backend/pull/6446)
Expand All @@ -22,6 +23,7 @@
- FIXED: Typo in file name src/util/timed_historgram.cpp -> src/util/timed_histogram.cpp [#6428](https://github.com/Project-OSRM/osrm-backend/issues/6428)
- CHANGED: Replace boost::string_ref with std::string_view [#6433](https://github.com/Project-OSRM/osrm-backend/pull/6433)
- ADDED: Print tracebacks for Lua runtime errors [#6564](https://github.com/Project-OSRM/osrm-backend/pull/6564)
- FIXED: Added a variable to preprocessor guard in file osrm-backend/include/util/range_table.hpp to solve build error. [#6596](https://github.com/Project-OSRM/osrm-backend/pull/6596)
- Routing:
- FIXED: Fix adding traffic signal penalties during compression [#6419](https://github.com/Project-OSRM/osrm-backend/pull/6419)
# 5.27.1
Expand Down
5 changes: 2 additions & 3 deletions include/guidance/intersection_handler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@

#include <algorithm>
#include <cstddef>
#include <optional>
#include <utility>
#include <vector>

#include <boost/optional.hpp>

namespace osrm::guidance
{

Expand Down Expand Up @@ -129,7 +128,7 @@ class IntersectionHandler
// ^ via
//
// For this scenario returns intersection at `b` and `b`.
boost::optional<IntersectionHandler::IntersectionViewAndNode>
std::optional<IntersectionHandler::IntersectionViewAndNode>
getNextIntersection(const NodeID at, const EdgeID via) const;

bool isSameName(const EdgeID source_edge_id, const EdgeID target_edge_id) const;
Expand Down
9 changes: 4 additions & 5 deletions include/guidance/sliproad_handler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@

#include "util/node_based_graph.hpp"

#include <optional>
#include <vector>

#include <boost/optional.hpp>

namespace osrm::guidance
{

Expand Down Expand Up @@ -43,9 +42,9 @@ class SliproadHandler final : public IntersectionHandler
Intersection intersection) const override final;

private:
boost::optional<std::size_t> getObviousIndexWithSliproads(const EdgeID from,
const Intersection &intersection,
const NodeID at) const;
std::optional<std::size_t> getObviousIndexWithSliproads(const EdgeID from,
const Intersection &intersection,
const NodeID at) const;

// Next intersection from `start` onto `onto` is too far away for a Siproad scenario
bool nextIntersectionIsTooFarAway(const NodeID start, const EdgeID onto) const;
Expand Down
7 changes: 3 additions & 4 deletions include/guidance/turn_handler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@
#include "util/attributes.hpp"
#include "util/node_based_graph.hpp"

#include <boost/optional.hpp>

#include <cstddef>
#include <optional>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -72,7 +71,7 @@ class TurnHandler final : public IntersectionHandler

bool hasObvious(const EdgeID &via_edge, const Fork &fork) const;

boost::optional<Fork> findForkCandidatesByGeometry(Intersection &intersection) const;
std::optional<Fork> findForkCandidatesByGeometry(Intersection &intersection) const;

bool isCompatibleByRoadClass(const Intersection &intersection, const Fork fork) const;

Expand All @@ -96,7 +95,7 @@ class TurnHandler final : public IntersectionHandler
handleDistinctConflict(const EdgeID via_edge, ConnectedRoad &left, ConnectedRoad &right) const;

// Classification
boost::optional<Fork> findFork(const EdgeID via_edge, Intersection &intersection) const;
std::optional<Fork> findFork(const EdgeID via_edge, Intersection &intersection) const;

OSRM_ATTR_WARN_UNUSED
Intersection assignLeftTurns(const EdgeID via_edge,
Expand Down
8 changes: 4 additions & 4 deletions include/nodejs/node_osrm_support.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
#include <napi.h>

#include <boost/assert.hpp>
#include <boost/optional.hpp>

#include <algorithm>
#include <iostream>
#include <iterator>
#include <optional>
#include <sstream>
#include <stdexcept>
#include <string>
Expand Down Expand Up @@ -345,11 +345,11 @@ inline engine_config_ptr argumentsToEngineConfig(const Napi::CallbackInfo &args)
return engine_config;
}

inline boost::optional<std::vector<osrm::Coordinate>>
inline std::optional<std::vector<osrm::Coordinate>>
parseCoordinateArray(const Napi::Array &coordinates_array)
{
Napi::HandleScope scope(coordinates_array.Env());
boost::optional<std::vector<osrm::Coordinate>> resulting_coordinates;
std::optional<std::vector<osrm::Coordinate>> resulting_coordinates;
std::vector<osrm::Coordinate> temp_coordinates;

for (uint32_t i = 0; i < coordinates_array.Length(); ++i)
Expand Down Expand Up @@ -968,7 +968,7 @@ inline bool parseCommonParameters(const Napi::Object &obj, ParamType &params)

inline PluginParameters argumentsToPluginParameters(
const Napi::CallbackInfo &args,
const boost::optional<osrm::engine::api::BaseParameters::OutputFormatType> &output_format = {})
const std::optional<osrm::engine::api::BaseParameters::OutputFormatType> &output_format = {})
{
if (args.Length() < 3 || !args[1].IsObject())
{
Expand Down
8 changes: 4 additions & 4 deletions include/util/coordinate_calculation.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
#include "util/coordinate.hpp"

#include <boost/math/constants/constants.hpp>
#include <boost/optional.hpp>

#include <algorithm>
#include <cmath>
#include <numeric>
#include <optional>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -102,9 +102,9 @@ double bearing(const Coordinate first_coordinate, const Coordinate second_coordi
double computeAngle(const Coordinate first, const Coordinate second, const Coordinate third);

// find the center of a circle through three coordinates
boost::optional<Coordinate> circleCenter(const Coordinate first_coordinate,
const Coordinate second_coordinate,
const Coordinate third_coordinate);
std::optional<Coordinate> circleCenter(const Coordinate first_coordinate,
const Coordinate second_coordinate,
const Coordinate third_coordinate);

// find the radius of a circle through three coordinates
double circleRadius(const Coordinate first_coordinate,
Expand Down
11 changes: 5 additions & 6 deletions include/util/geojson_debug_policies.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#ifndef OSRM_GEOJSON_DEBUG_POLICIES
#define OSRM_GEOJSON_DEBUG_POLICIES

#include <optional>
#include <vector>

#include "extractor/query_node.hpp"
Expand All @@ -9,8 +10,6 @@
#include "util/node_based_graph.hpp"
#include "util/typedefs.hpp"

#include <boost/optional.hpp>

namespace osrm::util
{

Expand All @@ -20,7 +19,7 @@ struct NodeIdVectorToLineString

// converts a vector of node ids into a linestring geojson feature
util::json::Object operator()(const std::vector<NodeID> &node_ids,
const boost::optional<json::Object> &properties = {}) const;
const std::optional<json::Object> &properties = {}) const;

const std::vector<util::Coordinate> &node_coordinates;
};
Expand All @@ -29,7 +28,7 @@ struct CoordinateVectorToLineString
{
// converts a vector of node ids into a linestring geojson feature
util::json::Object operator()(const std::vector<util::Coordinate> &coordinates,
const boost::optional<json::Object> &properties = {}) const;
const std::optional<json::Object> &properties = {}) const;
};

struct NodeIdVectorToMultiPoint
Expand All @@ -38,7 +37,7 @@ struct NodeIdVectorToMultiPoint

// converts a vector of node ids into a linestring geojson feature
util::json::Object operator()(const std::vector<NodeID> &node_ids,
const boost::optional<json::Object> &properties = {}) const;
const std::optional<json::Object> &properties = {}) const;

const std::vector<util::Coordinate> &node_coordinates;
};
Expand All @@ -47,7 +46,7 @@ struct CoordinateVectorToMultiPoint
{
// converts a vector of node ids into a linestring geojson feature
util::json::Object operator()(const std::vector<util::Coordinate> &coordinates,
const boost::optional<json::Object> &properties = {}) const;
const std::optional<json::Object> &properties = {}) const;
};

} // namespace osrm::util
Expand Down
5 changes: 2 additions & 3 deletions include/util/geojson_debug_policy_toolkit.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@

#include <algorithm>
#include <iterator>

#include <boost/optional.hpp>
#include <optional>

namespace osrm::util
{
Expand Down Expand Up @@ -84,7 +83,7 @@ struct NodeIdToCoordinate

inline util::json::Object makeFeature(std::string type,
util::json::Array coordinates,
const boost::optional<util::json::Object> &properties = {})
const std::optional<util::json::Object> &properties = {})
{
util::json::Object result;
result.values["type"] = "Feature";
Expand Down
14 changes: 7 additions & 7 deletions include/util/query_heap.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@

#include <boost/assert.hpp>
#include <boost/heap/d_ary_heap.hpp>
#include <boost/optional.hpp>

#include <algorithm>
#include <limits>
#include <map>
#include <optional>
#include <unordered_map>
#include <vector>

Expand Down Expand Up @@ -289,26 +289,26 @@ class QueryHeap
return inserted_nodes[index].node == node;
}

boost::optional<HeapNode &> GetHeapNodeIfWasInserted(const NodeID node)
HeapNode *GetHeapNodeIfWasInserted(const NodeID node)
{
const auto index = node_index.peek_index(node);
if (index >= static_cast<decltype(index)>(inserted_nodes.size()) ||
inserted_nodes[index].node != node)
{
return {};
return nullptr;
}
return inserted_nodes[index];
return &inserted_nodes[index];
}

boost::optional<const HeapNode &> GetHeapNodeIfWasInserted(const NodeID node) const
const HeapNode *GetHeapNodeIfWasInserted(const NodeID node) const
{
const auto index = node_index.peek_index(node);
if (index >= static_cast<decltype(index)>(inserted_nodes.size()) ||
inserted_nodes[index].node != node)
{
return {};
return nullptr;
}
return inserted_nodes[index];
return &inserted_nodes[index];
}

NodeID Min() const
Expand Down
4 changes: 3 additions & 1 deletion include/util/range_table.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ template <unsigned BLOCK_SIZE, storage::Ownership Ownership> class RangeTable
unsigned last_length = 0;
unsigned lengths_prefix_sum = 0;
unsigned block_idx = 0;
unsigned block_counter = 0;
BlockT block;
#ifndef BOOST_ASSERT_IS_VOID
unsigned block_counter = 0;
Copy link
Member

@danpat danpat Apr 19, 2023

Choose a reason for hiding this comment

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

These changes were merged to master in another PR #6596 - they should not also be showing up in this PR. I suspect some merges have been incorrectly applied or lost on the branch. This is probably also why CI is failing. I suspect if you tidy this up, CI should pass.

Copy link
Contributor

@AlTimofeyev AlTimofeyev Apr 20, 2023

Choose a reason for hiding this comment

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

The source code on master branch has block_counter = 0 on line 85, meanwhile in this commit it is placed on line 84. Could that be what's causing CI to fail?

Copy link
Member

Choose a reason for hiding this comment

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

@mugr1x may be try to squash all your commits into single one? Or open another PR? Tbh for me it is not clear why it is happening - looks more like some GH bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AlTimofeyev Well yes. Idk that much about CI.
@SiarheiFedartsou I did so #6611. Should I close this one ?
I also tried to rewrite some parsers. Point here is that the ones that are easy to rewrite work fine with switching to std::optional but the /server core API (and /engine because it there are crosslinks) don't.
I think this would be considered a too breaking change (at least ~1k lines of code) of new parser-related code.
Do you think there is value in #6592 or should I close this too ?
Also off-topic, but did you receive my email regarding GSoC. I was asking for what you mentors would expect. (I submitted my second version a bit late, though in time. Hope that is not a problem.)

unsigned block_sum = 0;
#endif
for (const unsigned l : lengths)
Expand Down Expand Up @@ -109,7 +109,9 @@ template <unsigned BLOCK_SIZE, storage::Ownership Ownership> class RangeTable
if (BLOCK_SIZE == block_idx)
{
diff_blocks.push_back(block);
#ifndef BOOST_ASSERT_IS_VOID
block_counter++;
#endif
}

// we can only store strings with length 255
Expand Down
4 changes: 2 additions & 2 deletions include/util/timezones.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
#include <boost/filesystem/path.hpp>
#include <boost/geometry.hpp>
#include <boost/geometry/index/rtree.hpp>
#include <boost/optional.hpp>

#include <rapidjson/document.h>

#include <chrono>
#include <optional>

namespace osrm::updater
{
Expand All @@ -34,7 +34,7 @@ class Timezoner
Timezoner(const char geojson[], std::time_t utc_time_now);
Timezoner(const boost::filesystem::path &tz_shapes_filename, std::time_t utc_time_now);

boost::optional<struct tm> operator()(const point_t &point) const;
std::optional<struct tm> operator()(const point_t &point) const;

private:
void LoadLocalTimesRTree(rapidjson::Document &geojson, std::time_t utc_time);
Expand Down
9 changes: 4 additions & 5 deletions src/guidance/intersection_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ void IntersectionHandler::assignTrivialTurns(const EdgeID via_eid,
}
}

boost::optional<IntersectionHandler::IntersectionViewAndNode>
std::optional<IntersectionHandler::IntersectionViewAndNode>
IntersectionHandler::getNextIntersection(const NodeID at, const EdgeID via) const
{
// We use the intersection generator to jump over traffic signals, barriers. The intersection
Expand All @@ -449,7 +449,7 @@ IntersectionHandler::getNextIntersection(const NodeID at, const EdgeID via) cons
if (intersection_parameters.node == SPECIAL_NODEID ||
intersection_parameters.edge == SPECIAL_EDGEID)
{
return boost::none;
return {};
}

auto intersection = extractor::intersection::getConnectedRoads<false>(node_based_graph,
Expand All @@ -464,11 +464,10 @@ IntersectionHandler::getNextIntersection(const NodeID at, const EdgeID via) cons

if (intersection.size() <= 2 || intersection.isTrafficSignalOrBarrier())
{
return boost::none;
return {};
}

return boost::make_optional(
IntersectionViewAndNode{std::move(intersection), intersection_node});
return std::make_optional(IntersectionViewAndNode{std::move(intersection), intersection_node});
}

bool IntersectionHandler::isSameName(const EdgeID source_edge_id, const EdgeID target_edge_id) const
Expand Down
Loading