C++ std::optional implementation

Question

Took a shot at implementing a subset of std::optional functionality. A lot of core features are there but some things like converting constructors, etc are missing as I just wanted to focus on the basic ideas.

The implementation:

#include <compare>
#include <typeinfo>
#include <utility>

struct BadOptionalAccess : std::exception {
    const char* what() const noexcept override {
        return "Tried to access an Optional()'s value, but no value exists!";
    }
};


template<typename T>
class Optional {
public:
    Optional() noexcept {
        std::memset(m_data, 0, sizeof(T));
    }

    Optional(const T& value) : 
        m_has_value{true} {
            new (m_data) T(value);
        }
    
    template<typename... Args>
    Optional(std::in_place_t, Args&&... args) {
        emplace(std::forward<Args>(args)...);
    }
    
    Optional(const Optional& other) : 
        m_has_value{other.m_has_value} {
            if (other.m_has_value) {
                new(m_data) T(other.value());
            }
        }
    
    Optional(Optional&& other) noexcept : 
        m_has_value{other.m_has_value} {
            if (other.has_value()) {
                new(m_data) T(std::move(other.value()));
                other.m_has_value = false;
            }
        }
    
    Optional& operator=(const Optional& other) {
        Optional temp {other};
        temp.swap(*this);
        return *this;
    }

    Optional& operator=(Optional&& other) noexcept {
        Optional temp {std::move(other)};
        temp.swap(*this);
        
        return *this;
    }

    auto operator<=>(const Optional& other) const noexcept {
        if (!has_value() && !other.has_value()) {
            return std::strong_ordering::equal;
        } else if (!has_value() && other.has_value()) {
            return std::strong_ordering::less;
        } else if (has_value() && !other.has_value()) {
            return std::strong_ordering::greater;
        } else {
            return value() <=> other.value();
        }
    }

    bool operator==(const Optional& other) const noexcept {
        return (*this <=> other) == std::strong_ordering::equal;
    }

    bool operator!=(const Optional& other) const noexcept {
        return !(*this == other);
    }

    explicit operator bool() const noexcept {
        return has_value();
    }
    
    template<typename... Args>
    void emplace(Args&&... args) {
        reset();
        new (m_data) T(std::forward<Args>(args)...);
        m_has_value = true;
    }

    template<typename U>
    T value_or(U&& default_value) {
        return has_value() ? std::move(**this) 
            : static_cast<T>(std::forward<U>(default_value));
    }

    void reset() {
        if (m_has_value) {
            T* val = reinterpret_cast<T*>(m_data);
            val->~T();
            m_has_value = false;
        }
    }

    void swap(Optional& other) noexcept {
        // Do I need to do this? My first instinct was to do
        // an ordinary swap of the m_data's, but it seemed wrong 
        // to do in case the types weren't trivially copyable/movable.
        // Maybe I should special case in those situations?
        if (other.has_value() && has_value()) {
            std::swap(*reinterpret_cast<T*>(m_data), *reinterpret_cast<T*>(other.m_data));
        } else if (other.has_value()) {
            new (m_data) T(std::move(other.value()));
        } else if (has_value()) {
            new(other.m_data) T(std::move(value()));
        }
        
        std::swap(m_has_value, other.m_has_value);
    }

    bool has_value() const noexcept {
        return m_has_value;
    }

    const T& value() const {
        return *ptr();
    }

    const T& operator*() const& {
        return value();
    }

    const T* operator->() const {
        return ptr();
    }

    T& value() {
        return *ptr();
    }

    T& operator*() & {
        return value();
    }

    T&& operator*() && noexcept {
        return std::move(value());
    }

    const T&& operator*() const&& noexcept {
        // not 100% sure how to use/test this overload. Why would an rvalue be const?
        return std::move(**this);
    }

    T* operator->() {
        return ptr();
    }

    ~Optional() {
        reset();
    }
        

private:
    alignas(T) unsigned char m_data[sizeof(T)];
    bool m_has_value = false;

    const T* ptr() const {
        if (!m_has_value) {
            throw BadOptionalAccess();
        }

        return (reinterpret_cast<const T*>(m_data));
    }

    T* ptr() {
        if (!m_has_value) {
            throw BadOptionalAccess();
        }

        return (reinterpret_cast<T*>(m_data));
    }
};

template<typename T, typename... Args>
Optional<T> makeOptional(Args&&... args) {
    return Optional<T>(std::in_place, std::forward<Args>(args)...);
}

Some tests:

#define BOOST_TEST_MODULE optionaltest

#ifdef BOOST_TEST_DYN_LINK
#include <boost/test/unit_test.hpp>
#else
#include <boost/test/included/unit_test.hpp>
#endif // BOOST_TEST_DYN_LINK

