[all] Replace Result<T> with optional<T> plus out Error parameter

benchmark/parse/filter.benchmark.cpp

@@ -14,7 +14,8 @@ using namespace mbgl;
 style::Filter parse(const char* expression) {
     rapidjson::GenericDocument<rapidjson::UTF8<>, rapidjson::CrtAllocator> doc;
     doc.Parse<0>(expression);
-    return *style::conversion::convert<style::Filter, JSValue>(doc);
+    style::conversion::Error error;
+    return *style::conversion::convert<style::Filter, JSValue>(doc, error);
 }
 
 static void Parse_Filter(benchmark::State& state) {

cmake/core-files.cmake

@@ -276,8 +276,6 @@ set(MBGL_CORE_FILES
     src/mbgl/style/possibly_evaluated_property_value.hpp
     src/mbgl/style/property_evaluation_parameters.hpp
     src/mbgl/style/property_evaluator.hpp
-    src/mbgl/style/property_parsing.cpp
-    src/mbgl/style/property_parsing.hpp
     src/mbgl/style/rapidjson_conversion.hpp
     src/mbgl/style/source.cpp
     src/mbgl/style/source_impl.cpp

include/mbgl/style/conversion.hpp

@@ -1,6 +1,6 @@
 #pragma once
 
-#include <mbgl/util/variant.hpp>
+#include <mbgl/util/optional.hpp>
 
 #include <string>
 
@@ -21,11 +21,11 @@ namespace conversion {
    A single template function serves as the public interface:
 
        template <class T, class V>
-       Result<T> convert(const V& value);
+       optional<T> convert(const V& value, Error& error);
 
-   Where `T` is one of the above types. If the conversion fails, the `Error` variant of `Result` is
-   returned, which includes diagnostic text suitable for presentation to a library user. Otherwise,
-   the `T` variant of `Result` is returned.
+   Where `T` is one of the above types. If the conversion fails, the result is empty, and the
+   error parameter includes diagnostic text suitable for presentation to a library user. Otherwise,
+   a filled optional is returned.
 
    The implementation of `convert` requires that the following are legal expressions for a value `v`
    of type `const V&`:
@@ -57,37 +57,12 @@ namespace conversion {
 
 struct Error { std::string message; };
 
-template <class T>
-class Result : private variant<T, Error> {
-public:
-    using variant<T, Error>::variant;
-
-    explicit operator bool() const {
-        return this->template is<T>();
-    }
-
-    T& operator*() {
-        assert(this->template is<T>());
-        return this->template get<T>();
-    }
-
-    const T& operator*() const {
-        assert(this->template is<T>());
-        return this->template get<T>();
-    }
-
-    const Error& error() const {
-        assert(this->template is<Error>());
-        return this->template get<Error>();
-    }
-};
-
 template <class T, class Enable = void>
 struct Converter;
 
 template <class T, class V, class...Args>
-Result<T> convert(const V& value, Args&&...args) {
-    return Converter<T>()(value, std::forward<Args>(args)...);
+optional<T> convert(const V& value, Error& error, Args&&...args) {
+    return Converter<T>()(value, error, std::forward<Args>(args)...);
 }
 
 } // namespace conversion

include/mbgl/style/conversion/constant.hpp

@@ -16,10 +16,11 @@ namespace conversion {
 template <>
 struct Converter<bool> {
     template <class V>
-    Result<bool> operator()(const V& value) const {
+    optional<bool> operator()(const V& value, Error& error) const {
         optional<bool> converted = toBool(value);
         if (!converted) {
-            return Error { "value must be a boolean" };
+            error = { "value must be a boolean" };
+            return {};
         }
         return *converted;
     }
@@ -28,10 +29,11 @@ struct Converter<bool> {
 template <>
 struct Converter<float> {
     template <class V>
-    Result<float> operator()(const V& value) const {
+    optional<float> operator()(const V& value, Error& error) const {
         optional<float> converted = toNumber(value);
         if (!converted) {
-            return Error { "value must be a number" };
+            error = { "value must be a number" };
+            return {};
         }
         return *converted;
     }
@@ -40,10 +42,11 @@ struct Converter<float> {
 template <>
 struct Converter<std::string> {
     template <class V>
-    Result<std::string> operator()(const V& value) const {
+    optional<std::string> operator()(const V& value, Error& error) const {
         optional<std::string> converted = toString(value);
         if (!converted) {
-            return Error { "value must be a string" };
+            error = { "value must be a string" };
+            return {};
         }
         return *converted;
     }
@@ -52,15 +55,17 @@ struct Converter<std::string> {
 template <class T>
 struct Converter<T, typename std::enable_if_t<std::is_enum<T>::value>> {
     template <class V>
-    Result<T> operator()(const V& value) const {
+    optional<T> operator()(const V& value, Error& error) const {
         optional<std::string> string = toString(value);
         if (!string) {
-            return Error { "value must be a string" };
+            error = { "value must be a string" };
+            return {};
         }
 
         const auto result = Enum<T>::toEnum(*string);
         if (!result) {
-            return Error { "value must be a valid enumeration value" };
+            error = { "value must be a valid enumeration value" };
+            return {};
         }
 
         return *result;
@@ -70,15 +75,17 @@ struct Converter<T, typename std::enable_if_t<std::is_enum<T>::value>> {
 template <>
 struct Converter<Color> {
     template <class V>
-    Result<Color> operator()(const V& value) const {
+    optional<Color> operator()(const V& value, Error& error) const {
         optional<std::string> string = toString(value);
         if (!string) {
-            return Error { "value must be a string" };
+            error = { "value must be a string" };
+            return {};
         }
 
         optional<Color> color = Color::parse(*string);
         if (!color) {
-            return Error { "value must be a valid color" };
+            error = { "value must be a valid color" };
+            return {};
         }
 
         return *color;
@@ -88,15 +95,17 @@ struct Converter<Color> {
 template <>
 struct Converter<std::array<float, 2>> {
     template <class V>
-    Result<std::array<float, 2>> operator()(const V& value) const {
+    optional<std::array<float, 2>> operator()(const V& value, Error& error) const {
         if (!isArray(value) || arrayLength(value) != 2) {
-            return Error { "value must be an array of two numbers" };
+            error = { "value must be an array of two numbers" };
+            return {};
         }
 
         optional<float> first = toNumber(arrayMember(value, 0));
         optional<float> second = toNumber(arrayMember(value, 1));
         if (!first || !second) {
-            return Error { "value must be an array of two numbers" };
+            error = { "value must be an array of two numbers" };
+            return {};
         }
 
         return std::array<float, 2> {{ *first, *second }};
@@ -106,17 +115,19 @@ struct Converter<std::array<float, 2>> {
 template <>
 struct Converter<std::array<float, 4>> {
     template <class V>
-    Result<std::array<float, 4>> operator()(const V& value) const {
+    optional<std::array<float, 4>> operator()(const V& value, Error& error) const {
         if (!isArray(value) || arrayLength(value) != 4) {
-            return Error { "value must be an array of four numbers" };
+            error = { "value must be an array of four numbers" };
+            return {};
         }
 
         optional<float> first = toNumber(arrayMember(value, 0));
         optional<float> second = toNumber(arrayMember(value, 1));
         optional<float> third = toNumber(arrayMember(value, 2));
         optional<float> fourth = toNumber(arrayMember(value, 3));
         if (!first || !second) {
-            return Error { "value must be an array of four numbers" };
+            error = { "value must be an array of four numbers" };
+            return {};
         }
 
         return std::array<float, 4> {{ *first, *second, *third, *fourth }};
@@ -126,9 +137,10 @@ struct Converter<std::array<float, 4>> {
 template <>
 struct Converter<std::vector<float>> {
     template <class V>
-    Result<std::vector<float>> operator()(const V& value) const {
+    optional<std::vector<float>> operator()(const V& value, Error& error) const {
         if (!isArray(value)) {
-            return Error { "value must be an array" };
+            error = { "value must be an array" };
+            return {};
         }
 
         std::vector<float> result;
@@ -137,7 +149,8 @@ struct Converter<std::vector<float>> {
         for (std::size_t i = 0; i < arrayLength(value); ++i) {
             optional<float> number = toNumber(arrayMember(value, i));
             if (!number) {
-                return Error { "value must be an array of numbers" };
+                error = { "value must be an array of numbers" };
+                return {};
             }
             result.push_back(*number);
         }
@@ -149,9 +162,10 @@ struct Converter<std::vector<float>> {
 template <>
 struct Converter<std::vector<std::string>> {
     template <class V>
-    Result<std::vector<std::string>> operator()(const V& value) const {
+    optional<std::vector<std::string>> operator()(const V& value, Error& error) const {
         if (!isArray(value)) {
-            return Error { "value must be an array" };
+            error = { "value must be an array" };
+            return {};
         }
 
         std::vector<std::string> result;
@@ -160,7 +174,8 @@ struct Converter<std::vector<std::string>> {
         for (std::size_t i = 0; i < arrayLength(value); ++i) {
             optional<std::string> string = toString(arrayMember(value, i));
             if (!string) {
-                return Error { "value must be an array of strings" };
+                error = { "value must be an array of strings" };
+                return {};
             }
             result.push_back(*string);
         }

include/mbgl/style/conversion/data_driven_property_value.hpp

@@ -12,29 +12,29 @@ namespace conversion {
 template <class T>
 struct Converter<DataDrivenPropertyValue<T>> {
     template <class V>
-    Result<DataDrivenPropertyValue<T>> operator()(const V& value) const {
+    optional<DataDrivenPropertyValue<T>> operator()(const V& value, Error& error) const {
         if (isUndefined(value)) {
-            return {};
+            return DataDrivenPropertyValue<T>();
         } else if (!isObject(value)) {
-            Result<T> constant = convert<T>(value);
+            optional<T> constant = convert<T>(value, error);
             if (!constant) {
-                return constant.error();
+                return {};
             }
             return DataDrivenPropertyValue<T>(*constant);
         } else if (!objectMember(value, "property")) {
-            Result<CameraFunction<T>> function = convert<CameraFunction<T>>(value);
+            optional<CameraFunction<T>> function = convert<CameraFunction<T>>(value, error);
             if (!function) {
-                return function.error();
+                return {};
             }
             return DataDrivenPropertyValue<T>(*function);
         } else {
-            Result<CompositeFunction<T>> composite = convert<CompositeFunction<T>>(value);
+            optional<CompositeFunction<T>> composite = convert<CompositeFunction<T>>(value, error);
             if (composite) {
                 return DataDrivenPropertyValue<T>(*composite);
             }
-            Result<SourceFunction<T>> source = convert<SourceFunction<T>>(value);
+            optional<SourceFunction<T>> source = convert<SourceFunction<T>>(value, error);
             if (!source) {
-                return source.error();
+                return {};
             }
             return DataDrivenPropertyValue<T>(*source);
         }