IVGCVSW-1975 : Remove boost::optional from the public interface

IVGCVSW-1964 : Optional to support passing references

Change-Id: I4bf8c264d1726aab7e417de5ffd7d04248332e54

2 files changed, 278 insertions, 40 deletions

diff --git a/include/armnn/Optional.hpp b/include/armnn/Optional.hpp
index 6fc207f425..47afca87b5 100644
--- a/include/armnn/Optional.hpp
+++ b/include/armnn/Optional.hpp
@@ -5,83 +5,152 @@
 #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}
+    OptionalBase() noexcept
+        : m_HasValue{false}
     {
-        new (m_Storage) T(value);
     }
 
-    Optional(const T& value)
-        : m_HasValue{true}
+    bool has_value() const noexcept
     {
-        new (m_Storage) T(value);
+        return m_HasValue;
     }
 
-    Optional(const Optional& other)
-        : m_HasValue{false}
+    operator bool() const noexcept
     {
-        *this = other;
+        return has_value();
     }
 
-    Optional() noexcept
-        : m_HasValue{false}
+protected:
+    OptionalBase(bool hasValue) noexcept
+        : m_HasValue{hasValue}
     {
     }
 
-    ~Optional()
+    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{}
     {
-        reset();
+        Construct(value);
     }
 
-    operator bool() const noexcept
+    OptionalReferenceSwitch(const OptionalReferenceSwitch& other)
+        : Base{}
     {
-        return has_value();
+        *this = other;
+    }
+
+    // temporary support for limited conversion from boost
+    OptionalReferenceSwitch(const boost::optional<T>& other)
+        : Base{}
+    {
+        *this = other;
     }
 
-    Optional& operator=(T&& value)
+    OptionalReferenceSwitch& operator=(const T& value)
     {
         reset();
-        new (m_Storage) T(value);
-        m_HasValue = true;
+        Construct(value);
         return *this;
     }
 
-    Optional& operator=(const T& value)
+    OptionalReferenceSwitch& operator=(const OptionalReferenceSwitch& other)
     {
         reset();
-        new(m_Storage) T(value);
-        m_HasValue = true;
+        if (other.has_value())
+        {
+            Construct(other.value());
+        }
+
         return *this;
     }
 
-    Optional& operator=(const Optional& other)
+    // temporary support for limited conversion from boost
+    OptionalReferenceSwitch& operator=(const boost::optional<T>& other)
     {
         reset();
-        if (other.has_value())
+        if (other.is_initialized())
         {
-            new (m_Storage) T(other.value());
-            m_HasValue = true;
+            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 @@ public:
 
     T& value()
     {
-        if (!has_value())
+        if (!Base::has_value())
         {
             throw BadOptionalAccessException("Optional has no value");
         }
@@ -101,23 +170,110 @@ public:
         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 1b5aaa7db6..87fd156ece 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_AUTO_TEST_CASE(SimpleStringTests)
     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;