From 8d26f7409106c94f37689d206f9a68aac57572a4 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Mon, 9 Apr 2018 17:59:18 -0400 Subject: Fix style parsing bug for constant expressions (#11606) * Fix style parsing bug for constant expressions Closes #10849 * Ignore tests for unported GL JS change Refs https://github.com/mapbox/mapbox-gl-js/pull/6429 * Fuller fix * Update mapbox-gl-js --- .../conversion/data_driven_property_value.hpp | 2 -- include/mbgl/style/expression/let.hpp | 3 ++ mapbox-gl-js | 2 +- platform/node/test/ignores.json | 2 ++ src/mbgl/style/expression/parsing_context.cpp | 41 ++++++++++++++-------- 5 files changed, 33 insertions(+), 17 deletions(-) diff --git a/include/mbgl/style/conversion/data_driven_property_value.hpp b/include/mbgl/style/conversion/data_driven_property_value.hpp index 5f4395f35a..b4297816df 100644 --- a/include/mbgl/style/conversion/data_driven_property_value.hpp +++ b/include/mbgl/style/conversion/data_driven_property_value.hpp @@ -44,8 +44,6 @@ struct Converter> { } else if (!featureConstant && !zoomConstant) { return DataDrivenPropertyValue(CompositeFunction(std::move(*expression))); } else { - // If an expression is neither zoom- nor feature-dependent, it - // should have been reduced to a Literal when it was parsed. auto literal = dynamic_cast(expression->get()); assert(literal); optional constant = fromExpressionValue(literal->getValue()); diff --git a/include/mbgl/style/expression/let.hpp b/include/mbgl/style/expression/let.hpp index 75d2adda62..d0210d8bba 100644 --- a/include/mbgl/style/expression/let.hpp +++ b/include/mbgl/style/expression/let.hpp @@ -70,6 +70,9 @@ public: mbgl::Value serialize() const override; std::string getOperator() const override { return "var"; } + + const std::shared_ptr& getBoundExpression() const { return value; } + private: std::string name; std::shared_ptr value; diff --git a/mapbox-gl-js b/mapbox-gl-js index bbee7e5169..b01859294f 160000 --- a/mapbox-gl-js +++ b/mapbox-gl-js @@ -1 +1 @@ -Subproject commit bbee7e51695184e9b3c0ecb74d314c4ab73d4c27 +Subproject commit b01859294fec88111601028a539e45be1f088666 diff --git a/platform/node/test/ignores.json b/platform/node/test/ignores.json index 79debab587..e0c2475b75 100644 --- a/platform/node/test/ignores.json +++ b/platform/node/test/ignores.json @@ -1,6 +1,8 @@ { "query-tests/circle-pitch-scale/viewport-inside-align-map": "https://github.com/mapbox/mapbox-gl-native/issues/10615", "query-tests/circle-pitch-scale/viewport-inside-align-viewport": "https://github.com/mapbox/mapbox-gl-native/issues/10615", + "query-tests/edge-cases/box-cutting-antimeridian-z0": "https://github.com/mapbox/mapbox-gl-native/issues/11607", + "query-tests/edge-cases/null-island": "https://github.com/mapbox/mapbox-gl-native/issues/11607", "query-tests/geometry/multilinestring": "needs investigation", "query-tests/geometry/multipolygon": "needs investigation", "query-tests/geometry/polygon": "needs investigation", diff --git a/src/mbgl/style/expression/parsing_context.cpp b/src/mbgl/style/expression/parsing_context.cpp index 364c3f740a..713c0b5d27 100644 --- a/src/mbgl/style/expression/parsing_context.cpp +++ b/src/mbgl/style/expression/parsing_context.cpp @@ -32,23 +32,36 @@ namespace style { namespace expression { bool isConstant(const Expression& expression) { - if (dynamic_cast(&expression)) { - return false; + if (auto varExpression = dynamic_cast(&expression)) { + return isConstant(*varExpression->getBoundExpression()); } - + if (auto compound = dynamic_cast(&expression)) { if (compound->getName() == "error") { return false; } } + + bool isTypeAnnotation = dynamic_cast(&expression) || + dynamic_cast(&expression) || + dynamic_cast(&expression); - bool literalArgs = true; + bool childrenConstant = true; expression.eachChild([&](const Expression& child) { - if (!dynamic_cast(&child)) { - literalArgs = false; + // We can _almost_ assume that if `expressions` children are constant, + // they would already have been evaluated to Literal values when they + // were parsed. Type annotations are the exception, because they might + // have been inferred and added after a child was parsed. + + // So we recurse into isConstant() for the children of type annotations, + // but otherwise simply check whether they are Literals. + if (isTypeAnnotation) { + childrenConstant = childrenConstant && isConstant(child); + } else { + childrenConstant = childrenConstant && dynamic_cast(&child); } }); - if (!literalArgs) { + if (!childrenConstant) { return false; } @@ -141,13 +154,13 @@ ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption return parsed; } - if (expected) { - auto array = [&](std::unique_ptr expression) { - std::vector> args; - args.push_back(std::move(expression)); - return args; - }; + auto array = [&](std::unique_ptr expression) { + std::vector> args; + args.push_back(std::move(expression)); + return args; + }; + if (expected) { const type::Type actual = (*parsed)->getType(); if ((*expected == type::String || *expected == type::Number || *expected == type::Boolean || *expected == type::Object) && actual == type::Value) { if (typeAnnotationOption == includeTypeAnnotations) { @@ -169,7 +182,7 @@ ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption } } - // If an expression's arguments are all literals, we can evaluate + // If an expression's arguments are all constant, we can evaluate // it immediately and replace it with a literal value in the // parsed result. if (!dynamic_cast(parsed->get()) && isConstant(**parsed)) { -- cgit v1.2.1