#include <boost/test/data/monomorphic.hpp>
#include <boost/test/data/test_case.hpp>

#include "Optional.h"
#include <string>



BOOST_AUTO_TEST_CASE(default_constructor_test) {
    Optional<int> o;
}

BOOST_AUTO_TEST_CASE(basic_constructors_test) {
     Optional<int> o1;
     Optional<int> o2 = 1;
     Optional<int> o3 = o2;
 
    // calls std::string( size_type count, CharT ch ) constructor
    Optional<std::string> o4(std::in_place, 3, 'A');
    Optional<std::string> o5 = makeOptional<std::string>(3, 'A');

    BOOST_CHECK(o4==o5);
 
    // Move-constructed from std::string using deduction guide to pick the type
    Optional o6(std::string{"deduction very long type"});
}

BOOST_AUTO_TEST_CASE(access_operator_test) {
    using namespace std::string_literals;

    Optional<int> opt1 = 1;
    BOOST_CHECK_EQUAL(*opt1, 1);
 
    *opt1 = 2;
    BOOST_CHECK_EQUAL(*opt1, 2);
 
    Optional<std::string> opt2 = "abc"s;
    BOOST_CHECK_EQUAL(*opt2, "abc"s);
    BOOST_CHECK_EQUAL(opt2->size(), 3); 
 
    Optional<std::string> taken = std::move(opt2);
    BOOST_CHECK_EQUAL(*taken, "abc"s); 
    BOOST_CHECK_EQUAL(taken->size(), 3); 

    BOOST_CHECK(!opt2.has_value()); 
}

BOOST_AUTO_TEST_CASE(value_check_test) {
    Optional<int> opt;
    BOOST_CHECK(!opt.has_value());
 
    opt = 43;
    BOOST_CHECK(opt.has_value());

    if (opt) {
        BOOST_CHECK(true);
    } else {
        BOOST_CHECK(false);
    }

    opt.reset();
    BOOST_CHECK(!opt.has_value());
}


BOOST_AUTO_TEST_CASE(get_value_test) {
    Optional<int> opt = {};

    BOOST_CHECK_THROW(opt.value(), BadOptionalAccess);
    BOOST_CHECK_THROW(opt.value() = 42, BadOptionalAccess);

    opt = 43;
    BOOST_CHECK_EQUAL(*opt, 43);
    BOOST_CHECK_EQUAL(opt.value(), 43);
    
    opt.value() = 44;
    BOOST_CHECK_EQUAL(*opt, 44);
    BOOST_CHECK_EQUAL(opt.value(), 44); 
}

BOOST_AUTO_TEST_CASE(ValueOrTest) {
    // Test with existing value
    Optional<int> opt1(5);
    BOOST_CHECK_EQUAL(opt1.value_or(10), 5);

    // Test without an existing value
    Optional<int> opt2; // Empty optional
    BOOST_CHECK_EQUAL(opt2.value_or(10), 10);

    // Test with a different type
    Optional<double> opt3; // Empty optional
    BOOST_CHECK_CLOSE(opt3.value_or(10), 10.0, 0.0001); // BOOST_CHECK_CLOSE for floating point comparison

    // Test with rvalue when Optional has a value
    Optional<std::string> opt4("Hello");
    BOOST_CHECK_EQUAL(opt4.value_or(std::string("World")), "Hello");

    // Test with rvalue when Optional does not have a value
    Optional<std::string> opt5; // Empty optional
    BOOST_CHECK_EQUAL(opt5.value_or(std::string("World")), "World");
}

BOOST_AUTO_TEST_CASE(swap_test) {
    Optional<std::string> opt1("Lorem ipsum dolor sit amet, consectetur tincidunt.");
    Optional<std::string> opt2("Some other lorem ipsum");

    opt1.swap(opt2);
    BOOST_CHECK_EQUAL(*opt2, "Lorem ipsum dolor sit amet, consectetur tincidunt.");
    BOOST_CHECK_EQUAL(*opt1, "Some other lorem ipsum");

    opt1.reset();
    opt1.swap(opt2);

    BOOST_CHECK_EQUAL(*opt1, "Lorem ipsum dolor sit amet, consectetur tincidunt.");
    BOOST_CHECK(!opt2.has_value());
}

