From 3194678e52635c6d4e757f159013b378a83f38e0 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 8 Jan 2018 13:18:40 -0800 Subject: [core] Improve typing for !=, == expressions --- cmake/core-files.cmake | 2 + include/mbgl/style/expression/equals.hpp | 31 +++++++++ platform/node/test/ignores.json | 14 ---- src/mbgl/style/expression/compound_expression.cpp | 16 ----- src/mbgl/style/expression/equals.cpp | 83 +++++++++++++++++++++++ src/mbgl/style/expression/parsing_context.cpp | 3 + 6 files changed, 119 insertions(+), 30 deletions(-) create mode 100644 include/mbgl/style/expression/equals.hpp create mode 100644 src/mbgl/style/expression/equals.cpp diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake index 5c060f4698..e3856a35a5 100644 --- a/cmake/core-files.cmake +++ b/cmake/core-files.cmake @@ -410,6 +410,7 @@ set(MBGL_CORE_FILES include/mbgl/style/expression/coalesce.hpp include/mbgl/style/expression/coercion.hpp include/mbgl/style/expression/compound_expression.hpp + include/mbgl/style/expression/equals.hpp include/mbgl/style/expression/expression.hpp include/mbgl/style/expression/find_zoom_curve.hpp include/mbgl/style/expression/get_covering_stops.hpp @@ -432,6 +433,7 @@ set(MBGL_CORE_FILES src/mbgl/style/expression/coalesce.cpp src/mbgl/style/expression/coercion.cpp src/mbgl/style/expression/compound_expression.cpp + src/mbgl/style/expression/equals.cpp src/mbgl/style/expression/find_zoom_curve.cpp src/mbgl/style/expression/get_covering_stops.cpp src/mbgl/style/expression/interpolate.cpp diff --git a/include/mbgl/style/expression/equals.hpp b/include/mbgl/style/expression/equals.hpp new file mode 100644 index 0000000000..3c0c024294 --- /dev/null +++ b/include/mbgl/style/expression/equals.hpp @@ -0,0 +1,31 @@ +#pragma once + +#include +#include +#include + +#include + +namespace mbgl { +namespace style { +namespace expression { + +class Equals : public Expression { +public: + Equals(std::unique_ptr lhs, std::unique_ptr rhs, bool negate); + + static ParseResult parse(const mbgl::style::conversion::Convertible&, ParsingContext&); + + void eachChild(const std::function& visit) const override; + bool operator==(const Expression&) const override; + EvaluationResult evaluate(const EvaluationContext&) const override; + +private: + std::unique_ptr lhs; + std::unique_ptr rhs; + bool negate; +}; + +} // namespace expression +} // namespace style +} // namespace mbgl diff --git a/platform/node/test/ignores.json b/platform/node/test/ignores.json index 95335e530a..6e551f9daa 100644 --- a/platform/node/test/ignores.json +++ b/platform/node/test/ignores.json @@ -3,19 +3,6 @@ "expression-tests/array/implicit-2": "https://github.com/mapbox/mapbox-gl-native/issues/10533", "expression-tests/array/implicit-3": "https://github.com/mapbox/mapbox-gl-native/issues/10533", "expression-tests/coalesce/inference": "https://github.com/mapbox/mapbox-gl-native/issues/10588", - "expression-tests/equal/array": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/equal/color": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/equal/mismatch": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/equal/null-lhs": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/equal/null-rhs": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/equal/number": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/equal/object": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/equal/string": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/equal/value": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/not_equal/mismatch": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/not_equal/number": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/not_equal/string": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/not_equal/value": "https://github.com/mapbox/mapbox-gl-native/issues/10678", "expression-tests/interpolate/linear-color": "https://github.com/mapbox/mapbox-gl-native/issues/10604", "expression-tests/to-rgba/alpha": "https://github.com/mapbox/mapbox-gl-native/issues/10604", "expression-tests/to-rgba/basic": "https://github.com/mapbox/mapbox-gl-native/issues/10604", @@ -74,7 +61,6 @@ "render-tests/regressions/mapbox-gl-js#5370": "skip - https://github.com/mapbox/mapbox-gl-native/pull/9439", "render-tests/regressions/mapbox-gl-js#5599": "https://github.com/mapbox/mapbox-gl-native/issues/10399", "render-tests/regressions/mapbox-gl-js#5740": "https://github.com/mapbox/mapbox-gl-native/issues/10619", - "render-tests/regressions/mapbox-gl-js#5947": "https://github.com/mapbox/mapbox-gl-native/issues/10678", "render-tests/regressions/mapbox-gl-native#7357": "https://github.com/mapbox/mapbox-gl-native/issues/7357", "render-tests/runtime-styling/image-add-sdf": "https://github.com/mapbox/mapbox-gl-native/issues/9847", "render-tests/runtime-styling/paint-property-fill-flat-to-extrude": "https://github.com/mapbox/mapbox-gl-native/issues/6745", diff --git a/src/mbgl/style/expression/compound_expression.cpp b/src/mbgl/style/expression/compound_expression.cpp index 70bc6dfd95..ff21157faa 100644 --- a/src/mbgl/style/expression/compound_expression.cpp +++ b/src/mbgl/style/expression/compound_expression.cpp @@ -175,12 +175,6 @@ struct Signature::value>> using Definition = CompoundExpressionRegistry::Definition; -template -Result equal(const T& lhs, const T& rhs) { return lhs == rhs; } - -template -Result notEqual(const T& lhs, const T& rhs) { return lhs != rhs; } - template static std::unique_ptr makeSignature(Fn evaluateFunction) { return std::make_unique>(evaluateFunction); @@ -376,16 +370,6 @@ std::unordered_map initiali return result; }); - define("==", equal); - define("==", equal); - define("==", equal); - define("==", equal); - - define("!=", notEqual); - define("!=", notEqual); - define("!=", notEqual); - define("!=", notEqual); - define(">", [](double lhs, double rhs) -> Result { return lhs > rhs; }); define(">", [](const std::string& lhs, const std::string& rhs) -> Result { return lhs > rhs; }); define(">=", [](double lhs, double rhs) -> Result { return lhs >= rhs; }); diff --git a/src/mbgl/style/expression/equals.cpp b/src/mbgl/style/expression/equals.cpp new file mode 100644 index 0000000000..08ef85e92b --- /dev/null +++ b/src/mbgl/style/expression/equals.cpp @@ -0,0 +1,83 @@ +#include + +namespace mbgl { +namespace style { +namespace expression { + +Equals::Equals(std::unique_ptr lhs_, std::unique_ptr rhs_, bool negate_) + : Expression(type::Boolean), + lhs(std::move(lhs_)), + rhs(std::move(rhs_)), + negate(negate_) { +} + +EvaluationResult Equals::evaluate(const EvaluationContext& params) const { + EvaluationResult lhsResult = lhs->evaluate(params); + if (!lhsResult) return lhsResult; + + EvaluationResult rhsResult = rhs->evaluate(params); + if (!rhsResult) return lhsResult; + + bool result = *lhsResult == *rhsResult; + if (negate) { + result = !result; + } + return result; +} + +void Equals::eachChild(const std::function& visit) const { + visit(*lhs); + visit(*rhs); +} + +bool Equals::operator==(const Expression& e) const { + if (auto eq = dynamic_cast(&e)) { + return eq->negate == negate && *eq->lhs == *lhs && *eq->rhs == *rhs; + } + return false; +} + +static bool isComparableType(const type::Type& type) { + return type == type::String || + type == type::Number || + type == type::Boolean || + type == type::Null; +} + +using namespace mbgl::style::conversion; +ParseResult Equals::parse(const Convertible& value, ParsingContext& ctx) { + std::size_t length = arrayLength(value); + + if (length != 3) { + ctx.error("Expected two arguments."); + return ParseResult(); + } + + bool negate = toString(arrayMember(value, 0)) == std::string("!="); + + ParseResult lhs = ctx.parse(arrayMember(value, 1), 1, {type::Value}); + if (!lhs) return ParseResult(); + + ParseResult rhs = ctx.parse(arrayMember(value, 2), 2, {type::Value}); + if (!rhs) return ParseResult(); + + type::Type lhsType = (*lhs)->getType(); + type::Type rhsType = (*rhs)->getType(); + + if (!isComparableType(lhsType) && !isComparableType(rhsType)) { + ctx.error("Expected at least one argument to be a string, number, boolean, or null, but found (" + + toString(lhsType) + ", " + toString(rhsType) + ") instead."); + return ParseResult(); + } + + if (lhsType != rhsType && lhsType != type::Value && rhsType != type::Value) { + ctx.error("Cannot compare " + toString(lhsType) + " and " + toString(rhsType) + "."); + return ParseResult(); + } + + return ParseResult(std::make_unique(std::move(*lhs), std::move(*rhs), negate)); +} + +} // namespace expression +} // namespace style +} // namespace mbgl diff --git a/src/mbgl/style/expression/parsing_context.cpp b/src/mbgl/style/expression/parsing_context.cpp index 501ba2749f..8dd99dd8d0 100644 --- a/src/mbgl/style/expression/parsing_context.cpp +++ b/src/mbgl/style/expression/parsing_context.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -75,6 +76,8 @@ ParseResult ParsingContext::parse(const Convertible& value, std::size_t index_, const ExpressionRegistry& getExpressionRegistry() { static ExpressionRegistry registry {{ + {"==", Equals::parse}, + {"!=", Equals::parse}, {"all", All::parse}, {"any", Any::parse}, {"array", ArrayAssertion::parse}, -- cgit v1.2.1