IVGCVSW-1975 : Remove boost::optional from the public interface
IVGCVSW-1964 : Optional to support passing references
Change-Id: I4bf8c264d1726aab7e417de5ffd7d04248332e54
diff --git a/include/armnn/Optional.hpp b/include/armnn/Optional.hpp
index 6fc207f..47afca8 100644
--- a/include/armnn/Optional.hpp
+++ b/include/armnn/Optional.hpp
@@ -5,45 +5,50 @@
#pragma once
#include "Exceptions.hpp"
+#include <type_traits>
+#include <cstring>
+
+#include <boost/optional.hpp>
+
+// Optional is a drop in replacement for std::optional until we migrate
+// to c++-17. Only a subset of the optional features are implemented that
+// we intend to use in ArmNN.
+
+// There are two distinct implementations here:
+//
+// 1, for normal constructable/destructable types and reference types
+// 2, for reference types
+
+// The std::optional features we support are:
+//
+// - has_value() and operator bool() to tell if the optional has a value
+// - value() returns a reference to the held object
+//
+
+// There is a deprecated and limited support for boost::optional in this class,
+// which will be removed in the 19.02 release.
namespace armnn
{
-// NOTE: the members of the Optional class don't follow the ArmNN
-// coding convention because the interface to be close to
-// the C++-17 interface so we can easily migrate to std::optional
-// later.
+// EmptyOptional is used to initialize the Optional class in case we want
+// to have default value for an Optional in a function declaration.
+struct EmptyOptional {};
-template <typename T>
-class Optional final
+
+// OptionalBase is the common functionality between reference and non-reference
+// optional types.
+class OptionalBase
{
public:
- Optional(T&& value)
- : m_HasValue{true}
- {
- new (m_Storage) T(value);
- }
-
- Optional(const T& value)
- : m_HasValue{true}
- {
- new (m_Storage) T(value);
- }
-
- Optional(const Optional& other)
- : m_HasValue{false}
- {
- *this = other;
- }
-
- Optional() noexcept
+ OptionalBase() noexcept
: m_HasValue{false}
{
}
- ~Optional()
+ bool has_value() const noexcept
{
- reset();
+ return m_HasValue;
}
operator bool() const noexcept
@@ -51,37 +56,101 @@
return has_value();
}
- Optional& operator=(T&& value)
+protected:
+ OptionalBase(bool hasValue) noexcept
+ : m_HasValue{hasValue}
+ {
+ }
+
+ bool m_HasValue;
+};
+
+//
+// The default implementation is the non-reference case. This
+// has an unsigned char array for storing the optional value which
+// is in-place constructed there.
+//
+template <bool IsReference, typename T>
+class OptionalReferenceSwitch : public OptionalBase
+{
+public:
+ using Base = OptionalBase;
+
+ OptionalReferenceSwitch() noexcept : Base{} {}
+ OptionalReferenceSwitch(EmptyOptional) noexcept : Base{} {}
+
+ OptionalReferenceSwitch(const T& value)
+ : Base{}
+ {
+ Construct(value);
+ }
+
+ OptionalReferenceSwitch(const OptionalReferenceSwitch& other)
+ : Base{}
+ {
+ *this = other;
+ }
+
+ // temporary support for limited conversion from boost
+ OptionalReferenceSwitch(const boost::optional<T>& other)
+ : Base{}
+ {
+ *this = other;
+ }
+
+ OptionalReferenceSwitch& operator=(const T& value)
{
reset();
- new (m_Storage) T(value);
- m_HasValue = true;
+ Construct(value);
return *this;
}
- Optional& operator=(const T& value)
- {
- reset();
- new(m_Storage) T(value);
- m_HasValue = true;
- return *this;
- }
-
- Optional& operator=(const Optional& other)
+ OptionalReferenceSwitch& operator=(const OptionalReferenceSwitch& other)
{
reset();
if (other.has_value())
{
- new (m_Storage) T(other.value());
- m_HasValue = true;
+ Construct(other.value());
}
return *this;
}
+ // temporary support for limited conversion from boost
+ OptionalReferenceSwitch& operator=(const boost::optional<T>& other)
+ {
+ reset();
+ if (other.is_initialized())
+ {
+ Construct(other.get());
+ }
+
+ return *this;
+ }
+
+ OptionalReferenceSwitch& operator=(EmptyOptional)
+ {
+ reset();
+ return *this;
+ }
+
+ ~OptionalReferenceSwitch()
+ {
+ reset();
+ }
+
+ void reset()
+ {
+ if (Base::has_value())
+ {
+ value().T::~T();
+ Base::m_HasValue = false;
+ }
+ }
+
const T& value() const
{
- if (!has_value())
+ if (!Base::has_value())
{
throw BadOptionalAccessException("Optional has no value");
}
@@ -92,7 +161,7 @@
T& value()
{
- if (!has_value())
+ if (!Base::has_value())
{
throw BadOptionalAccessException("Optional has no value");
}
@@ -101,23 +170,110 @@
return *valuePtr;
}
- bool has_value() const noexcept
+private:
+ void Construct(const T& value)
{
- return m_HasValue;
+ new (m_Storage) T(value);
+ m_HasValue = true;
+ }
+
+ alignas(alignof(T)) unsigned char m_Storage[sizeof(T)];
+};
+
+//
+// This is the special case for reference types. This holds a pointer
+// to the referenced type. This doesn't own the referenced memory and
+// it never calls delete on the pointer.
+//
+template <typename T>
+class OptionalReferenceSwitch<true, T> : public OptionalBase
+{
+public:
+ using Base = OptionalBase;
+ using NonRefT = typename std::remove_reference<T>::type;
+
+ OptionalReferenceSwitch() noexcept : Base{}, m_Storage{nullptr} {}
+ OptionalReferenceSwitch(EmptyOptional) noexcept : Base{}, m_Storage{nullptr} {}
+
+ OptionalReferenceSwitch(const OptionalReferenceSwitch& other) : Base{}
+ {
+ *this = other;
+ }
+
+ OptionalReferenceSwitch(T value)
+ : Base{true}
+ , m_Storage{&value}
+ {
+ }
+
+ OptionalReferenceSwitch& operator=(const T value)
+ {
+ m_Storage = &value;
+ Base::m_HasValue = true;
+ return *this;
+ }
+
+ OptionalReferenceSwitch& operator=(const OptionalReferenceSwitch& other)
+ {
+ m_Storage = other.m_Storage;
+ Base::m_HasValue = other.has_value();
+ return *this;
+ }
+
+ OptionalReferenceSwitch& operator=(EmptyOptional)
+ {
+ reset();
+ return *this;
+ }
+
+ ~OptionalReferenceSwitch()
+ {
+ reset();
}
void reset()
{
- if (has_value())
+ Base::m_HasValue = false;
+ m_Storage = nullptr;
+ }
+
+ const T value() const
+ {
+ if (!Base::has_value())
{
- value().T::~T();
- m_HasValue = false;
+ throw BadOptionalAccessException("Optional has no value");
}
+
+ return *m_Storage;
+ }
+
+ T value()
+ {
+ if (!Base::has_value())
+ {
+ throw BadOptionalAccessException("Optional has no value");
+ }
+
+ return *m_Storage;
}
private:
- alignas(alignof(T)) unsigned char m_Storage[sizeof(T)];
- bool m_HasValue;
+ NonRefT* m_Storage;
+};
+
+template <typename T>
+class Optional final : public OptionalReferenceSwitch<std::is_reference<T>::value, T>
+{
+public:
+ using BaseSwitch = OptionalReferenceSwitch<std::is_reference<T>::value, T>;
+
+ Optional(const T& value) : BaseSwitch{value} {}
+ Optional() noexcept : BaseSwitch{} {}
+ Optional(EmptyOptional empty) : BaseSwitch{empty} {}
+ Optional(const Optional& other) : BaseSwitch{other} {}
+
+ // temporary support for limited conversion from boost
+ Optional(const boost::optional<T>& other) : BaseSwitch{other} {}
};
}
diff --git a/src/armnn/test/OptionalTest.cpp b/src/armnn/test/OptionalTest.cpp
index 1b5aaa7..87fd156 100644
--- a/src/armnn/test/OptionalTest.cpp
+++ b/src/armnn/test/OptionalTest.cpp
@@ -5,8 +5,34 @@
#include <boost/test/unit_test.hpp>
#include <armnn/Optional.hpp>
+#include <boost/optional.hpp>
#include <string>
+namespace
+{
+
+void PassStringRef(armnn::Optional<std::string&> value)
+{
+}
+
+void PassStringRefWithDefault(armnn::Optional<std::string&> value = armnn::EmptyOptional())
+{
+}
+
+void BoostCompatibilityTester(const armnn::Optional<std::string>& optionalString,
+ bool hasValue,
+ const std::string& expectedValue)
+{
+ BOOST_TEST(optionalString.has_value() == hasValue);
+ if (hasValue)
+ {
+ BOOST_TEST(optionalString.value() == expectedValue);
+ }
+}
+
+}
+
+
BOOST_AUTO_TEST_SUITE(OptionalTests)
BOOST_AUTO_TEST_CASE(SimpleStringTests)
@@ -41,6 +67,62 @@
BOOST_TEST(optionalString3.value() == "Hello World");
}
+
+BOOST_AUTO_TEST_CASE(StringRefTests)
+{
+ armnn::Optional<std::string&> optionalStringRef{armnn::EmptyOptional()};
+ BOOST_TEST(optionalStringRef.has_value() == false);
+
+ PassStringRef(optionalStringRef);
+ PassStringRefWithDefault();
+
+ armnn::Optional<std::string&> optionalStringRef2 = optionalStringRef;
+
+ std::string helloWorld("Hello World");
+
+ std::string& helloWorldRef = helloWorld;
+ armnn::Optional<std::string&> optionalHelloRef = helloWorldRef;
+ BOOST_TEST(optionalHelloRef.has_value() == true);
+ BOOST_TEST(optionalHelloRef.value() == "Hello World");
+
+ armnn::Optional<std::string&> optionalHelloRef2 = helloWorld;
+ BOOST_TEST(optionalHelloRef2.has_value() == true);
+ BOOST_TEST(optionalHelloRef2.value() == "Hello World");
+
+ armnn::Optional<std::string&> optionalHelloRef3{helloWorldRef};
+ BOOST_TEST(optionalHelloRef3.has_value() == true);
+ BOOST_TEST(optionalHelloRef3.value() == "Hello World");
+
+ armnn::Optional<std::string&> optionalHelloRef4{helloWorld};
+ BOOST_TEST(optionalHelloRef4.has_value() == true);
+ BOOST_TEST(optionalHelloRef4.value() == "Hello World");
+
+ // modify through the optional reference
+ optionalHelloRef4.value().assign("Long Other String");
+ BOOST_TEST(helloWorld == "Long Other String");
+ BOOST_TEST(optionalHelloRef.value() == "Long Other String");
+ BOOST_TEST(optionalHelloRef2.value() == "Long Other String");
+ BOOST_TEST(optionalHelloRef3.value() == "Long Other String");
+}
+
+BOOST_AUTO_TEST_CASE(BoostCompatibilityTests)
+{
+ // sanity checks
+ BoostCompatibilityTester(armnn::Optional<std::string>(), false, "");
+ BoostCompatibilityTester(armnn::Optional<std::string>("Hello World"), true, "Hello World");
+
+ // the real thing is to see that we can pass a boost::optional in place
+ // of an ArmNN Optional
+ boost::optional<std::string> empty;
+ boost::optional<std::string> helloWorld("Hello World");
+
+ BoostCompatibilityTester(empty, false, "");
+ BoostCompatibilityTester(helloWorld, true, "Hello World");
+
+ BoostCompatibilityTester(boost::optional<std::string>(), false, "");
+ BoostCompatibilityTester(boost::optional<std::string>("Hello World"), true, "Hello World");
+}
+
BOOST_AUTO_TEST_CASE(SimpleIntTests)
{
const int intValue = 123;