BOOST_AUTO_TEST_CASE(comparison_test) {
    Optional<int> o1, o2;
    BOOST_CHECK(o1 == o2);

    o2.emplace(10);
    BOOST_CHECK(o1 < o2);
    BOOST_CHECK(o2 > o1);
    BOOST_CHECK(o2 != o1);

    o1.emplace(10);
    BOOST_CHECK(o1 == o2);
    BOOST_CHECK(o1 <= o2);
    BOOST_CHECK(o1 >= o2);
}
```

Answer 1

Missing #includes

You are missing #include <cstring> for std::memset(), and #include <new> for the placement new operator.

The converting constructors are a core feature

You have a non-converting constructor, but that does not even exist in std::optional. I would say that this is a core feature. Without it, you will get apparently inconsistent and surprising behavior. Consider:

Optional<int> foo = 3.1415;                  // works
Optional<float> bar = 42;                    // works
Optional<std::string> baz = "Hello, world!"; // fails to compile

The first two statements will happily cause conversions to happen. Fixing this so the third statement also compiles is trivial:

Optional(const auto& value) : 
    m_has_value{true} {
        new (m_data) T(value);
    }

I am very surprised by the choice of "basic features"; you have implemented operator<=>(), but I have never seen any code using the relational operators on std::optionals.

Unnecessary zeroing of memory in the default constructor

The default constructor does not need to zero m_data[], it's just going to cost you performance.

The move constructor should not unset other.m_has_value

In the move constructor, you set other.m_has_value to false, but you don't call the destructor of the other's value. And since the destructor of other will then not destruct its m_has_value, you will have created an object which will never be destructed propery.

You must leave other in a state such that everything will eventually be correctly destructed. However, as Igor G mentioned, std::optional's specification says that after a move construction, other.has_value() should not return a different value than before the move construction. So the best thing to do is to just not set other.m_has_value = false at all.

Incorrect behavior of swap()

Related to the above: you have a similar problem in swap(), in case you swap an optional which has a value with one which hasn't. Here you have to explicitly call reset() on the side which will no longer have a value after the swap.

About swapping

// Do I need to do this? My first instinct was to do
// an ordinary swap of the m_data's, but it seemed wrong 
// to do in case the types weren't trivially copyable/movable.
// Maybe I should special case in those situations?

You did the right thing in the code. You can't just swap the m_datas, that would just swap bytes without checking if that is legal, and would also bypass any specializations of std::swap for T. And in case T is trivially copyable/movable, then std::swap<T> would do exactly the same as std::swap<decltype<m_data>>, so there is no need to make this a special case.

Answer 2

Casting a storage to an object requires std::launder()

Since this question is tagged C++20, lines like this one:

return (reinterpret_cast<T*>(m_data));

should really be

return std::launder(reinterpret_cast<T*>(m_data));

Don't forget to set m_has_value every time you construct a new object.

Construction of T in m_data happens in many places, and sometimes it isn't accompanied by setting m_has_value to true. This kind of errors can be avoided if all construction was factored out to a single private function.

} else if (other.has_value()) {
    new (m_data) T(std::move(other.value()));
    // Hey, the object is there, but m_has_value is still false!
} else if ........

If moved-from optional contained some value before the move, it must still contain a value after the move

If an Optional contains a value, then every move should be performed on that contained value only.

Optional<int>      src = 42;
Optional<int>      dst(std::move(src));
assert( src.has_value() );

So, this line should be removed from constructor:

other.m_has_value = false;    // Wrong!

operator* and operator-> must be noexcept

Which means they cannot throw. Hence, they may not call neither value() nor ptr().

T& operator*() & {
    return value();   // That function may throw!
}

Don't underestimate SFINAE

Optional constructors and operators must participate in overload resolution only if certain conditions are met (read: they must be heavily loaded with SFINAE code checking those conditions). Getting all those conditions right isn't easy and requires lots of tests.

Answer 3

You have a lot of repeated, low level code. Create some private unsafe_ methods to avoid that repetition.

template<class...Ts>
void unsafe_emplace(Ts&&...ts);
void unsafe_destroy();
T* unsafe_get();
T const* unsafe_get() const;

these contain your reintepret_casts, std::launders, ::new( (void*)m_data ), ->~T()s.

They do absolutely no checking - that is what unsafe_ is for - and they are private.

Now your public methods:

void reset() {
  if (m_has_value) {
    unsafe_destroy();
  }
}

no longer fiddle around with the low-level operations.

The only "safe" thing the unsafe_ functions do is assign to m_has_value for destroy/emplace. They do not check the value before they operate.

And they work together:

void unsafe_destroy() {
  unsafe_get()->~T();
  m_has_value = false;
}

the reason I skip checking m_has_value is that reasonably often you'll know if it is true or not prior to calling these operations, and what to do when you get it wrong is either complex or nonsense.

Adding asserts is good however:

void unsafe_destroy() {
  assert(m_has_value);
  unsafe_get()->~T();
  m_has_value = false;
}

a second benefit is that when you are using placement new, you want to use ::new and cast its argument to void* to avoid the possibility that you hit an accidental manual override. By having your placement new code be in exactly one spot, fixing it in a bunch of spots stops being a problem.