Centralize Storage-based vtable method definitions

7 files changed, 178 insertions, 161 deletions

diff --git a/include/mbgl/style/conversion.hpp b/include/mbgl/style/conversion.hpp
index dd24d0acf3..9bea2014ee 100644
--- a/include/mbgl/style/conversion.hpp
+++ b/include/mbgl/style/conversion.hpp
@@ -44,6 +44,45 @@ struct Storage : std::aligned_storage_t<16, 8> {
 
 struct Error { std::string message; };
 
+template <typename T>
+const T& cast(const Storage& storage) {
+    return reinterpret_cast<const T&>(storage);
+}
+
+template <typename T>
+T& cast(Storage& storage) {
+    return reinterpret_cast<T&>(storage);
+}
+
+template <typename T>
+void destroy (Storage& storage) {
+    const T value(std::move(cast<T>(storage)));
+    (void)value; // appease linter
+}
+
+template <typename T>
+void move(Storage&& src, Storage& dest) {
+    new (static_cast<void*>(&dest)) const T (std::move(cast<T>(src)));
+    destroy<T>(src);
+}
+
+template <typename T>
+struct ValueTraits {
+    static bool isUndefined(const T& value);
+    static bool isArray(const T& value);
+    static std::size_t arrayLength(const T& value);
+    static T arrayMember(const T& value, std::size_t i);
+    static bool isObject(const T& value);
+    static optional<T> objectMember(const T& value, const char * name);
+    static optional<Error> eachMember(const T& value, const std::function<optional<Error> (const std::string&, const T&)>& fn);
+    static optional<bool> toBool(const T& value);
+    static optional<float> toNumber(const T& value);
+    static optional<double> toDouble(const T& value);
+    static optional<std::string> toString(const T& value);
+    static optional<mbgl::Value> toValue(const T& value);
+    static optional<GeoJSON> toGeoJSON(const T& value, Error& error);
+};
+
 class Value {
 public:
     struct VTable {
@@ -82,6 +121,13 @@ public:
         vtable->move(std::move(v.storage), this->storage);
     }
 
+    template <typename T>
+    Value(const T value) : vtable(vtableForType<T>()) {
+        static_assert(sizeof(Storage) >= sizeof(T), "Storage must be large enough to hold value type");
+
+        new (static_cast<void*>(&storage)) const T (value);
+   }
+
     ~Value() {
         vtable->destroy(storage);
     }
@@ -150,6 +196,60 @@ public:
     }
 
 private:
+    template <typename T>
+    static VTable* vtableForType() {
+        using Traits = ValueTraits<T>;
+    
+        static Value::VTable vtable = {
+            move<T>,
+            destroy<T>,
+            [] (const Storage& s) {
+                return Traits::isUndefined(cast<T>(s));
+            },
+            [] (const Storage& s) {
+                return Traits::isArray(cast<T>(s));
+            },
+            [] (const Storage& s) {
+                return Traits::arrayLength(cast<T>(s));
+            },
+            [] (const Storage& s, std::size_t i) {
+                return Value(Traits::arrayMember(cast<T>(s), i));
+            },
+            [] (const Storage& s) {
+                return Traits::isObject(cast<T>(s));
+            },
+            [] (const Storage& s, const char * key) {
+                optional<T> member = Traits::objectMember(cast<T>(s), key);
+                if (member) return optional<Value>(*member);
+                return optional<Value>();
+            },
+            [] (const Storage& s, const std::function<optional<Error> (const std::string&, const Value&)>& fn) {
+                return Traits::eachMember(cast<T>(s), [&](const std::string& k, const T& v) {
+                    return fn(k, Value(v));
+                });
+            },
+            [] (const Storage& s) {
+                return Traits::toBool(cast<T>(s));
+            },
+            [] (const Storage& s) {
+                return Traits::toNumber(cast<T>(s));
+            },
+            [] (const Storage& s) {
+                return Traits::toDouble(cast<T>(s));
+            },
+            [] (const Storage& s) {
+                return Traits::toString(cast<T>(s));
+            },
+            []  (const Storage& s) {
+                return Traits::toValue(cast<T>(s));
+            },
+            [] (const Storage& s, Error& err) {
+                return Traits::toGeoJSON(cast<T>(s), err);
+            }
+        };
+        return &vtable;
+    }
+
     VTable* vtable;
     Storage storage;
 };
