LibIPC+Everywhere: Change IPC decoders to construct values in-place

Currently, the generated IPC decoders will default-construct the type to
be decoded, then pass that value by reference to the concrete decoder.
This, of course, requires that the type is default-constructible. This
was an issue for decoding Variants, which had to require the first type
in the Variant list is Empty, to ensure it is default constructible.

Further, this made it possible for values to become uninitialized in
user-defined decoders.

This patch makes the decoder interface such that the concrete decoders
themselves contruct the decoded type upon return from the decoder. To do
so, the default decoders in IPC::Decoder had to be moved to the IPC
namespace scope, as these decoders are now specializations instead of
overloaded methods (C++ requires specializations to be in a namespace
scope).

1 files changed, 124 insertions, 129 deletions

diff --git a/Userland/Libraries/LibIPC/Decoder.h b/Userland/Libraries/LibIPC/Decoder.h
index 1cc3902a58..ad636f1b8c 100644
--- a/Userland/Libraries/LibIPC/Decoder.h
+++ b/Userland/Libraries/LibIPC/Decoder.h
@@ -9,6 +9,7 @@
 #include <AK/Concepts.h>
 #include <AK/DeprecatedString.h>
 #include <AK/Forward.h>
+#include <AK/MemoryStream.h>
 #include <AK/NumericLimits.h>
 #include <AK/StdLibExtras.h>
 #include <AK/Try.h>
@@ -16,6 +17,7 @@
 #include <AK/Variant.h>
 #include <LibCore/SharedCircularQueue.h>
 #include <LibCore/Stream.h>
+#include <LibIPC/Concepts.h>
 #include <LibIPC/File.h>
 #include <LibIPC/Forward.h>
 #include <LibIPC/Message.h>
