summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAnand Thakker <anandthakker@users.noreply.github.com>2018-04-09 17:59:18 -0400
committerGitHub <noreply@github.com>2018-04-09 17:59:18 -0400
commit8d26f7409106c94f37689d206f9a68aac57572a4 (patch)
treefa1dea3966bc88b76c4877663160b354ac5a01ab
parentc1dabbde8d58143b1f7f64eddeb75c06ab417d32 (diff)
downloadqtlocation-mapboxgl-8d26f7409106c94f37689d206f9a68aac57572a4.tar.gz
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
-rw-r--r--include/mbgl/style/conversion/data_driven_property_value.hpp2
-rw-r--r--include/mbgl/style/expression/let.hpp3
m---------mapbox-gl-js0
-rw-r--r--platform/node/test/ignores.json2
-rw-r--r--src/mbgl/style/expression/parsing_context.cpp41
5 files changed, 32 insertions, 16 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<DataDrivenPropertyValue<T>> {
} else if (!featureConstant && !zoomConstant) {
return DataDrivenPropertyValue<T>(CompositeFunction<T>(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<Literal*>(expression->get());
assert(literal);
optional<T> constant = fromExpressionValue<T>(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<Expression>& getBoundExpression() const { return value; }
+
private:
std::string name;
std::shared_ptr<Expression> value;
diff --git a/mapbox-gl-js b/mapbox-gl-js
-Subproject bbee7e51695184e9b3c0ecb74d314c4ab73d4c2
+Subproject b01859294fec88111601028a539e45be1f08866
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<const Var*>(&expression)) {
- return false;
+ if (auto varExpression = dynamic_cast<const Var*>(&expression)) {
+ return isConstant(*varExpression->getBoundExpression());
}
-
+
if (auto compound = dynamic_cast<const CompoundExpressionBase*>(&expression)) {
if (compound->getName() == "error") {
return false;
}
}
+
+ bool isTypeAnnotation = dynamic_cast<const Coercion*>(&expression) ||
+ dynamic_cast<const Assertion*>(&expression) ||
+ dynamic_cast<const ArrayAssertion*>(&expression);
- bool literalArgs = true;
+ bool childrenConstant = true;
expression.eachChild([&](const Expression& child) {
- if (!dynamic_cast<const Literal*>(&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<const Literal*>(&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> expression) {
- std::vector<std::unique_ptr<Expression>> args;
- args.push_back(std::move(expression));
- return args;
- };
+ auto array = [&](std::unique_ptr<Expression> expression) {
+ std::vector<std::unique_ptr<Expression>> 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<Literal *>(parsed->get()) && isConstant(**parsed)) {