diff --git a/platform/node/src/node_conversion.cpp b/platform/node/src/node_conversion.cpp
index 3787cf1d57..3526041718 100644
--- a/platform/node/src/node_conversion.cpp
+++ b/platform/node/src/node_conversion.cpp
@@ -5,57 +5,41 @@ namespace mbgl {
 namespace style {
 namespace conversion {
 
-static const v8::Local<v8::Value>& cast(const Storage& storage) {
-    return *static_cast<const v8::Local<v8::Value>*>(static_cast<const void*>(&storage));
-}
-
-static v8::Local<v8::Value>& cast(Storage& storage) {
-    return *static_cast<v8::Local<v8::Value>*>(static_cast<void*>(&storage));
-}
-
-static void destroy(Storage& storage) {
-    const v8::Local<v8::Value> value(std::move(cast(storage)));
-    (void)value; // appease linter
-}
+using V8Value = v8::Local<v8::Value>;
 
-static void move(Storage&& src, Storage& dest) {
-    new (static_cast<void*>(&dest)) const v8::Local<v8::Value> (std::move(cast(src)));
-    destroy(src);
-}
-
-inline bool isUndefined(const Storage& storage) {
+template<> bool ValueTraits<V8Value>::isUndefined(const V8Value& value) {
     Nan::HandleScope scope;
-    const v8::Local<v8::Value>& value = cast(storage);
+    
     return value->IsUndefined() || value->IsNull();
 }
 
-inline bool isArray(const Storage& storage) {
+template<> bool ValueTraits<V8Value>::isArray(const V8Value& value) {
     Nan::HandleScope scope;
-    const v8::Local<v8::Value>& value = cast(storage);
+    
     return value->IsArray();
 }
 
-inline std::size_t arrayLength(const Storage& storage) {
+template<> std::size_t ValueTraits<V8Value>::arrayLength(const V8Value& value) {
     Nan::HandleScope scope;
-    const v8::Local<v8::Value>& value = cast(storage);
+    
     return value.As<v8::Array>()->Length();
 }
 
-inline Value arrayMember(const Storage& storage, std::size_t i) {
+template<> V8Value ValueTraits<V8Value>::arrayMember(const V8Value& value, std::size_t i) {
     Nan::EscapableHandleScope scope;
-    const v8::Local<v8::Value>& value = cast(storage);
-    return makeValue(scope.Escape(Nan::Get(value.As<v8::Array>(), i).ToLocalChecked()));
+    
+    return scope.Escape(Nan::Get(value.As<v8::Array>(), i).ToLocalChecked());
 }
 
-inline bool isObject(const Storage& storage) {
+template<> bool ValueTraits<V8Value>::isObject(const V8Value& value) {
     Nan::HandleScope scope;
-    const v8::Local<v8::Value>& value = cast(storage);
+    
     return value->IsObject() && !value->IsArray();
 }
 
-inline optional<Value> objectMember(const Storage& storage, const char * name) {
+template<> optional<V8Value> ValueTraits<V8Value>::objectMember(const V8Value& value, const char * name) {
     Nan::EscapableHandleScope scope;
-    const v8::Local<v8::Value>& value = cast(storage);
+    
     if (!Nan::Has(Nan::To<v8::Object>(value).ToLocalChecked(), Nan::New(name).ToLocalChecked()).FromJust()) {
         return {};
     }
@@ -63,17 +47,17 @@ inline optional<Value> objectMember(const Storage& storage, const char * name) {
     if (result.IsEmpty()) {
         return {};
     }
-    return makeValue(scope.Escape(result.ToLocalChecked()));
+    return {scope.Escape(result.ToLocalChecked())};
 }
 
-optional<Error> eachMember(const Storage& storage, const std::function<optional<Error> (const std::string&, const Value&)>& fn) {
+template<> optional<Error> ValueTraits<V8Value>::eachMember(const V8Value& value, const std::function<optional<Error> (const std::string&, const V8Value&)>& fn) {
     Nan::HandleScope scope;
-    const v8::Local<v8::Value>& value = cast(storage);
+    
     v8::Local<v8::Array> names = Nan::GetOwnPropertyNames(Nan::To<v8::Object>(value).ToLocalChecked()).ToLocalChecked();
     for (uint32_t i = 0; i < names->Length(); ++i) {
         v8::Local<v8::Value> k = Nan::Get(names, i).ToLocalChecked();
         v8::Local<v8::Value> v = Nan::Get(Nan::To<v8::Object>(value).ToLocalChecked(), k).ToLocalChecked();
-        optional<Error> result = fn(*Nan::Utf8String(k), makeValue(v));
+        optional<Error> result = fn(*Nan::Utf8String(k), v);
         if (result) {
             return result;
         }
@@ -81,44 +65,44 @@ optional<Error> eachMember(const Storage& storage, const std::function<optional<
     return {};
 }
 
-inline optional<bool> toBool(const Storage& storage) {
+template<> optional<bool> ValueTraits<V8Value>::toBool(const V8Value& value) {
     Nan::HandleScope scope;
-    const v8::Local<v8::Value>& value = cast(storage);
+    
     if (!value->IsBoolean()) {
         return {};
     }
     return value->BooleanValue();
 }
 
-inline optional<float> toNumber(const Storage& storage) {
+template<> optional<float> ValueTraits<V8Value>::toNumber(const V8Value& value) {
     Nan::HandleScope scope;
-    const v8::Local<v8::Value>& value = cast(storage);
+    
     if (!value->IsNumber()) {
         return {};
     }
     return value->NumberValue();
 }
 
-inline optional<double> toDouble(const Storage& storage) {
+template<> optional<double> ValueTraits<V8Value>::toDouble(const V8Value& value) {
     Nan::HandleScope scope;
-    const v8::Local<v8::Value>& value = cast(storage);
+    
     if (!value->IsNumber()) {
         return {};
     }
     return value->NumberValue();
 }
 
-inline optional<std::string> toString(const Storage& storage) {
+template<> optional<std::string> ValueTraits<V8Value>::toString(const V8Value& value) {
     Nan::HandleScope scope;
-    const v8::Local<v8::Value>& value = cast(storage);
+    
     if (!value->IsString()) {
         return {};
     }
     return std::string(*Nan::Utf8String(value));
 }
 
-inline optional<mbgl::Value> toValue(const Storage& storage) {
-    const v8::Local<v8::Value>& value = cast(storage);
+template<> optional<mbgl::Value> ValueTraits<V8Value>::toValue(const V8Value& value) {
+    
     if (value->IsFalse()) {
         return { false };
     } else if (value->IsTrue()) {
@@ -136,10 +120,10 @@ inline optional<mbgl::Value> toValue(const Storage& storage) {
     }
 }
 
-static optional<GeoJSON> toGeoJSON(const Storage& storage, Error& error) {
+template<> optional<GeoJSON> ValueTraits<V8Value>::toGeoJSON(const V8Value& value, Error& error) {
     try {
         Nan::JSON JSON;
-        const v8::Local<v8::Value>& value = cast(storage);
+        
         std::string string = *Nan::Utf8String(JSON.Stringify(value->ToObject()).ToLocalChecked());
         return parseGeoJSON(string, error);
     } catch (const std::exception& ex) {
@@ -148,30 +132,6 @@ static optional<GeoJSON> toGeoJSON(const Storage& storage, Error& error) {
     }
 }
 
-Value makeValue(const v8::Local<v8::Value> value) {
-    static Value::VTable vtable = {
-        move,
-        destroy,
-        isUndefined,
-        isArray,
-        arrayLength,
-        arrayMember,
-        isObject,
-        objectMember,
-        eachMember,
-        toBool,
-        toNumber,
-        toDouble,
-        toString,
-        toValue,
-        toGeoJSON
-    };
-
-    Storage storage;
-    new (static_cast<void*>(&storage)) const v8::Local<v8::Value> (value);
-    return Value(&vtable, std::move(storage));
-}
-
 } // namespace conversion
 } // namespace style
 } // namespace mbgl
diff --git a/platform/node/src/node_conversion.hpp b/platform/node/src/node_conversion.hpp
index fb3f7a7415..7f0a7ed39e 100644
--- a/platform/node/src/node_conversion.hpp
+++ b/platform/node/src/node_conversion.hpp
@@ -14,11 +14,9 @@ namespace mbgl {
 namespace style {
 namespace conversion {
 
-Value makeValue(const v8::Local<v8::Value>);
-
 template <class T, class...Args>
 optional<T> convert(const v8::Local<v8::Value>& value, Error& error, Args&&...args) {
-    return convert<T>(makeValue(value), error, std::forward<Args>(args)...);
+    return convert<T>(Value(value), error, std::forward<Args>(args)...);
 }
 
 
diff --git a/platform/node/src/node_map.cpp b/platform/node/src/node_map.cpp
index a09f8385e1..db86e8899c 100644
--- a/platform/node/src/node_map.cpp
+++ b/platform/node/src/node_map.cpp
@@ -698,7 +698,7 @@ void NodeMap::SetLayoutProperty(const Nan::FunctionCallbackInfo<v8::Value>& info
         return Nan::ThrowTypeError("Second argument must be a string");
     }
 
-    mbgl::optional<Error> error = setLayoutProperty(*layer, *Nan::Utf8String(info[1]), makeValue(info[2]));
+    mbgl::optional<Error> error = setLayoutProperty(*layer, *Nan::Utf8String(info[1]), Value(info[2]));
     if (error) {
         return Nan::ThrowTypeError(error->message.c_str());
     }
@@ -730,7 +730,7 @@ void NodeMap::SetPaintProperty(const Nan::FunctionCallbackInfo<v8::Value>& info)
         return Nan::ThrowTypeError("Second argument must be a string");
     }
 
-    mbgl::optional<Error> error = setPaintProperty(*layer, *Nan::Utf8String(info[1]), makeValue(info[2]));
+    mbgl::optional<Error> error = setPaintProperty(*layer, *Nan::Utf8String(info[1]), Value(info[2]));
     if (error) {
         return Nan::ThrowTypeError(error->message.c_str());
     }
diff --git a/src/mbgl/style/parser.cpp b/src/mbgl/style/parser.cpp
index 54e7e708e8..23901f5dc9 100644
--- a/src/mbgl/style/parser.cpp
+++ b/src/mbgl/style/parser.cpp
@@ -258,7 +258,7 @@ void Parser::parseLayer(const std::string& id, const JSValue& value, std::unique
         }
 
         layer = reference->cloneRef(id);
-        conversion::setPaintProperties(*layer, conversion::makeValue(&value));
+        conversion::setPaintProperties(*layer, conversion::Value(&value));
     } else {
         conversion::Error error;
         optional<std::unique_ptr<Layer>> converted = conversion::convert<std::unique_ptr<Layer>>(value, error);
diff --git a/src/mbgl/style/rapidjson_conversion.cpp b/src/mbgl/style/rapidjson_conversion.cpp
index 6bc939013f..a458b3b6f7 100644
--- a/src/mbgl/style/rapidjson_conversion.cpp
+++ b/src/mbgl/style/rapidjson_conversion.cpp
@@ -1,59 +1,47 @@
 #include <mbgl/style/rapidjson_conversion.hpp>
-
 #include <mapbox/geojson.hpp>
 #include <mapbox/geojson/rapidjson.hpp>
 
+
 namespace mbgl {
 namespace style {
 namespace conversion {
 
-static const JSValue& cast(const Storage& storage) {
-    return **static_cast<const JSValue* const*>(static_cast<const void*>(&storage));
-}
-
-static void destroy(Storage&) {
-    return;
-}
-
-static void move(Storage&& src, Storage& dest) {
-    new (static_cast<void*>(&dest)) JSValue* (std::move(*static_cast<JSValue**>(static_cast<void*>(&src))));
-    destroy(src);
-}
+using JSValuePointer = const JSValue*;
 
-static bool isUndefined(const Storage& storage) {
-    return cast(storage).IsNull();
+template<> bool ValueTraits<JSValuePointer>::isUndefined(const JSValuePointer& value) {
+    return value->IsNull();
 }
 
-static bool isArray(const Storage& storage) {
-    return cast(storage).IsArray();
+template<> bool ValueTraits<JSValuePointer>::isArray(const JSValuePointer& value) {
+    return value->IsArray();
 }
 
-static std::size_t arrayLength(const Storage& storage) {
-    return cast(storage).Size();
+template<> std::size_t ValueTraits<JSValuePointer>::arrayLength(const JSValuePointer& value) {
+    return value->Size();
 }
 
-static Value arrayMember(const Storage& storage, std::size_t i) {
-    return makeValue(&cast(storage)[rapidjson::SizeType(i)]);
+template<> JSValuePointer ValueTraits<JSValuePointer>::arrayMember(const JSValuePointer& value, std::size_t i) {
+    return &(*value)[rapidjson::SizeType(i)];
 }
 
-static bool isObject(const Storage& storage) {
-    return cast(storage).IsObject();
+template<> bool ValueTraits<JSValuePointer>::isObject(const JSValuePointer& value) {
+    return value->IsObject();
 }
 
-static optional<Value> objectMember(const Storage& storage, const char * name) {
-    const JSValue& value = cast(storage);
-    if (!value.HasMember(name)) {
-        return optional<Value>();
+template<> optional<JSValuePointer> ValueTraits<JSValuePointer>::objectMember(const JSValuePointer& value, const char * name) {
+    if (!value->HasMember(name)) {
+        return optional<JSValuePointer>();
     }
-    return makeValue(&value[name]);
+    const JSValuePointer& member = &(*value)[name];
+    return {member};
 }
 
-static optional<Error> eachMember(const Storage& storage, const std::function<optional<Error> (const std::string&, const Value&)>& fn) {
-    const JSValue& value = cast(storage);
-    assert(value.IsObject());
-    for (const auto& property : value.GetObject()) {
+template<> optional<Error> ValueTraits<JSValuePointer>::eachMember(const JSValuePointer& value, const std::function<optional<Error> (const std::string&, const JSValuePointer&)>& fn) {
+    assert(value->IsObject());
+    for (const auto& property : value->GetObject()) {
         optional<Error> result =
-            fn({ property.name.GetString(), property.name.GetStringLength() }, makeValue(&property.value));
+            fn({ property.name.GetString(), property.name.GetStringLength() }, &property.value);
         if (result) {
             return result;
         }
@@ -61,41 +49,36 @@ static optional<Error> eachMember(const Storage& storage, const std::function<op
     return {};
 }
 
-static optional<bool> toBool(const Storage& storage) {
-    const JSValue& value = cast(storage);
-    if (!value.IsBool()) {
+template<> optional<bool> ValueTraits<JSValuePointer>::toBool(const JSValuePointer& value) {
+    if (!value->IsBool()) {
         return {};
     }
-    return value.GetBool();
+    return value->GetBool();
 }
 
-static optional<float> toNumber(const Storage& storage) {
-    const JSValue& value = cast(storage);
-    if (!value.IsNumber()) {
+template<> optional<float> ValueTraits<JSValuePointer>::toNumber(const JSValuePointer& value) {
+    if (!value->IsNumber()) {
         return {};
     }
-    return value.GetDouble();
+    return value->GetDouble();
 }
 
-static optional<double> toDouble(const Storage& storage) {
-    const JSValue& value = cast(storage);
-    if (!value.IsNumber()) {
+template<> optional<double> ValueTraits<JSValuePointer>::toDouble(const JSValuePointer& value) {
+    if (!value->IsNumber()) {
         return {};
     }
-    return value.GetDouble();
+    return value->GetDouble();
 }
 
-static optional<std::string> toString(const Storage& storage) {
-    const JSValue& value = cast(storage);
-    if (!value.IsString()) {
+template<> optional<std::string> ValueTraits<JSValuePointer>::toString(const JSValuePointer& value) {
+    if (!value->IsString()) {
         return {};
     }
-    return {{ value.GetString(), value.GetStringLength() }};
+    return {{ value->GetString(), value->GetStringLength() }};
 }
 
-static optional<mbgl::Value> toValue(const Storage& storage) {
-    const JSValue& value = cast(storage);
-    switch (value.GetType()) {
+template<> optional<mbgl::Value> ValueTraits<JSValuePointer>::toValue(const JSValuePointer& value) {
+    switch (value->GetType()) {
         case rapidjson::kNullType:
         case rapidjson::kFalseType:
             return { false };
@@ -104,50 +87,27 @@ static optional<mbgl::Value> toValue(const Storage& storage) {
             return { true };
 
         case rapidjson::kStringType:
-            return { std::string { value.GetString(), value.GetStringLength() } };
+            return { std::string { value->GetString(), value->GetStringLength() } };
 
         case rapidjson::kNumberType:
-            if (value.IsUint64()) return { value.GetUint64() };
-            if (value.IsInt64()) return { value.GetInt64() };
-            return { value.GetDouble() };
+            if (value->IsUint64()) return { value->GetUint64() };
+            if (value->IsInt64()) return { value->GetInt64() };
+            return { value->GetDouble() };
 
         default:
             return {};
     }
 }
 
-static optional<GeoJSON> toGeoJSON(const Storage& storage, Error& error) {
+template<> optional<GeoJSON> ValueTraits<JSValuePointer>::toGeoJSON(const JSValuePointer& value, Error& error) {
     try {
-        return mapbox::geojson::convert(cast(storage));
+        return mapbox::geojson::convert(*value);
     } catch (const std::exception& ex) {
         error = { ex.what() };
         return {};
     }
 }
 
-Value makeValue(const JSValue* value) {
-    static Value::VTable vtable = {
-        move,
-        destroy,
-        isUndefined,
-        isArray,
-        arrayLength,
-        arrayMember,
-        isObject,
-        objectMember,
-        eachMember,
-        toBool,
-        toNumber,
-        toDouble,
-        toString,
-        toValue,
-        toGeoJSON
-    };
-
-    Storage storage;
-    new (static_cast<void*>(&storage)) const JSValue* (value);
-    return Value(&vtable, std::move(storage));
-}
 
 } // namespace conversion
 } // namespace style
diff --git a/src/mbgl/style/rapidjson_conversion.hpp b/src/mbgl/style/rapidjson_conversion.hpp
index 858a143012..082fd1c064 100644
--- a/src/mbgl/style/rapidjson_conversion.hpp
+++ b/src/mbgl/style/rapidjson_conversion.hpp
@@ -1,19 +1,18 @@
 #pragma once
 
-#include <mbgl/style/conversion.hpp>
 #include <mbgl/util/rapidjson.hpp>
+#include <mbgl/style/conversion.hpp>
 
 namespace mbgl {
 namespace style {
 namespace conversion {
 
-Value makeValue(const JSValue*);
-
 template <class T, class...Args>
 optional<T> convert(const JSValue& value, Error& error, Args&&...args) {
-    return convert<T>(makeValue(&value), error, std::forward<Args>(args)...);
+    return convert<T>(Value(&value), error, std::forward<Args>(args)...);
 }
 
 } // namespace conversion
 } // namespace style
 } // namespace mbgl
+