Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
[core] Improve typing for !=, == expressions
Browse files Browse the repository at this point in the history
  • Loading branch information
jfirebaugh committed Jan 10, 2018
1 parent 43a9bd3 commit a13027c
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 30 deletions.
2 changes: 2 additions & 0 deletions cmake/core-files.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,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
Expand All @@ -441,6 +442,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
Expand Down
31 changes: 31 additions & 0 deletions include/mbgl/style/expression/equals.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#pragma once

#include <mbgl/style/expression/expression.hpp>
#include <mbgl/style/expression/parsing_context.hpp>
#include <mbgl/style/conversion.hpp>

#include <memory>

namespace mbgl {
namespace style {
namespace expression {

class Equals : public Expression {
public:
Equals(std::unique_ptr<Expression> lhs, std::unique_ptr<Expression> rhs, bool negate);

static ParseResult parse(const mbgl::style::conversion::Convertible&, ParsingContext&);

void eachChild(const std::function<void(const Expression&)>& visit) const override;
bool operator==(const Expression&) const override;
EvaluationResult evaluate(const EvaluationContext&) const override;

private:
std::unique_ptr<Expression> lhs;
std::unique_ptr<Expression> rhs;
bool negate;
};

} // namespace expression
} // namespace style
} // namespace mbgl
14 changes: 0 additions & 14 deletions platform/node/test/ignores.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
16 changes: 0 additions & 16 deletions src/mbgl/style/expression/compound_expression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,6 @@ struct Signature<Lambda, std::enable_if_t<std::is_class<Lambda>::value>>

using Definition = CompoundExpressionRegistry::Definition;

template <typename T>
Result<bool> equal(const T& lhs, const T& rhs) { return lhs == rhs; }

template <typename T>
Result<bool> notEqual(const T& lhs, const T& rhs) { return lhs != rhs; }

template <typename Fn>
static std::unique_ptr<detail::SignatureBase> makeSignature(Fn evaluateFunction) {
return std::make_unique<detail::Signature<Fn>>(evaluateFunction);
Expand Down Expand Up @@ -376,16 +370,6 @@ std::unordered_map<std::string, CompoundExpressionRegistry::Definition> initiali
return result;
});

define("==", equal<double>);
define("==", equal<const std::string&>);
define("==", equal<bool>);
define("==", equal<NullValue>);

define("!=", notEqual<double>);
define("!=", notEqual<const std::string&>);
define("!=", notEqual<bool>);
define("!=", notEqual<NullValue>);

define(">", [](double lhs, double rhs) -> Result<bool> { return lhs > rhs; });
define(">", [](const std::string& lhs, const std::string& rhs) -> Result<bool> { return lhs > rhs; });
define(">=", [](double lhs, double rhs) -> Result<bool> { return lhs >= rhs; });
Expand Down
83 changes: 83 additions & 0 deletions src/mbgl/style/expression/equals.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
#include <mbgl/style/expression/equals.hpp>

namespace mbgl {
namespace style {
namespace expression {

Equals::Equals(std::unique_ptr<Expression> lhs_, std::unique_ptr<Expression> 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<void(const Expression&)>& visit) const {
visit(*lhs);
visit(*rhs);
}

bool Equals::operator==(const Expression& e) const {
if (auto eq = dynamic_cast<const Equals*>(&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<Equals>(std::move(*lhs), std::move(*rhs), negate));
}

} // namespace expression
} // namespace style
} // namespace mbgl
3 changes: 3 additions & 0 deletions src/mbgl/style/expression/parsing_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#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/interpolate.hpp>
#include <mbgl/style/expression/let.hpp>
#include <mbgl/style/expression/literal.hpp>
Expand Down Expand Up @@ -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},
Expand Down

0 comments on commit a13027c

Please sign in to comment.