From 089c4e413fbe80711ebd874520d3b8fdcb997112 Mon Sep 17 00:00:00 2001
From: John Firebaugh <john.firebaugh@gmail.com>
Date: Wed, 22 Jun 2016 12:31:49 -0700
Subject: [PATCH] [core] Split up and clean up conversion.hpp

---
 benchmark/parse/filter.cpp                    |   3 +-
 include/mbgl/style/conversion.hpp             | 389 ++----------------
 include/mbgl/style/conversion/constant.hpp    | 174 ++++++++
 include/mbgl/style/conversion/filter.hpp      | 150 +++++++
 include/mbgl/style/conversion/function.hpp    |  69 ++++
 .../mbgl/style/conversion/property_value.hpp  |  34 ++
 src/mbgl/style/conversion.cpp                 |  25 --
 src/mbgl/style/parser.cpp                     |   9 +-
 src/mbgl/style/property_parsing.hpp           |  10 +-
 test/style/filter.cpp                         |   3 +-
 10 files changed, 483 insertions(+), 383 deletions(-)
 create mode 100644 include/mbgl/style/conversion/constant.hpp
 create mode 100644 include/mbgl/style/conversion/filter.hpp
 create mode 100644 include/mbgl/style/conversion/function.hpp
 create mode 100644 include/mbgl/style/conversion/property_value.hpp
 delete mode 100644 src/mbgl/style/conversion.cpp

diff --git a/benchmark/parse/filter.cpp b/benchmark/parse/filter.cpp
index 4269f8e16..24314e858 100644
--- a/benchmark/parse/filter.cpp
+++ b/benchmark/parse/filter.cpp
@@ -4,6 +4,7 @@
 #include <mbgl/style/filter_evaluator.hpp>
 #include <mbgl/style/rapidjson_conversion.hpp>
 #include <mbgl/style/conversion.hpp>
+#include <mbgl/style/conversion/filter.hpp>
 #include <mbgl/tile/geometry_tile_data.hpp>
 
 #include <rapidjson/document.h>
