[SharedCache] Make MetadataSerializable::Load static

`MetadataSerializable` is updated to allow a subclass to optionally
specify the return type of `Load`. If not specified it defaults to the
subclass itself. `SharedCache` specifies `std::optional` to represent
the case where the serialized metadata is in a format or version it does
not recognize.

By making `Load` a static member function and having it return a new
object it becomes easier to reason about the state of objects when a
deserialization failure occurs. It can return `std::optional` to
indicate failure and there is no object left in an unknown state.

Prior to this, `Load` worked on an existing instance of an object. It
was unclear what state the object would be in if a deserialization
failure occurred (its original state prior to `Load`? some
partially-loaded state? a null state?). The failure itself also had to
be communicated out of band.

1 files changed, 15 insertions, 25 deletions

diff --git a/view/sharedcache/core/MetadataSerializable.hpp b/view/sharedcache/core/MetadataSerializable.hpp
index 7b44ccbd..91ab2ed2 100644
--- a/view/sharedcache/core/MetadataSerializable.hpp
+++ b/view/sharedcache/core/MetadataSerializable.hpp
@@ -34,6 +34,10 @@
  avoid that.
  * */
 
+#ifndef SHAREDCACHE_CORE_METADATASERIALIZABLE_HPP
+#define SHAREDCACHE_CORE_METADATASERIALIZABLE_HPP
+
+#include <cassert>
 #include "binaryninjaapi.h"
 #include "rapidjson/document.h"
 #include "rapidjson/stringbuffer.h"
@@ -41,9 +45,6 @@
 #include "../api/sharedcachecore.h"
 #include "view/macho/machoview.h"
 
-#ifndef SHAREDCACHE_CORE_METADATASERIALIZABLE_HPP
-#define SHAREDCACHE_CORE_METADATASERIALIZABLE_HPP
-
 namespace SharedCacheCore {
 
 #define MSS(name)						 context.store(#name, name)
@@ -51,7 +52,6 @@ namespace SharedCacheCore {
 #define MSS_SUBCLASS(name)		 		 Serialize(context, #name, name)
 #define MSL(name)						 name = context.load<decltype(name)>(#name)
 #define MSL_CAST(name, storedType, type) name = (type)context.load<storedType>(#name)
-#define MSL_SUBCLASS(name)				 Deserialize(context, #name, name)
 
 using namespace BinaryNinja;
 
@@ -83,44 +83,34 @@ struct DeserializationContext {
 	}
 };
 
-template <typename Derived>
-class MetadataSerializable
-{
+template <typename Derived, typename LoadResult = Derived>
+class MetadataSerializable {
 public:
-	std::string AsString() const
-	{
+	std::string AsString() const {
 		SerializationContext context;
 		Store(context);
 
 		return context.buffer.GetString();
 	}
 
-	void LoadFromString(const std::string& s)
-	{
+	static LoadResult LoadFromString(const std::string& s) {
 		DeserializationContext context;
-		context.doc.Parse(s.c_str());
-		AsDerived().Load(context);
+		[[maybe_unused]] rapidjson::ParseResult result = context.doc.Parse(s.c_str());
+		assert(result);
+		return Derived::Load(context);
 	}
 
-	void LoadFromValue(rapidjson::Value& s)
-	{
+	static LoadResult LoadFromValue(rapidjson::Value& s) {
 		DeserializationContext context;
 		context.doc.CopyFrom(s, context.doc.GetAllocator());
-		AsDerived().Load(context);
+		return Derived::Load(context);
 	}
 
-	Ref<Metadata> AsMetadata() {
+	Ref<Metadata> AsMetadata() const {
 		return new Metadata(AsString());
 	}
 
-	bool LoadFromMetadata(const Ref<Metadata>& meta)
-	{
-		if (!meta->IsString())
-			return false;
-		LoadFromString(meta->GetString());
-		return true;
-	}
-
+	template <typename... Args>
 	void Store(SerializationContext& context) const {
 		context.writer.StartObject();
 		AsDerived().Store(context);