Skip to content
This repository has been archived by the owner on Jul 22, 2021. It is now read-only.

Commit

Permalink
Replace rmf_utils::optional with std::optional (#177)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mxgrey
mxgrey committed Sep 23, 2020
1 parent a029d21 commit 1ca2c4d
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 277 deletions.
1 change: 1 addition & 0 deletions rmf_traffic/CHANGELOG.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Forthcoming
* Fix an issue with moving robots between floors: [#163](https://github.com/osrf/rmf_core/pull/163/)
* Add a generic waiting event: [#158](https://github.com/osrf/rmf_core/pull/158)
* Fix bug that caused exit events to get skipped sometimes: [#166](https://github.com/osrf/rmf_core/pull/166)
* Bump to C++17 and migrate to `std::optional`: [#177](https://github.com/osrf/rmf_core/pull/177)

1.0.2 (2020-07-27)
------------------
Expand Down
2 changes: 1 addition & 1 deletion rmf_traffic/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ set(CMAKE_EXPORT_COMPILE_COMMANDS on)

# Default to C++14
if(NOT CMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD 17)
endif()

if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
Expand Down
2 changes: 1 addition & 1 deletion rmf_traffic/src/rmf_traffic/schedule/InconsistencyTracker.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ struct RangeLess
{
using Range = Inconsistencies::Ranges::Range;

bool operator()(const Range& lhs, const Range& rhs)
bool operator()(const Range& lhs, const Range& rhs) const
{
return rmf_utils::modular(lhs.upper).less_than(rhs.upper);
}
Expand Down
3 changes: 0 additions & 3 deletions rmf_traffic/test/unit/agv/test_Planner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2546,9 +2546,6 @@ SCENARIO("Test starts using graph with non-colinear waypoints")
Eigen::Vector2d{-4, 3.5};

Planner::Start start3{initial_time, 1, -M_PI_2, start_location1};
// Displaced 0.5m along lane 1
rmf_utils::optional<Eigen::Vector2d> start_location2 =
Eigen::Vector2d{-3.6, 2.7};
Planner::Start start4{initial_time, 1, -0.64, start_location1};

std::vector<Planner::Start> starts{start1, start2, start3, start4};
Expand Down
2 changes: 2 additions & 0 deletions rmf_utils/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ target_include_directories(rmf_utils
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
)

target_compile_features(rmf_utils INTERFACE cxx_std_17)

# Create cmake config files
include(CMakePackageConfigHelpers)

Expand Down
278 changes: 6 additions & 272 deletions rmf_utils/include/rmf_utils/optional.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,288 +18,22 @@
#ifndef RMF_UTILS__OPTIONAL_HPP
#define RMF_UTILS__OPTIONAL_HPP

#include <utility>
#include <stdexcept>
#include <optional>

namespace rmf_utils {

struct nullopt_t { };

inline constexpr nullopt_t nullopt{};

struct bad_optional_access : std::logic_error
{
bad_optional_access()
: std::logic_error("bad optional access")
{
// Do nothing
}
};

//==============================================================================
// TODO(MXG): Replace this with a std::optional when we're able to support C++17
// TODO(MXG): Consider removing the safety check for the dereference operators
template<typename T>
class optional
{
public:

optional()
: _has_value(false),
_storage(nullopt)
{
// Do nothing
}
optional(nullopt_t)
: _has_value(false),
_storage(nullopt)
{
// Do nothing
}
optional& operator=(nullopt_t)
{
if (_has_value)
_storage._value.~T();

_has_value = false;
_storage._null = nullopt;
return *this;
}

optional(const optional& other)
: _has_value(other._has_value),
_storage(other._has_value, other._storage)
{
// Do nothing
}

optional(optional&& other)
: _has_value(other._has_value),
_storage(other._has_value, std::move(other._storage))
{
// Do nothing
}

optional& operator=(const optional& other)
{
if (other._has_value)
{
*this = other._storage._value;
return *this;
}

if (_has_value)
{
// If the other does not have a value but this one does,
// then destruct the value contained in this object's storage.
_storage._value.~T();
}

_has_value = false;
_storage._null = nullopt;
return *this;
}

optional& operator=(optional&& other)
{
if (other._has_value)
{
*this = std::move(other._storage._value);
return *this;
}

if (_has_value)
{
// If the other does not have a value but this one does,
// then destruct the value contained in this object's storage.
_storage._value.~T();
}

_has_value = false;
_storage._null = nullopt;
return *this;
}

optional(const T& value)
: _has_value(true),
_storage(T(value))
{
// Do nothing
}
optional(T&& value)
: _has_value(true),
_storage(T(std::move(value)))
{
// Do nothing
}
optional& operator=(const T& value)
{
if (_has_value)
{
_storage._value = value;
}
else
{
new (&_storage._value) T(value);
}

_has_value = true;
return *this;
}
optional& operator=(T&& value)
{
if (_has_value)
{
_storage._value = std::move(value);
}
else
{
new (&_storage._value) T(std::move(value));
}

_has_value = true;
return *this;
}


bool has_value() const
{
return _has_value;
}
operator bool() const
{
return _has_value;
}

// Disable implicit conversion to integer types
operator int() const = delete;
operator std::size_t() const = delete;


T& value()
{
if (!_has_value)
throw bad_optional_access();

return _storage._value;
}
const T& value() const
{
if (!_has_value)
throw bad_optional_access();

return _storage._value;
}

T* operator->()
{
if (!_has_value)
throw bad_optional_access();

return &_storage._value;
}
const T* operator->() const
{
if (!_has_value)
throw bad_optional_access();

return &_storage._value;
}

T& operator*() &
{
if (!_has_value)
throw bad_optional_access();

return _storage._value;
}
const T& operator*() const&
{
if (!_has_value)
throw bad_optional_access();

return _storage._value;
}

T&& operator*() &&
{
if (!_has_value)
throw bad_optional_access();

return std::move(_storage._value);
}
const T&& operator*() const&&
{
if (!_has_value)
throw bad_optional_access();

return std::move(_storage._value);
}

~optional()
{
if(_has_value)
_storage._value.~T();
}

private:

bool _has_value;
union Storage {

Storage(nullopt_t)
: _null(nullopt)
{
// Do nothing
}

Storage(const T& other)
: _value(other)
{
// Do nothing
}

Storage(T&& other)
: _value(std::move(other))
{
// Do nothing
}

Storage(bool has_value, const Storage& other)
: _null(nullopt)
{
if (has_value)
{
new (&_value) T(other._value);
}
}

Storage(bool has_value, Storage&& other)
: _null(nullopt)
{
if (has_value)
{
new (&_value) T(std::move(other._value));
}
}

nullopt_t _null;
T _value;

~Storage() { }
} _storage;
template <typename T>
using optional = std::optional<T>;

};
inline constexpr std::nullopt_t nullopt{std::nullopt};

//==============================================================================
// Use this function to convert a potentially null pointer to an optional
template <typename T>
rmf_utils::optional<T> pointer_to_opt(const T* const ptr)
std::optional<T> pointer_to_opt(const T* const ptr)
{
if (ptr)
return *ptr;

return rmf_utils::nullopt;
return std::nullopt;
}

} // namespace rmf_utils
Expand Down

3 comments on commit 1ca2c4d

@Jason-Zhou1
Copy link

Choose a reason for hiding this comment

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

Hi friends,
I met some error when I colcon build:
error: could not convert ‘negotiation’ from ‘rmf_traffic::schedule::Negotiation’ to ‘rmf_utils::optional<rmf_traffic::schedule::Negotiation> {aka std::optional<rmf_traffic::schedule::Negotiation>}’

May I know how to solve it?

@codebot
Copy link
Member

Choose a reason for hiding this comment

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

This is not a good place to start a conversation about compiling errors. You're adding a comment to a commit. Please create a new issue ticket.

@Jason-Zhou1
Copy link

Choose a reason for hiding this comment

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

Ok, thank you for reminding!

Please sign in to comment.