@@ -17,7 +18,7 @@ typedef std::multimap<std::string, Value> Properties;
 style::Filter parse(const char* expression) {
     rapidjson::GenericDocument<rapidjson::UTF8<>, rapidjson::CrtAllocator> doc;
     doc.Parse<0>(expression);
-    return style::conversion::convertFilter(doc).get<style::Filter>();
+    return *style::conversion::convert<style::Filter>(doc);
 }
 
 static void Parse_Filter(benchmark::State& state) {
diff --git a/include/mbgl/style/conversion.hpp b/include/mbgl/style/conversion.hpp
index 18326c8dd..eff7e293a 100644
--- a/include/mbgl/style/conversion.hpp
+++ b/include/mbgl/style/conversion.hpp
@@ -1,39 +1,32 @@
 #pragma once
 
-#include <mbgl/style/types.hpp>
-#include <mbgl/style/property_value.hpp>
-#include <mbgl/style/filter.hpp>
-
 #include <mbgl/util/variant.hpp>
-#include <mbgl/util/optional.hpp>
-#include <mbgl/util/color.hpp>
-#include <mbgl/util/enum.hpp>
-
-#include <array>
-#include <string>
-#include <vector>
 
 namespace mbgl {
 namespace style {
 namespace conversion {
 
 /*
-   This namespace defines a generic conversion from a templated type `V` representing a JSON object conforming
-   to a style property from the Mapbox Style Specification, to `PropertyValue<T>`:
+   The `conversion` namespace defines conversions from a templated type `V` representing a JSON
+   object conforming to the schema defined by the Mapbox Style Specification, to the various C++
+   types that form the C++ model of that domain:
+
+       * `std::unique_ptr<Source>`
+       * `std::unique_ptr<Layer>`
+       * `Filter`
+       * `PropertyValue<T>`
+
+   A single template function serves as the public interface:
 
        template <class T, class V>
-       Result<PropertyValue<T>> convertPropertyValue(const V& value);
+       Result<T> convert(const V& value);
 
-   This is used concretely for conversions from RapidJSON types in mbgl core, and from v8 types in
-   the node bindings.
+   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.
 
-   It also defines a convertion from `V` to `Filter`, representing a JSON object conforming to a Style
-   Specification filter object:
-
-       template <class V>
-       Result<Filter> convertFilter(const V& value);
-
-   The requirements are that the following are legal expressions for a value `v` of type `const V&`:
+   The implementation of `convert` requires that the following are legal expressions for a value `v`
+   of type `const V&`:
 
       * `isArray(v)` -- returns a boolean indicating whether `v` represents a JSON array
       * `arrayLength(v)` -- called only if `isArray(v)`; returns a size_t length
@@ -51,341 +44,43 @@ namespace conversion {
         absence indicating `v` is not a boolean, number, or string. Numbers should be converted to
         unsigned integer, signed integer, or floating point, in descending preference.
 
-   If for any reason the conversion fails, the result of `convertPropertyValue` will be the `Error` variant,
-   which includes explanatory text.
+   The mbgl core implements these requirements for RapidJSON types, and the node bindings implement
+   them for v8 types.
 */
 
 struct Error { const char * message; };
 
-template <class V>
-using Result = variant<V, Error>;
+template <class T>
+class Result : private variant<T, Error> {
+public:
+    using variant<T, Error>::variant;
 
-template <class V, class T, class Enable = void>
-struct ConstantConverter {};
+    explicit operator bool() const {
+        return this->template is<T>();
+    }
 
-template <class V>
-struct ConstantConverter<V, bool> {
-    Result<bool> operator()(const V& value) const {
-        optional<bool> converted = toBool(value);
-        if (!converted) {
-            return Error { "value must be a boolean" };
-        }
-        return *converted;
+    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 V>
-struct ConstantConverter<V, float> {
-    Result<float> operator()(const V& value) const {
-        optional<float> converted = toNumber(value);
-        if (!converted) {
-            return Error { "value must be a number" };
-        }
-        return *converted;
-    }
-};
-
-template <class V>
-struct ConstantConverter<V, std::string> {
-    Result<std::string> operator()(const V& value) const {
-        optional<std::string> converted = toString(value);
-        if (!converted) {
-            return Error { "value must be a string" };
-        }
-        return *converted;
-    }
-};
-
-template <class V, class T>
-struct ConstantConverter<V, T, typename std::enable_if_t<std::is_enum<T>::value>> {
-    Result<T> operator()(const V& value) const {
-        optional<std::string> string = toString(value);
-        if (!string) {
-            return Error { "value must be a string" };
-        }
-
-        const auto result = Enum<T>::toEnum(*string);
-        if (!result) {
-            return Error { "value must be a valid enumeration value" };
-        }
-
-        return *result;
-    }
-};
-
-template <class V>
-struct ConstantConverter<V, Color> {
-    Result<Color> operator()(const V& value) const {
-        optional<std::string> string = toString(value);
-        if (!string) {
-            return Error { "value must be a string" };
-        }
-
-        optional<Color> color = Color::parse(*string);
-        if (!color) {
-            return Error { "value must be a valid color" };
-        }
-
-        return *color;
-    }
-};
-
-template <class V>
-struct ConstantConverter<V, std::array<float, 2>> {
-    Result<std::array<float, 2>> operator()(const V& value) const {
-        if (!isArray(value) || arrayLength(value) != 2) {
-            return Error { "value must be an array of two numbers" };
-        }
-
-        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" };
-        }
-
-        return std::array<float, 2> {{ *first, *second }};
-    }
-};
-
-template <class V>
-struct ConstantConverter<V, std::array<float, 4>> {
-    Result<std::array<float, 4>> operator()(const V& value) const {
-        if (!isArray(value) || arrayLength(value) != 4) {
-            return Error { "value must be an array of four numbers" };
-        }
-
-        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" };
-        }
-
-        return std::array<float, 4> {{ *first, *second, *third, *fourth }};
-    }
-};
-
-template <class V>
-struct ConstantConverter<V, std::vector<float>> {
-    Result<std::vector<float>> operator()(const V& value) const {
-        if (!isArray(value)) {
-            return Error { "value must be an array" };
-        }
-
-        std::vector<float> result;
-        result.reserve(arrayLength(value));
-
-        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" };
-            }
-            result.push_back(*number);
-        }
-
-        return result;
-    }
-};
-
-template <class V>
-struct ConstantConverter<V, std::vector<std::string>> {
-    Result<std::vector<std::string>> operator()(const V& value) const {
-        if (!isArray(value)) {
-            return Error { "value must be an array" };
-        }
-
-        std::vector<std::string> result;
-        result.reserve(arrayLength(value));
-
-        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" };
-            }
-            result.push_back(*string);
-        }
-
-        return result;
-    }
-};
+template <class T, class Enable = void>
+struct Converter;
 
 template <class T, class V>
-Result<PropertyValue<T>> convertPropertyValue(const V& value) {
-    if (!isObject(value)) {
-        Result<T> constant = ConstantConverter<V, T>()(value);
-        if (constant.template is<Error>()) {
-            return constant.template get<Error>();
-        }
-        return constant.template get<T>();
-    }
-
-    auto stopsValue = objectMember(value, "stops");
-    if (!stopsValue) {
-        return Error { "function value must specify stops" };
-    }
-
-    if (!isArray(*stopsValue)) {
-        return Error { "function stops must be an array" };
-    }
-
-    std::vector<std::pair<float, T>> stops;
-    for (std::size_t i = 0; i < arrayLength(*stopsValue); ++i) {
-        const auto& stopValue = arrayMember(*stopsValue, i);
-
-        if (!isArray(stopValue)) {
-            return Error { "function stop must be an array" };
-        }
-
-        if (arrayLength(stopValue) != 2) {
-            return Error { "function stop must have two elements" };
-        }
-
-        optional<float> z = toNumber(arrayMember(stopValue, 0));
-        if (!z) {
-            return Error { "function stop zoom level must be a number" };
-        }
-
-        Result<T> v = ConstantConverter<V, T>()(arrayMember(stopValue, 1));
-        if (v.template is<Error>()) {
-            return v.template get<Error>();
-        }
-
-        stops.emplace_back(*z, v.template get<T>());
-    }
-
-    auto baseValue = objectMember(value, "base");
-    if (!baseValue) {
-        return Function<T>(stops, 1.0f);
-    }
-
-    optional<float> base = toNumber(*baseValue);
-    if (!base) {
-        return Error { "function base must be a number"};
-    }
-
-    return Function<T>(stops, *base);
-}
-
-Result<Value> normalizeFilterValue(const std::string& key, const optional<Value>&);
-
-template <class V>
-Result<Filter> convertFilter(const V& value);
-
-template <class FilterType, class V>
-Result<Filter> parseUnaryFilter(const V& value) {
-    if (arrayLength(value) < 2) {
-        return Error { "filter expression must have 2 elements" };
-    }
-
-    optional<std::string> key = toString(arrayMember(value, 1));
-    if (!key) {
-        return Error { "filter expression key must be a string" };
-    }
-
-    return FilterType { *key };
-}
-
-template <class FilterType, class V>
-Result<Filter> parseBinaryFilter(const V& value) {
-    if (arrayLength(value) < 3) {
-        return Error { "filter expression must have 3 elements" };
-    }
-
-    optional<std::string> key = toString(arrayMember(value, 1));
-    if (!key) {
-        return Error { "filter expression key must be a string" };
-    }
-
-    Result<Value> filterValue = normalizeFilterValue(*key, toValue(arrayMember(value, 2)));
-    if (filterValue.is<Error>()) {
-        return filterValue.get<Error>();
-    }
-
-    return FilterType { *key, filterValue.get<Value>() };
-}
-
-template <class FilterType, class V>
-Result<Filter> parseSetFilter(const V& value) {
-    if (arrayLength(value) < 2) {
-        return Error { "filter expression must at least 2 elements" };
-    }
-
-    optional<std::string> key = toString(arrayMember(value, 1));
-    if (!key) {
-        return Error { "filter expression key must be a string" };
-    }
-
-    std::vector<Value> values;
-    for (std::size_t i = 2; i < arrayLength(value); ++i) {
-        Result<Value> filterValue = normalizeFilterValue(*key, toValue(arrayMember(value, i)));
-        if (filterValue.is<Error>()) {
-            return filterValue.get<Error>();
-        }
-        values.push_back(filterValue.get<Value>());
-    }
-
-    return FilterType { *key, std::move(values) };
-}
-
-template <class FilterType, class V>
-Result<Filter> parseCompoundFilter(const V& value) {
-    std::vector<Filter> filters;
-    for (std::size_t i = 1; i < arrayLength(value); ++i) {
-        Result<Filter> element = convertFilter(arrayMember(value, i));
-        if (element.is<Error>()) {
-            return element;
-        }
-        filters.push_back(element.get<Filter>());
-    }
-
-    return FilterType { std::move(filters) };
-}
-
-template <class V>
-Result<Filter> convertFilter(const V& value) {
-    if (!isArray(value)) {
-        return Error { "filter expression must be an array" };
-    }
-
-    if (arrayLength(value) < 1) {
-        return Error { "filter expression must have at least 1 element" };
-    }
-
-    optional<std::string> op = toString(arrayMember(value, 0));
-    if (!op) {
-        return Error { "filter operator must be a string" };
-    }
-
-    if (*op == "==") {
-        return parseBinaryFilter<EqualsFilter>(value);
-    } else if (*op == "!=") {
-        return parseBinaryFilter<NotEqualsFilter>(value);
-    } else if (*op == ">") {
-        return parseBinaryFilter<GreaterThanFilter>(value);
-    } else if (*op == ">=") {
-        return parseBinaryFilter<GreaterThanEqualsFilter>(value);
-    } else if (*op == "<") {
-        return parseBinaryFilter<LessThanFilter>(value);
-    } else if (*op == "<=") {
-        return parseBinaryFilter<LessThanEqualsFilter>(value);
-    } else if (*op == "in") {
-        return parseSetFilter<InFilter>(value);
-    } else if (*op == "!in") {
-        return parseSetFilter<NotInFilter>(value);
-    } else if (*op == "all") {
-        return parseCompoundFilter<AllFilter>(value);
-    } else if (*op == "any") {
-        return parseCompoundFilter<AnyFilter>(value);
-    } else if (*op == "none") {
-        return parseCompoundFilter<NoneFilter>(value);
-    } else if (*op == "has") {
-        return parseUnaryFilter<HasFilter>(value);
-    } else if (*op == "!has") {
-       return parseUnaryFilter<NotHasFilter>(value);
-    }
-
-    return Error { "filter operator must be one of \"==\", \"!=\", \">\", \">=\", \"<\", \"<=\", \"in\", \"!in\", \"all\", \"any\", \"none\", \"has\", or \"!has\"" };
+Result<T> convert(const V& value) {
+    return Converter<T>()(value);
 }
 
 } // namespace conversion
diff --git a/include/mbgl/style/conversion/constant.hpp b/include/mbgl/style/conversion/constant.hpp
new file mode 100644
index 000000000..05bf968f4
--- /dev/null
+++ b/include/mbgl/style/conversion/constant.hpp
@@ -0,0 +1,174 @@
+#pragma once
+
+#include <mbgl/style/conversion.hpp>
+#include <mbgl/util/optional.hpp>
+#include <mbgl/util/color.hpp>
+#include <mbgl/util/enum.hpp>
+
+#include <array>
+#include <string>
+#include <vector>
+
+namespace mbgl {
+namespace style {
+namespace conversion {
+
+template <>
+struct Converter<bool> {
+    template <class V>
+    Result<bool> operator()(const V& value) const {
+        optional<bool> converted = toBool(value);
+        if (!converted) {
+            return Error { "value must be a boolean" };
+        }
+        return *converted;
+    }
+};
+
+template <>
+struct Converter<float> {
+    template <class V>
+    Result<float> operator()(const V& value) const {
+        optional<float> converted = toNumber(value);
+        if (!converted) {
+            return Error { "value must be a number" };
+        }
+        return *converted;
+    }
+};
+
+template <>
+struct Converter<std::string> {
+    template <class V>
+    Result<std::string> operator()(const V& value) const {
+        optional<std::string> converted = toString(value);
+        if (!converted) {
+            return Error { "value must be a string" };
+        }
+        return *converted;
+    }
+};
+
+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<std::string> string = toString(value);
+        if (!string) {
+            return Error { "value must be a string" };
+        }
+
+        const auto result = Enum<T>::toEnum(*string);
+        if (!result) {
+            return Error { "value must be a valid enumeration value" };
+        }
+
+        return *result;
+    }
+};
+
+template <>
+struct Converter<Color> {
+    template <class V>
+    Result<Color> operator()(const V& value) const {
+        optional<std::string> string = toString(value);
+        if (!string) {
+            return Error { "value must be a string" };
+        }
+
+        optional<Color> color = Color::parse(*string);
+        if (!color) {
+            return Error { "value must be a valid color" };
+        }
+
+        return *color;
+    }
+};
+
+template <>
+struct Converter<std::array<float, 2>> {
+    template <class V>
+    Result<std::array<float, 2>> operator()(const V& value) const {
+        if (!isArray(value) || arrayLength(value) != 2) {
+            return Error { "value must be an array of two numbers" };
+        }
+
+        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" };
+        }
+
+        return std::array<float, 2> {{ *first, *second }};
+    }
+};
+
+template <>
+struct Converter<std::array<float, 4>> {
+    template <class V>
+    Result<std::array<float, 4>> operator()(const V& value) const {
+        if (!isArray(value) || arrayLength(value) != 4) {
+            return Error { "value must be an array of four numbers" };
+        }
+
+        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" };
+        }
+
+        return std::array<float, 4> {{ *first, *second, *third, *fourth }};
+    }
+};
+
+template <>
+struct Converter<std::vector<float>> {
+    template <class V>
+    Result<std::vector<float>> operator()(const V& value) const {
+        if (!isArray(value)) {
+            return Error { "value must be an array" };
+        }
+
+        std::vector<float> result;
+        result.reserve(arrayLength(value));
+
+        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" };
+            }
+            result.push_back(*number);
+        }
+
+        return result;
+    }
+};
+
+template <>
+struct Converter<std::vector<std::string>> {
+    template <class V>
+    Result<std::vector<std::string>> operator()(const V& value) const {
+        if (!isArray(value)) {
+            return Error { "value must be an array" };
+        }
+
+        std::vector<std::string> result;
+        result.reserve(arrayLength(value));
+
+        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" };
+            }
+            result.push_back(*string);
+        }
+
+        return result;
+    }
+};
+
+} // namespace conversion
+} // namespace style
+} // namespace mbgl
diff --git a/include/mbgl/style/conversion/filter.hpp b/include/mbgl/style/conversion/filter.hpp
new file mode 100644
index 000000000..3ab91e5bb
--- /dev/null
+++ b/include/mbgl/style/conversion/filter.hpp
@@ -0,0 +1,150 @@
+#pragma once
+
+#include <mbgl/style/filter.hpp>
+#include <mbgl/style/conversion.hpp>
+#include <mbgl/util/geometry.hpp>
+
+namespace mbgl {
+namespace style {
+namespace conversion {
+
+template <>
+struct Converter<Filter> {
+public:
+    template <class V>
+    Result<Filter> operator()(const V& value) const {
+        if (!isArray(value)) {
+            return Error { "filter expression must be an array" };
+        }
+
+        if (arrayLength(value) < 1) {
+            return Error { "filter expression must have at least 1 element" };
+        }
+
+        optional<std::string> op = toString(arrayMember(value, 0));
+        if (!op) {
+            return Error { "filter operator must be a string" };
+        }
+
+        if (*op == "==") {
+            return convertBinaryFilter<EqualsFilter>(value);
+        } else if (*op == "!=") {
+            return convertBinaryFilter<NotEqualsFilter>(value);
+        } else if (*op == ">") {
+            return convertBinaryFilter<GreaterThanFilter>(value);
+        } else if (*op == ">=") {
+            return convertBinaryFilter<GreaterThanEqualsFilter>(value);
+        } else if (*op == "<") {
+            return convertBinaryFilter<LessThanFilter>(value);
+        } else if (*op == "<=") {
+            return convertBinaryFilter<LessThanEqualsFilter>(value);
+        } else if (*op == "in") {
+            return convertSetFilter<InFilter>(value);
+        } else if (*op == "!in") {
+            return convertSetFilter<NotInFilter>(value);
+        } else if (*op == "all") {
+            return convertCompoundFilter<AllFilter>(value);
+        } else if (*op == "any") {
+            return convertCompoundFilter<AnyFilter>(value);
+        } else if (*op == "none") {
+            return convertCompoundFilter<NoneFilter>(value);
+        } else if (*op == "has") {
+            return convertUnaryFilter<HasFilter>(value);
+        } else if (*op == "!has") {
+           return convertUnaryFilter<NotHasFilter>(value);
+        }
+
+        return Error { "filter operator must be one of \"==\", \"!=\", \">\", \">=\", \"<\", \"<=\", \"in\", \"!in\", \"all\", \"any\", \"none\", \"has\", or \"!has\"" };
+    }
+
+private:
+    Result<Value> normalizeValue(const std::string& key, const optional<Value>& value) const {
+        if (!value) {
+            return Error { "filter expression value must be a boolean, number, or string" };
+        } else if (key != "$type") {
+            return *value;
+        } else if (*value == std::string("Point")) {
+            return Value(uint64_t(FeatureType::Point));
+        } else if (*value == std::string("LineString")) {
+            return Value(uint64_t(FeatureType::LineString));
+        } else if (*value == std::string("Polygon")) {
+            return Value(uint64_t(FeatureType::Polygon));
+        } else {
+            return Error { "value for $type filter must be Point, LineString, or Polygon" };
+        }
+    }
+
+    template <class FilterType, class V>
+    Result<Filter> convertUnaryFilter(const V& value) const {
+        if (arrayLength(value) < 2) {
+            return Error { "filter expression must have 2 elements" };
+        }
+
+        optional<std::string> key = toString(arrayMember(value, 1));
+        if (!key) {
+            return Error { "filter expression key must be a string" };
+        }
+
+        return FilterType { *key };
+    }
+
+    template <class FilterType, class V>
+    Result<Filter> convertBinaryFilter(const V& value) const {
+        if (arrayLength(value) < 3) {
+            return Error { "filter expression must have 3 elements" };
+        }
+
+        optional<std::string> key = toString(arrayMember(value, 1));
+        if (!key) {
+            return Error { "filter expression key must be a string" };
+        }
+
+        Result<Value> filterValue = normalizeValue(*key, toValue(arrayMember(value, 2)));
+        if (!filterValue) {
+            return filterValue.error();
+        }
+
+        return FilterType { *key, *filterValue };
+    }
+
+    template <class FilterType, class V>
+    Result<Filter> convertSetFilter(const V& value) const {
+        if (arrayLength(value) < 2) {
+            return Error { "filter expression must at least 2 elements" };
+        }
+
+        optional<std::string> key = toString(arrayMember(value, 1));
+        if (!key) {
+            return Error { "filter expression key must be a string" };
+        }
+
+        std::vector<Value> values;
+        for (std::size_t i = 2; i < arrayLength(value); ++i) {
+            Result<Value> filterValue = normalizeValue(*key, toValue(arrayMember(value, i)));
+            if (!filterValue) {
+                return filterValue.error();
+            }
+            values.push_back(*filterValue);
+        }
+
+        return FilterType { *key, std::move(values) };
+    }
+
+    template <class FilterType, class V>
+    Result<Filter> convertCompoundFilter(const V& value) const {
+        std::vector<Filter> filters;
+        for (std::size_t i = 1; i < arrayLength(value); ++i) {
+            Result<Filter> element = operator()(arrayMember(value, i));
+            if (!element) {
+                return element.error();
+            }
+            filters.push_back(*element);
+        }
+
+        return FilterType { std::move(filters) };
+    }
+};
+
+} // namespace conversion
+} // namespace style
+} // namespace mbgl
diff --git a/include/mbgl/style/conversion/function.hpp b/include/mbgl/style/conversion/function.hpp
new file mode 100644
index 000000000..f14b5089b
--- /dev/null
+++ b/include/mbgl/style/conversion/function.hpp
@@ -0,0 +1,69 @@
+#pragma once
+
+#include <mbgl/style/function.hpp>
+#include <mbgl/style/conversion.hpp>
+#include <mbgl/style/conversion/constant.hpp>
+
+namespace mbgl {
+namespace style {
+namespace conversion {
+
+template <class T>
+struct Converter<Function<T>> {
+    template <class V>
+    Result<Function<T>> operator()(const V& value) const {
+        if (!isObject(value)) {
+            return Error { "function must be an object" };
+        }
+
+        auto stopsValue = objectMember(value, "stops");
+        if (!stopsValue) {
+            return Error { "function value must specify stops" };
+        }
+
+        if (!isArray(*stopsValue)) {
+            return Error { "function stops must be an array" };
+        }
+
+        std::vector<std::pair<float, T>> stops;
+        for (std::size_t i = 0; i < arrayLength(*stopsValue); ++i) {
+            const auto& stopValue = arrayMember(*stopsValue, i);
+
+            if (!isArray(stopValue)) {
+                return Error { "function stop must be an array" };
+            }
+
+            if (arrayLength(stopValue) != 2) {
+                return Error { "function stop must have two elements" };
+            }
+
+            optional<float> z = toNumber(arrayMember(stopValue, 0));
+            if (!z) {
+                return Error { "function stop zoom level must be a number" };
+            }
+
+            Result<T> v = convert<T>(arrayMember(stopValue, 1));
+            if (!v) {
+                return v.error();
+            }
+
+            stops.emplace_back(*z, *v);
+        }
+
+        auto baseValue = objectMember(value, "base");
+        if (!baseValue) {
+            return Function<T>(stops, 1.0f);
+        }
+
+        optional<float> base = toNumber(*baseValue);
+        if (!base) {
+            return Error { "function base must be a number"};
+        }
+
+        return Function<T>(stops, *base);
+    }
+};
+
+} // namespace conversion
+} // namespace style
+} // namespace mbgl
diff --git a/include/mbgl/style/conversion/property_value.hpp b/include/mbgl/style/conversion/property_value.hpp
new file mode 100644
index 000000000..840643abb
--- /dev/null
+++ b/include/mbgl/style/conversion/property_value.hpp
@@ -0,0 +1,34 @@
+#pragma once
+
+#include <mbgl/style/property_value.hpp>
+#include <mbgl/style/conversion.hpp>
+#include <mbgl/style/conversion/constant.hpp>
+#include <mbgl/style/conversion/function.hpp>
+
+namespace mbgl {
+namespace style {
+namespace conversion {
+
+template <class T>
+struct Converter<PropertyValue<T>> {
+    template <class V>
+    Result<PropertyValue<T>> operator()(const V& value) const {
+        if (isObject(value)) {
+            Result<Function<T>> function = convert<Function<T>>(value);
+            if (!function) {
+                return function.error();
+            }
+            return *function;
+        } else {
+            Result<T> constant = convert<T>(value);
+            if (!constant) {
+                return constant.error();
+            }
+            return *constant;
+        }
+    }
+};
+
+} // namespace conversion
+} // namespace style
+} // namespace mbgl
diff --git a/src/mbgl/style/conversion.cpp b/src/mbgl/style/conversion.cpp
deleted file mode 100644
index e863e285c..000000000
--- a/src/mbgl/style/conversion.cpp
+++ /dev/null
@@ -1,25 +0,0 @@
-#include <mbgl/style/conversion.hpp>
-
-namespace mbgl {
-namespace style {
-namespace conversion {
-
-Result<Value> normalizeFilterValue(const std::string& key, const optional<Value>& value) {
-    if (!value) {
-        return Error { "filter expression value must be a boolean, number, or string" };
-    } else if (key != "$type") {
-        return *value;
-    } else if (*value == std::string("Point")) {
-        return Value(uint64_t(FeatureType::Point));
-    } else if (*value == std::string("LineString")) {
-        return Value(uint64_t(FeatureType::LineString));
-    } else if (*value == std::string("Polygon")) {
-        return Value(uint64_t(FeatureType::Polygon));
-    } else {
-        return Error { "value for $type filter must be Point, LineString, or Polygon" };
-    }
-}
-
-} // namespace conversion
-} // namespace style
-} // namespace mbgl
diff --git a/src/mbgl/style/parser.cpp b/src/mbgl/style/parser.cpp
index eddda5d3f..bbe2fd786 100644
--- a/src/mbgl/style/parser.cpp
+++ b/src/mbgl/style/parser.cpp
@@ -11,6 +11,7 @@
 #include <mbgl/style/layers/background_layer.hpp>
 #include <mbgl/style/rapidjson_conversion.hpp>
 #include <mbgl/style/conversion.hpp>
