Omittable: get rid of value()

Xcode 10 doesn't have it, and value() is not quite fitting
mostly-exceptionless Quotient anyway.

3 files changed, 25 insertions, 15 deletions

diff --git a/lib/connection.cpp b/lib/connection.cpp
index 47c643b0..998b45d1 100644
--- a/lib/connection.cpp
+++ b/lib/connection.cpp
@@ -1182,7 +1182,7 @@ Room* Connection::provideRoom(const QString& id, Omittable<JoinState> joinState)
     // TODO: This whole function is a strong case for a RoomManager class.
     Q_ASSERT_X(!id.isEmpty(), __FUNCTION__, "Empty room id");
 
-    // If joinState.omitted(), all joinState == comparisons below are false.
+    // If joinState is empty, all joinState == comparisons below are false.
     const auto roomKey = qMakePair(id, joinState == JoinState::Invite);
     auto* room = d->roomMap.value(roomKey, nullptr);
     if (room) {
@@ -1214,17 +1214,17 @@ Room* Connection::provideRoom(const QString& id, Omittable<JoinState> joinState)
     if (!joinState)
         return room;
 
-    if (joinState == JoinState::Invite) {
+    if (*joinState == JoinState::Invite) {
         // prev is either Leave or nullptr
         auto* prev = d->roomMap.value({ id, false }, nullptr);
         emit invitedRoom(room, prev);
     } else {
-        room->setJoinState(joinState.value());
+        room->setJoinState(*joinState);
         // Preempt the Invite room (if any) with a room in Join/Leave state.
         auto* prevInvite = d->roomMap.take({ id, true });
-        if (joinState == JoinState::Join)
+        if (*joinState == JoinState::Join)
             emit joinedRoom(room, prevInvite);
-        else if (joinState == JoinState::Leave)
+        else if (*joinState == JoinState::Leave)
             emit leftRoom(room, prevInvite);
         if (prevInvite) {
             const auto dcUsers = prevInvite->directChatUsers();
diff --git a/lib/room.cpp b/lib/room.cpp
index b34c36ea..0806e30d 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -1208,7 +1208,7 @@ int Room::invitedCount() const
 {
     // TODO: Store invited users in Room too
     Q_ASSERT(d->summary.invitedMemberCount.has_value());
-    return d->summary.invitedMemberCount.value();
+    return d->summary.invitedMemberCount.value_or(0);
 }
 
 int Room::totalMemberCount() const { return joinedCount() + invitedCount(); }
diff --git a/lib/util.h b/lib/util.h
index b7bb8d4c..15c9fec8 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -56,6 +56,7 @@ class Omittable : public std::optional<T> {
                   "You cannot make an Omittable<> with a reference type");
 
 public:
+    using base_type = std::optional<T>;
     using value_type = std::decay_t<T>;
     static_assert(std::is_default_constructible_v<value_type>,
                   "Omittable<> requires a default-constructible type");
@@ -64,13 +65,22 @@ public:
 
     // Overload emplace() to allow passing braced-init-lists (the standard
     // emplace() does direct-initialisation but not direct-list-initialisation).
-    using std::optional<T>::emplace;
-    T& emplace(const T& val) { return std::optional<T>::emplace(val); }
-    T& emplace(T&& val) { return std::optional<T>::emplace(std::move(val)); }
-
+    using base_type::emplace;
+    T& emplace(const T& val) { return base_type::emplace(val); }
+    T& emplace(T&& val) { return base_type::emplace(std::move(val)); }
+
+    // use value_or() or check (with operator! or has_value) before accessing
+    // with operator-> or operator*
+    // The technical reason is that Xcode 10 has incomplete std::optional
+    // that has no value(); but using value() may also mean that you rely
+    // on the optional throwing an exception (which is not assumed practice
+    // throughout Quotient) or that you spend unnecessary CPU cycles on
+    // an extraneous has_value() check.
+    value_type& value() = delete;
+    const value_type& value() const = delete;
     value_type& edit()
     {
-        return this->has_value() ? this->value() : this->emplace();
+        return this->has_value() ? base_type::operator*() : this->emplace();
     }
 
     /// Merge the value from another Omittable
@@ -92,10 +102,10 @@ public:
     // a bit too surprising: value() & doesn't lazy-create an object;
     // and it's too easy to inadvertently change the underlying value.
 
-    const value_type* operator->() const& { return &this->value(); }
-    value_type* operator->() && { return &this->value(); }
-    const value_type& operator*() const& { return this->value(); }
-    value_type& operator*() && { return this->value(); }
+    const value_type* operator->() const& { return base_type::operator->(); }
+    value_type* operator->() && { return base_type::operator->(); }
+    const value_type& operator*() const& { return base_type::operator*(); }
+    value_type& operator*() && { return base_type::operator*(); }
 };
 
 namespace _impl {