From bf7785618ac5f3bbcba068e61ef6359d70aff92b Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 25 Jul 2018 18:15:24 -0700 Subject: [core] Simplify CompoundExpression implementation --- .../mbgl/style/expression/compound_expression.hpp | 119 +++------------- include/mbgl/style/expression/is_constant.hpp | 4 +- src/mbgl/style/expression/compound_expression.cpp | 149 ++++++++++++++------- src/mbgl/style/expression/find_zoom_curve.cpp | 8 +- src/mbgl/style/expression/is_constant.cpp | 4 +- src/mbgl/style/expression/parsing_context.cpp | 4 +- 6 files changed, 131 insertions(+), 157 deletions(-) diff --git a/include/mbgl/style/expression/compound_expression.hpp b/include/mbgl/style/expression/compound_expression.hpp index c618f2f206..ef10dadb55 100644 --- a/include/mbgl/style/expression/compound_expression.hpp +++ b/include/mbgl/style/expression/compound_expression.hpp @@ -5,9 +5,7 @@ #include #include #include - #include -#include #include #include @@ -16,115 +14,34 @@ namespace mbgl { namespace style { namespace expression { +namespace detail { +struct SignatureBase; +} // namespace detail + /* CompoundExpression provides a mechanism for implementing an expression simply by providing a list of pure functions of the form (const T0& arg0, const T1& arg1, ...) -> Result where T0, T1, ..., U are member types of mbgl::style::expression::Value. - + The majority of expressions specified in the style-spec are implemented in this fashion (see compound_expression.cpp). */ - - -/* - Represents the parameter list for an expression that takes an arbitrary - number of arguments (of a specific type). -*/ -struct VarargsType { type::Type type; }; -template -struct Varargs : std::vector { - template - Varargs(Args&&... args) : std::vector(std::forward(args)...) {} -}; - -namespace detail { -// Base class for the Signature structs that are used to determine -// each CompoundExpression definition's type::Type data from the type of its -// "evaluate" function. -struct SignatureBase { - SignatureBase(type::Type result_, variant, VarargsType> params_, std::string name_) : - result(std::move(result_)), - params(std::move(params_)), - name(std::move(name_)) - {} - virtual ~SignatureBase() = default; - virtual std::unique_ptr makeExpression(std::vector>) const = 0; - type::Type result; - variant, VarargsType> params; - std::string name; -}; -} // namespace detail - - -/* - Common base class for CompoundExpression instances. Used to - allow downcasting (and access to things like name & parameter list) during - an Expression tree traversal. -*/ -class CompoundExpressionBase : public Expression { +class CompoundExpression : public Expression { public: - CompoundExpressionBase(std::string name_, const detail::SignatureBase& signature) : - Expression(Kind::CompoundExpression, signature.result), - name(std::move(name_)), - params(signature.params) - {} + CompoundExpression(const detail::SignatureBase&, std::vector>); - std::string getName() const { return name; } - optional getParameterCount() const { - return params.match( - [&](const VarargsType&) { return optional(); }, - [&](const std::vector& p) -> optional { return p.size(); } - ); - } - - std::vector> possibleOutputs() const override { - return { nullopt }; - } - -private: - std::string name; - variant, VarargsType> params; -}; - -template -class CompoundExpression : public CompoundExpressionBase { -public: - using Args = typename Signature::Args; - - CompoundExpression(const std::string& name_, - Signature signature_, - typename Signature::Args args_) : - CompoundExpressionBase(name_, signature_), - signature(signature_), - args(std::move(args_)) - {} - - EvaluationResult evaluate(const EvaluationContext& evaluationParams) const override { - return signature.apply(evaluationParams, args); - } - - void eachChild(const std::function& visit) const override { - for (const std::unique_ptr& e : args) { - visit(*e); - } - } + std::string getOperator() const override; + EvaluationResult evaluate(const EvaluationContext& evaluationParams) const override; + std::vector> possibleOutputs() const override; + void eachChild(const std::function& visit) const override; + bool operator==(const Expression& e) const override; - bool operator==(const Expression& e) const override { - if (e.getKind() == Kind::CompoundExpression) { - auto rhs = static_cast(&e); - return getName() == rhs->getName() && Expression::childrenEqual(args, rhs->args); - } - return false; - } - - std::string getOperator() const override { - return signature.name; - } + optional getParameterCount() const; -private: - Signature signature; - typename Signature::Args args; +protected: + const detail::SignatureBase& signature; + std::vector> args; }; /* @@ -136,7 +53,9 @@ struct CompoundExpressionRegistry { static std::unordered_map definitions; }; -ParseResult parseCompoundExpression(const std::string name, const mbgl::style::conversion::Convertible& value, ParsingContext& ctx); +ParseResult parseCompoundExpression(const std::string name, + const mbgl::style::conversion::Convertible& value, + ParsingContext& ctx); ParseResult createCompoundExpression(const std::string& name, std::vector> args, diff --git a/include/mbgl/style/expression/is_constant.hpp b/include/mbgl/style/expression/is_constant.hpp index 065fa30db1..bd758977d8 100644 --- a/include/mbgl/style/expression/is_constant.hpp +++ b/include/mbgl/style/expression/is_constant.hpp @@ -10,9 +10,9 @@ namespace expression { template bool isGlobalPropertyConstant(const Expression& expression, const T& properties) { if (expression.getKind() == Kind::CompoundExpression) { - auto e = static_cast(&expression); + auto e = static_cast(&expression); for (const std::string& property : properties) { - if (e->getName() == property) { + if (e->getOperator() == property) { return false; } } diff --git a/src/mbgl/style/expression/compound_expression.cpp b/src/mbgl/style/expression/compound_expression.cpp index 4a74fa4f76..bbb57e19ab 100644 --- a/src/mbgl/style/expression/compound_expression.cpp +++ b/src/mbgl/style/expression/compound_expression.cpp @@ -9,13 +9,50 @@ #include #include #include +#include + #include namespace mbgl { namespace style { namespace expression { +/* + Represents the parameter list for an expression that takes an arbitrary + number of arguments (of a specific type). +*/ +struct VarargsType { type::Type type; }; + +bool operator==(const VarargsType& lhs, const VarargsType& rhs) { + return lhs.type == rhs.type; +} + +template +struct Varargs : std::vector { + template + Varargs(Args&&... args) : std::vector(std::forward(args)...) {} +}; + namespace detail { +// Base class for the Signature structs that are used to determine +// each CompoundExpression definition's type::Type data from the type of its +// "evaluate" function. +struct SignatureBase { + using Args = std::vector>; + + SignatureBase(type::Type result_, variant, VarargsType> params_, std::string name_) : + result(std::move(result_)), + params(std::move(params_)), + name(std::move(name_)) + {} + virtual ~SignatureBase() = default; + + virtual EvaluationResult apply(const EvaluationContext&, const Args&) const = 0; + + type::Type result; + variant, VarargsType> params; + std::string name; +}; /* The Signature structs are wrappers around an "evaluate()" function whose @@ -44,8 +81,6 @@ struct Signature; // Simple evaluate function (const T0&, const T1&, ...) -> Result template struct Signature : SignatureBase { - using Args = std::array, sizeof...(Params)>; - Signature(R (*evaluate_)(Params...), std::string name_) : SignatureBase( valueTypeToExpressionType>(), @@ -53,26 +88,23 @@ struct Signature : SignatureBase { std::move(name_) ), evaluate(evaluate_) {} - - EvaluationResult apply(const EvaluationContext& evaluationParameters, const Args& args) const { - return applyImpl(evaluationParameters, args, std::index_sequence_for{}); - } - std::unique_ptr makeExpression(std::vector> args) const override { - typename Signature::Args argsArray; - std::copy_n(std::make_move_iterator(args.begin()), sizeof...(Params), argsArray.begin()); - return std::make_unique>(name, *this, std::move(argsArray)); + EvaluationResult apply(const EvaluationContext& evaluationParameters, const Args& args) const override { + return applyImpl(evaluationParameters, args, std::index_sequence_for{}); } R (*evaluate)(Params...); + private: template EvaluationResult applyImpl(const EvaluationContext& evaluationParameters, const Args& args, std::index_sequence) const { - const std::array evaluated = {{std::get(args)->evaluate(evaluationParameters)...}}; - for (const auto& arg : evaluated) { - if(!arg) return arg.error(); + std::array evaluated; + for (std::size_t i = 0; i < sizeof...(Params); ++i) { + const EvaluationResult evaluatedArg = args.at(i)->evaluate(evaluationParameters); + if (!evaluatedArg) return evaluatedArg.error(); + evaluated[i] = std::move(*evaluatedArg); } - const R value = evaluate(*fromExpressionValue>(*(evaluated[I]))...); + const R value = evaluate(*fromExpressionValue>(evaluated[I])...); if (!value) return value.error(); return *value; } @@ -81,8 +113,6 @@ private: // Varargs evaluate function (const Varargs&) -> Result template struct Signature&)> : SignatureBase { - using Args = std::vector>; - Signature(R (*evaluate_)(const Varargs&), std::string name_) : SignatureBase( valueTypeToExpressionType>(), @@ -92,11 +122,7 @@ struct Signature&)> : SignatureBase { evaluate(evaluate_) {} - std::unique_ptr makeExpression(std::vector> args) const override { - return std::make_unique>(name, *this, std::move(args)); - }; - - EvaluationResult apply(const EvaluationContext& evaluationParameters, const Args& args) const { + EvaluationResult apply(const EvaluationContext& evaluationParameters, const Args& args) const override { Varargs evaluated; evaluated.reserve(args.size()); for (const auto& arg : args) { @@ -108,7 +134,7 @@ struct Signature&)> : SignatureBase { if (!value) return value.error(); return *value; } - + R (*evaluate)(const Varargs&); }; @@ -116,8 +142,6 @@ struct Signature&)> : SignatureBase { // (const EvaluationParams&, const T0&, const T1&, ...) -> Result template struct Signature : SignatureBase { - using Args = std::array, sizeof...(Params)>; - Signature(R (*evaluate_)(const EvaluationContext&, Params...), std::string name_) : SignatureBase( valueTypeToExpressionType>(), @@ -127,29 +151,24 @@ struct Signature : SignatureBase { evaluate(evaluate_) {} - std::unique_ptr makeExpression(std::vector> args) const override { - typename Signature::Args argsArray; - std::copy_n(std::make_move_iterator(args.begin()), sizeof...(Params), argsArray.begin()); - return std::make_unique>(name, *this, std::move(argsArray)); - } - - EvaluationResult apply(const EvaluationContext& evaluationParameters, const Args& args) const { + EvaluationResult apply(const EvaluationContext& evaluationParameters, const Args& args) const override { return applyImpl(evaluationParameters, args, std::index_sequence_for{}); } private: template EvaluationResult applyImpl(const EvaluationContext& evaluationParameters, const Args& args, std::index_sequence) const { - const std::array evaluated = {{std::get(args)->evaluate(evaluationParameters)...}}; - for (const auto& arg : evaluated) { - if(!arg) return arg.error(); + std::array evaluated; + for (std::size_t i = 0; i < sizeof...(Params); ++i) { + const EvaluationResult evaluatedArg = args.at(i)->evaluate(evaluationParameters); + if (!evaluatedArg) return evaluatedArg.error(); + evaluated[i] = std::move(*evaluatedArg); } - // TODO: assert correct runtime type of each arg value - const R value = evaluate(evaluationParameters, *fromExpressionValue>(*(evaluated[I]))...); + const R value = evaluate(evaluationParameters, *fromExpressionValue>(evaluated[I])...); if (!value) return value.error(); return *value; } - + R (*evaluate)(const EvaluationContext&, Params...); }; @@ -157,8 +176,6 @@ private: // (const EvaluationContext&, const Varargs&) -> Result template struct Signature&)> : SignatureBase { - using Args = std::vector>; - Signature(R (*evaluate_)(const EvaluationContext&, const Varargs&), std::string name_) : SignatureBase( valueTypeToExpressionType>(), @@ -167,12 +184,8 @@ struct Signature&)> : SignatureBas ), evaluate(evaluate_) {} - - std::unique_ptr makeExpression(std::vector> args) const override { - return std::make_unique>(name, *this, std::move(args)); - }; - - EvaluationResult apply(const EvaluationContext& evaluationParameters, const Args& args) const { + + EvaluationResult apply(const EvaluationContext& evaluationParameters, const Args& args) const override { Varargs evaluated; evaluated.reserve(args.size()); for (const auto& arg : args) { @@ -184,7 +197,7 @@ struct Signature&)> : SignatureBas if (!value) return value.error(); return *value; } - + R (*evaluate)(const EvaluationContext&, const Varargs&); }; @@ -680,7 +693,7 @@ static ParseResult createCompoundExpression(const Definition& definition, } if (signatureContext.getErrors().size() == 0) { - return ParseResult(signature->makeExpression(std::move(args))); + return ParseResult(std::make_unique(*signature, std::move(args))); } } @@ -792,6 +805,48 @@ ParseResult createCompoundExpression(const std::string& name, return createCompoundExpression(CompoundExpressionRegistry::definitions.at(name), std::move(args), ctx); } +CompoundExpression::CompoundExpression(const detail::SignatureBase& signature_, std::vector> args_) : + Expression(Kind::CompoundExpression, signature_.result), + signature(signature_), + args(std::move(args_)) +{} + +std::string CompoundExpression::getOperator() const { + return signature.name; +} + +EvaluationResult CompoundExpression::evaluate(const EvaluationContext& evaluationParams) const { + return signature.apply(evaluationParams, args); +} + +optional CompoundExpression::getParameterCount() const { + return signature.params.match( + [&](const VarargsType&) { return optional(); }, + [&](const std::vector& p) -> optional { return p.size(); } + ); +} + +std::vector> CompoundExpression::possibleOutputs() const { + return { nullopt }; +} + +void CompoundExpression::eachChild(const std::function& visit) const { + for (const std::unique_ptr& e : args) { + visit(*e); + } +} + +bool CompoundExpression::operator==(const Expression& e) const { + if (e.getKind() == Kind::CompoundExpression) { + auto rhs = static_cast(&e); + return signature.name == rhs->signature.name && + signature.result == rhs->signature.result && + signature.params == rhs->signature.params && + Expression::childrenEqual(args, rhs->args); + } + return false; +} + } // namespace expression } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/expression/find_zoom_curve.cpp b/src/mbgl/style/expression/find_zoom_curve.cpp index a27f8560ef..457ae8c927 100644 --- a/src/mbgl/style/expression/find_zoom_curve.cpp +++ b/src/mbgl/style/expression/find_zoom_curve.cpp @@ -34,8 +34,8 @@ optional> findZoomCurve(c case Kind::Interpolate: { auto curve = static_cast(e); if (curve->getInput()->getKind() == Kind::CompoundExpression) { - auto z = static_cast(curve->getInput().get()); - if (z && z->getName() == "zoom") { + auto z = static_cast(curve->getInput().get()); + if (z && z->getOperator() == "zoom") { result = {curve}; } } @@ -44,8 +44,8 @@ optional> findZoomCurve(c case Kind::Step: { auto step = static_cast(e); if (step->getInput()->getKind() == Kind::CompoundExpression) { - auto z = static_cast(step->getInput().get()); - if (z && z->getName() == "zoom") { + auto z = static_cast(step->getInput().get()); + if (z && z->getOperator() == "zoom") { result = {step}; } } diff --git a/src/mbgl/style/expression/is_constant.cpp b/src/mbgl/style/expression/is_constant.cpp index 3b1f1aba8c..69f27f6fef 100644 --- a/src/mbgl/style/expression/is_constant.cpp +++ b/src/mbgl/style/expression/is_constant.cpp @@ -10,8 +10,8 @@ constexpr static const char filter[] = "filter-"; bool isFeatureConstant(const Expression& expression) { if (expression.getKind() == Kind::CompoundExpression) { - auto e = static_cast(&expression); - const std::string name = e->getName(); + auto e = static_cast(&expression); + const std::string name = e->getOperator(); optional parameterCount = e->getParameterCount(); if (name == "get" && parameterCount && *parameterCount == 1) { return false; diff --git a/src/mbgl/style/expression/parsing_context.cpp b/src/mbgl/style/expression/parsing_context.cpp index a4c04b03b1..979eb58236 100644 --- a/src/mbgl/style/expression/parsing_context.cpp +++ b/src/mbgl/style/expression/parsing_context.cpp @@ -38,8 +38,8 @@ bool isConstant(const Expression& expression) { } if (expression.getKind() == Kind::CompoundExpression) { - auto compound = static_cast(&expression); - if (compound->getName() == "error") { + auto compound = static_cast(&expression); + if (compound->getOperator() == "error") { return false; } } -- cgit v1.2.1