+#include <mbgl/style/conversion/filter.hpp>
 
 #include <mbgl/platform/log.hpp>
 
@@ -277,11 +278,11 @@ void Parser::parseLayer(const std::string& id, const JSValue& value, std::unique
         }
 
         if (value.HasMember("filter")) {
-            conversion::Result<Filter> filter = conversion::convertFilter(value["filter"]);
-            if (filter.is<Filter>()) {
-                impl->filter = filter.get<Filter>();
+            conversion::Result<Filter> filter = conversion::convert<Filter>(value["filter"]);
+            if (filter) {
+                impl->filter = *filter;
             } else {
-                Log::Warning(Event::ParseStyle, filter.get<conversion::Error>().message);
+                Log::Warning(Event::ParseStyle, filter.error().message);
             }
         }
 
diff --git a/src/mbgl/style/property_parsing.hpp b/src/mbgl/style/property_parsing.hpp
index 0c750ca29..8c2bd2c0f 100644
--- a/src/mbgl/style/property_parsing.hpp
+++ b/src/mbgl/style/property_parsing.hpp
@@ -3,7 +3,7 @@
 #include <mbgl/style/property_value.hpp>
 #include <mbgl/style/transition_options.hpp>
 #include <mbgl/style/rapidjson_conversion.hpp>
-#include <mbgl/style/conversion.hpp>
+#include <mbgl/style/conversion/property_value.hpp>
 
 #include <mbgl/platform/log.hpp>
 
@@ -12,12 +12,12 @@ namespace style {
 
 template <typename T>
 PropertyValue<T> parseProperty(const char* name, const JSValue& value) {
-    conversion::Result<PropertyValue<T>> result = conversion::convertPropertyValue<T>(value);
-    if (result.template is<conversion::Error>()) {
-        Log::Warning(Event::ParseStyle, "%s: %s", name, result.template get<conversion::Error>().message);
+    conversion::Result<PropertyValue<T>> result = conversion::convert<PropertyValue<T>>(value);
+    if (!result) {
+        Log::Warning(Event::ParseStyle, "%s: %s", name, result.error().message);
         return {};
     }
-    return result.template get<PropertyValue<T>>();
+    return *result;
 }
 
 optional<TransitionOptions> parseTransitionOptions(const char * name, const JSValue&);
diff --git a/test/style/filter.cpp b/test/style/filter.cpp
index fd763896a..2d26a8eb6 100644
--- a/test/style/filter.cpp
+++ b/test/style/filter.cpp
@@ -4,6 +4,7 @@
 #include <mbgl/style/filter_evaluator.hpp>
 #include <mbgl/style/rapidjson_conversion.hpp>
 #include <mbgl/style/conversion.hpp>
+#include <mbgl/style/conversion/filter.hpp>
 
 #include <rapidjson/document.h>
 
@@ -17,7 +18,7 @@ typedef std::multimap<std::string, mbgl::Value> Properties;
 Filter parse(const char * expression) {
     rapidjson::GenericDocument<rapidjson::UTF8<>, rapidjson::CrtAllocator> doc;
     doc.Parse<0>(expression);
-    return conversion::convertFilter(doc).get<Filter>();
+    return *conversion::convert<Filter>(doc);
 }
 
 bool evaluate(const Filter& filter, const Properties& properties, FeatureType type = FeatureType::Unknown) {