tei/MapLibreParticleLab
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[core] Omit inferred type annotations for 'coalesce' arguments

Browse Source
This commit is contained in:
John Firebaugh
2018-01-08 14:46:44 -08:00
parent e887cda986
commit 0eb4b5cc0e
4 changed files with 60 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
include/mbgl/style/expression/parsing_context.hpp
View File

@@ -68,20 +68,27 @@ public:
optional<type::Type> getExpected() const { return expected; }
const std::vector<ParsingError>& getErrors() const { return *errors; }
enum TypeAnnotationOption {
includeTypeAnnotations,
omitTypeAnnotations
};
/*
Parse the given style-spec JSON value into an Expression object.
Specifically, this function is responsible for determining the expression
type (either Literal, or the one named in value[0]) and dispatching to the
appropriate ParseXxxx::parse(const V&, ParsingContext) method.
*/
ParseResult parse(const mbgl::style::conversion::Convertible& value);
ParseResult parse(const mbgl::style::conversion::Convertible& value,
TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations);
/*
Parse a child expression.
*/
ParseResult parse(const mbgl::style::conversion::Convertible&,
std::size_t,
optional<type::Type> = {});
optional<type::Type> = {},
TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations);
/*
Parse a child expression.

1
platform/node/test/ignores.json
View File

@@ -1,5 +1,4 @@
{
"expression-tests/coalesce/inference": "https://github.com/mapbox/mapbox-gl-native/issues/10588",
"query-tests/circle-stroke-width/inside": "https://github.com/mapbox/mapbox-gl-native/issues/10307",
"query-tests/geometry/multilinestring": "needs investigation",
"query-tests/geometry/multipolygon": "needs investigation",

22
src/mbgl/style/expression/coalesce.cpp
View File

@@ -1,4 +1,5 @@
#include <mbgl/style/expression/coalesce.hpp>
#include <mbgl/style/expression/check_subtype.hpp>
namespace mbgl {
namespace style {
@@ -36,14 +37,15 @@ ParseResult Coalesce::parse(const Convertible& value, ParsingContext& ctx) {
}
optional<type::Type> outputType;
if (ctx.getExpected() && *ctx.getExpected() != type::Value) {
outputType = ctx.getExpected();
optional<type::Type> expectedType = ctx.getExpected();
if (expectedType && *expectedType != type::Value) {
outputType = expectedType;
}
Coalesce::Args args;
args.reserve(length - 1);
for (std::size_t i = 1; i < length; i++) {
auto parsed = ctx.parse(arrayMember(value, i), i, outputType);
auto parsed = ctx.parse(arrayMember(value, i), i, outputType, ParsingContext::omitTypeAnnotations);
if (!parsed) {
return parsed;
}
@@ -52,9 +54,19 @@ ParseResult Coalesce::parse(const Convertible& value, ParsingContext& ctx) {
}
args.push_back(std::move(*parsed));
}
assert(outputType);
return ParseResult(std::make_unique<Coalesce>(*outputType, std::move(args)));
// Above, we parse arguments without inferred type annotation so that
// they don't produce a runtime error for `null` input, which would
// preempt the desired null-coalescing behavior.
// Thus, if any of our arguments would have needed an annotation, we
// need to wrap the enclosing coalesce expression with it instead.
bool needsAnnotation = expectedType &&
std::any_of(args.begin(), args.end(), [&] (const auto& arg) {
return type::checkSubtype(*expectedType, arg->getType());
});
return ParseResult(std::make_unique<Coalesce>(needsAnnotation ? type::Value : *outputType, std::move(args)));
}
} // namespace expression

59
src/mbgl/style/expression/parsing_context.cpp
View File

@@ -57,12 +57,15 @@ bool isConstant(const Expression& expression) {
using namespace mbgl::style::conversion;
ParseResult ParsingContext::parse(const Convertible& value, std::size_t index_, optional<type::Type> expected_) {
ParseResult ParsingContext::parse(const Convertible& value,
std::size_t index_,
optional<type::Type> expected_,
TypeAnnotationOption typeAnnotationOption) {
ParsingContext child(key + "[" + util::toString(index_) + "]",
errors,
std::move(expected_),
scope);
return child.parse(value);
return child.parse(value, typeAnnotationOption);
}
ParseResult ParsingContext::parse(const Convertible& value, std::size_t index_, optional<type::Type> expected_,
@@ -100,8 +103,7 @@ const ExpressionRegistry& getExpressionRegistry() {
return registry;
}
ParseResult ParsingContext::parse(const Convertible& value)
{
ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption typeAnnotationOption) {
ParseResult parsed;
if (isArray(value)) {
@@ -131,39 +133,44 @@ ParseResult ParsingContext::parse(const Convertible& value)
} else {
parsed = Literal::parse(value, *this);
}
if (!parsed) {
assert(errors->size() > 0);
} else if (expected) {
auto wrapForType = [&](const type::Type& target, std::unique_ptr<Expression> expression) -> std::unique_ptr<Expression> {
return parsed;
}
if (expected) {
auto array = [&](std::unique_ptr<Expression> expression) {
std::vector<std::unique_ptr<Expression>> args;
args.push_back(std::move(expression));
if (target == type::Color) {
return std::make_unique<Coercion>(target, std::move(args));
} else {
return std::make_unique<Assertion>(target, std::move(args));
}
return args;
};
const type::Type actual = (*parsed)->getType();
if (*expected == type::Color && (actual == type::String || actual == type::Value)) {
parsed = wrapForType(type::Color, std::move(*parsed));
if ((*expected == type::String || *expected == type::Number || *expected == type::Boolean) && actual == type::Value) {
if (typeAnnotationOption == includeTypeAnnotations) {
parsed = { std::make_unique<Assertion>(*expected, array(std::move(*parsed))) };
}
} else if (expected->is<type::Array>() && actual == type::Value) {
parsed = { std::make_unique<ArrayAssertion>(expected->get<type::Array>(), std::move(*parsed)) };
} else if ((*expected == type::String || *expected == type::Number || *expected == type::Boolean) && actual == type::Value) {
parsed = wrapForType(*expected, std::move(*parsed));
}
checkType((*parsed)->getType());
if (errors->size() > 0) {
return ParseResult();
if (typeAnnotationOption == includeTypeAnnotations) {
parsed = { std::make_unique<ArrayAssertion>(expected->get<type::Array>(), std::move(*parsed)) };
}
} else if (*expected == type::Color && (actual == type::Value || actual == type::String)) {
if (typeAnnotationOption == includeTypeAnnotations) {
parsed = { std::make_unique<Coercion>(*expected, array(std::move(*parsed))) };
}
} else {
checkType((*parsed)->getType());
if (errors->size() > 0) {
return ParseResult();
}
}
}
// If an expression's arguments are all literals, we can evaluate
// it immediately and replace it with a literal value in the
// parsed result.
if (parsed && !dynamic_cast<Literal *>(parsed->get()) && isConstant(**parsed)) {
if (!dynamic_cast<Literal *>(parsed->get()) && isConstant(**parsed)) {
EvaluationContext params(nullptr);
EvaluationResult evaluated((*parsed)->evaluate(params));
if (!evaluated) {
@@ -185,7 +192,7 @@ ParseResult ParsingContext::parse(const Convertible& value)
}
// if this is the root expression, enforce constraints on the use ["zoom"].
if (key.size() == 0 && parsed && !isZoomConstant(**parsed)) {
if (key.size() == 0 && !isZoomConstant(**parsed)) {
optional<variant<const InterpolateBase*, const Step*, ParsingError>> zoomCurve = findZoomCurve(parsed->get());
if (!zoomCurve) {
error(R"("zoom" expression may only be used as input to a top-level "step" or "interpolate" expression.)");