@@ -23,7 +25,7 @@
 namespace IPC {
 
 template<typename T>
-inline ErrorOr<void> decode(Decoder&, T&)
+inline ErrorOr<T> decode(Decoder&)
 {
     static_assert(DependentFalse<T>, "Base IPC::decoder() instantiated");
     VERIFY_NOT_REACHED();
@@ -37,151 +39,144 @@ public:
     {
     }
 
-    ErrorOr<void> decode(bool&);
-    ErrorOr<void> decode(u8&);
-    ErrorOr<void> decode(u16&);
-    ErrorOr<void> decode(unsigned&);
-    ErrorOr<void> decode(unsigned long&);
-    ErrorOr<void> decode(unsigned long long&);
-    ErrorOr<void> decode(i8&);
-    ErrorOr<void> decode(i16&);
-    ErrorOr<void> decode(i32&);
-    ErrorOr<void> decode(i64&);
-    ErrorOr<void> decode(float&);
-    ErrorOr<void> decode(double&);
-    ErrorOr<void> decode(DeprecatedString&);
-    ErrorOr<void> decode(ByteBuffer&);
-    ErrorOr<void> decode(JsonValue&);
-    ErrorOr<void> decode(URL&);
-    ErrorOr<void> decode(Dictionary&);
-    ErrorOr<void> decode(File&);
-    ErrorOr<void> decode(AK::Empty&);
-    template<typename K, typename V>
-    ErrorOr<void> decode(HashMap<K, V>& hashmap)
-    {
-        u32 size;
-        TRY(decode(size));
-        if (size > NumericLimits<i32>::max())
-            return Error::from_string_literal("IPC: Invalid HashMap size");
-
-        for (size_t i = 0; i < size; ++i) {
-            K key;
-            TRY(decode(key));
-            V value;
-            TRY(decode(value));
-            TRY(hashmap.try_set(move(key), move(value)));
-        }
-        return {};
-    }
+    template<typename T>
+    ErrorOr<T> decode();
 
-    template<typename K, typename V>
-    ErrorOr<void> decode(OrderedHashMap<K, V>& hashmap)
+    template<typename T>
+    ErrorOr<void> decode_into(T& value)
     {
-        u32 size;
-        TRY(decode(size));
-        if (size > NumericLimits<i32>::max())
-            return Error::from_string_literal("IPC: Invalid HashMap size");
-
-        for (size_t i = 0; i < size; ++i) {
-            K key;
-            TRY(decode(key));
-            V value;
-            TRY(decode(value));
-            TRY(hashmap.try_set(move(key), move(value)));
-        }
+        m_stream >> value;
+        TRY(m_stream.try_handle_any_error());
         return {};
     }
 
-    template<Enum T>
-    ErrorOr<void> decode(T& enum_value)
-    {
-        UnderlyingType<T> inner_value;
-        TRY(decode(inner_value));
-        enum_value = T(inner_value);
-        return {};
-    }
+    Core::Stream::LocalSocket& socket() { return m_socket; }
 
-    template<typename T>
-    ErrorOr<void> decode(T& value)
-    {
-        return IPC::decode(*this, value);
-    }
+private:
+    InputMemoryStream& m_stream;
+    Core::Stream::LocalSocket& m_socket;
+};
 
-    template<typename T>
-    ErrorOr<void> decode(Vector<T>& vector)
-    {
-        u64 size;
-        TRY(decode(size));
-        if (size > NumericLimits<i32>::max())
-            return Error::from_string_literal("IPC: Invalid Vector size");
-        VERIFY(vector.is_empty());
-        TRY(vector.try_ensure_capacity(size));
-        for (size_t i = 0; i < size; ++i) {
-            T value;
-            TRY(decode(value));
-            vector.template unchecked_append(move(value));
-        }
-        return {};
-    }
+template<Arithmetic T>
+ErrorOr<T> decode(Decoder& decoder)
+{
+    T value { 0 };
+    TRY(decoder.decode_into(value));
+    return value;
+}
 
-    template<typename T, size_t Size>
-    ErrorOr<void> decode(Core::SharedSingleProducerCircularQueue<T, Size>& queue)
-    {
-        // FIXME: We don't support decoding into valid queues.
-        VERIFY(!queue.is_valid());
+template<Enum T>
+ErrorOr<T> decode(Decoder& decoder)
+{
+    auto value = TRY(decoder.decode<UnderlyingType<T>>());
+    return static_cast<T>(value);
+}
 
-        IPC::File anon_file;
-        TRY(decode(anon_file));
-        queue = TRY((Core::SharedSingleProducerCircularQueue<T, Size>::try_create(anon_file.take_fd())));
-        return {};
-    }
+template<>
+ErrorOr<DeprecatedString> decode(Decoder&);
 
-    template<typename... VariantTypes>
-    ErrorOr<void> decode(Variant<VariantTypes...>& variant)
-    {
-        typename AK::Variant<VariantTypes...>::IndexType type_index;
-        TRY(decode(type_index));
-        if (type_index >= sizeof...(VariantTypes))
-            return Error::from_string_literal("IPC: Invalid variant index");
+template<>
+ErrorOr<ByteBuffer> decode(Decoder&);
 
-        TRY((decode_variant<0, sizeof...(VariantTypes), VariantTypes...>(type_index, variant)));
-        return {};
+template<>
+ErrorOr<JsonValue> decode(Decoder&);
+
+template<>
+ErrorOr<URL> decode(Decoder&);
+
+template<>
+ErrorOr<Dictionary> decode(Decoder&);
+
+template<>
+ErrorOr<File> decode(Decoder&);
+
+template<>
+ErrorOr<Empty> decode(Decoder&);
+
+template<Concepts::Vector T>
+ErrorOr<T> decode(Decoder& decoder)
+{
+    auto size = TRY(decoder.decode<u64>());
+    if (size > NumericLimits<i32>::max())
+        return Error::from_string_literal("IPC: Invalid Vector size");
+
+    T vector;
+    TRY(vector.try_ensure_capacity(size));
+
+    for (size_t i = 0; i < size; ++i) {
+        auto value = TRY(decoder.decode<typename T::ValueType>());
+        vector.template unchecked_append(move(value));
     }
 
-    template<typename T>
-    ErrorOr<void> decode(Optional<T>& optional)
-    {
-        bool has_value;
-        TRY(decode(has_value));
-        if (!has_value) {
-            optional = {};
-            return {};
-        }
-        T value;
-        TRY(decode(value));
-        optional = move(value);
-        return {};
+    return vector;
+}
+
+template<Concepts::HashMap T>
+ErrorOr<T> decode(Decoder& decoder)
+{
+    auto size = TRY(decoder.decode<u32>());
+    if (size > NumericLimits<i32>::max())
+        return Error::from_string_literal("IPC: Invalid HashMap size");
+
+    T hashmap;
+
+    for (size_t i = 0; i < size; ++i) {
+        auto key = TRY(decoder.decode<typename T::KeyType>());
+        auto value = TRY(decoder.decode<typename T::ValueType>());
+        TRY(hashmap.try_set(move(key), move(value)));
     }
 
-private:
-    template<size_t CurrentIndex, size_t Max, typename... VariantTypes>
-    ErrorOr<void> decode_variant(size_t index, Variant<VariantTypes...>& variant)
-    {
-        if constexpr (CurrentIndex < Max) {
-            if (index == CurrentIndex) {
-                typename TypeList<VariantTypes...>::template Type<CurrentIndex> element;
-                TRY(decode(element));
-                variant.set(move(element));
-                return {};
-            }
-            return decode_variant<CurrentIndex + 1, Max, VariantTypes...>(index, variant);
-        } else {
-            VERIFY_NOT_REACHED();
+    return hashmap;
+}
+
+template<Concepts::SharedSingleProducerCircularQueue T>
+ErrorOr<T> decode(Decoder& decoder)
+{
+    auto anon_file = TRY(decoder.decode<IPC::File>());
+    return T::try_create(anon_file.take_fd());
+}
+
+template<Concepts::Optional T>
+ErrorOr<T> decode(Decoder& decoder)
+{
+    if (auto has_value = TRY(decoder.decode<bool>()); !has_value)
+        return T {};
+    return T { TRY(decoder.decode<typename T::ValueType>()) };
+}
+
+namespace Detail {
+
+template<Concepts::Variant T, size_t Index = 0>
+ErrorOr<T> decode_variant(Decoder& decoder, size_t index)
+{
+    using ElementList = TypeList<T>;
+
+    if constexpr (Index < ElementList::size) {
+        if (index == Index) {
+            using ElementType = typename ElementList::template Type<Index>;
+            return T { TRY(decoder.decode<ElementType>()) };
         }
+
+        return decode_variant<T, Index + 1>(decoder, index);
+    } else {
+        VERIFY_NOT_REACHED();
     }
+}
 
-    InputMemoryStream& m_stream;
-    Core::Stream::LocalSocket& m_socket;
-};
+}
+
+template<Concepts::Variant T>
+ErrorOr<T> decode(Decoder& decoder)
+{
+    auto index = TRY(decoder.decode<typename T::IndexType>());
+    return Detail::decode_variant<T>(decoder, index);
+}
+
+// This must be last so that it knows about the above specializations.
+template<typename T>
+ErrorOr<T> Decoder::decode()
+{
+    return IPC::decode<T>(*this);
+}
 
 }