Fix constant folding

src/style-spec/expression/definitions/let.js

@@ -17,10 +17,7 @@ class Let implements Expression {
     }
 
     evaluate(ctx: EvaluationContext) {
-        ctx.pushScope(this.bindings);
-        const result = this.result.evaluate(ctx);
-        ctx.popScope();
-        return result;
+        return this.result.evaluate(ctx);
     }
 
     eachChild(fn: (Expression) => void) {

src/style-spec/expression/definitions/var.js

@@ -8,10 +8,12 @@ import type EvaluationContext  from '../evaluation_context';
 class Var implements Expression {
     type: Type;
     name: string;
+    boundExpression: Expression;
 
-    constructor(name: string, type: Type) {
-        this.type = type;
+    constructor(name: string, boundExpression: Expression) {
+        this.type = boundExpression.type;
         this.name = name;
+        this.boundExpression = boundExpression;
     }
 
     static parse(args: Array<mixed>, context: ParsingContext) {
@@ -23,11 +25,11 @@ class Var implements Expression {
             return context.error(`Unknown variable "${name}". Make sure "${name}" has been bound in an enclosing "let" expression before using it.`, 1);
         }
 
-        return new Var(name, context.scope.get(name).type);
+        return new Var(name, context.scope.get(name));
     }
 
     evaluate(ctx: EvaluationContext) {
-        return ctx.scope.get(this.name).evaluate(ctx);
+        return this.boundExpression.evaluate(ctx);
     }
 
     eachChild() {}

src/style-spec/expression/evaluation_context.js

@@ -1,24 +1,18 @@
 // @flow
 
-import assert from 'assert';
-
-import Scope from './scope';
 import { Color } from './values';
 
 import type { Feature, GlobalProperties } from './index';
-import type { Expression } from './expression';
 
 const geometryTypes = ['Unknown', 'Point', 'LineString', 'Polygon'];
 
 class EvaluationContext {
     globals: GlobalProperties;
     feature: ?Feature;
 
-    scope: Scope;
     _parseColorCache: {[string]: ?Color};
 
     constructor() {
-        this.scope = new Scope();
         this._parseColorCache = {};
     }
 
@@ -34,15 +28,6 @@ class EvaluationContext {
         return this.feature && this.feature.properties || {};
     }
 
-    pushScope(bindings: Array<[string, Expression]>) {
-        this.scope = this.scope.concat(bindings);
-    }
-
-    popScope() {
-        assert(this.scope.parent);
-        this.scope = (this.scope.parent: any);
-    }
-
     parseColor(input: string): ?Color {
         let cached = this._parseColorCache[input];
         if (!cached) {

src/style-spec/expression/parsing_context.js

@@ -63,33 +63,36 @@ class ParsingContext {
         bindings?: Array<[string, Expression]>,
         options: {omitTypeAnnotations?: boolean} = {}
     ): ?Expression {
-        let context = this;
         if (index) {
-            context = context.concat(index, expectedType, bindings);
+            return this.concat(index, expectedType, bindings)._parse(expr, options);
         }
+        return this._parse(expr, options);
+    }
+
+    _parse(expr: mixed, options: {omitTypeAnnotations?: boolean}): ?Expression {
 
         if (expr === null || typeof expr === 'string' || typeof expr === 'boolean' || typeof expr === 'number') {
             expr = ['literal', expr];
         }
 
         if (Array.isArray(expr)) {
             if (expr.length === 0) {
-                return context.error(`Expected an array with at least one element. If you wanted a literal array, use ["literal", []].`);
+                return this.error(`Expected an array with at least one element. If you wanted a literal array, use ["literal", []].`);
             }
 
             const op = expr[0];
             if (typeof op !== 'string') {
-                context.error(`Expression name must be a string, but found ${typeof op} instead. If you wanted a literal array, use ["literal", [...]].`, 0);
+                this.error(`Expression name must be a string, but found ${typeof op} instead. If you wanted a literal array, use ["literal", [...]].`, 0);
                 return null;
             }
 
-            const Expr = context.registry[op];
+            const Expr = this.registry[op];
             if (Expr) {
-                let parsed = Expr.parse(expr, context);
+                let parsed = Expr.parse(expr, this);
                 if (!parsed) return null;
 
-                if (context.expectedType) {
-                    const expected = context.expectedType;
+                if (this.expectedType) {
+                    const expected = this.expectedType;
                     const actual = parsed.type;
 
                     // When we expect a number, string, boolean, or array but
@@ -109,7 +112,7 @@ class ParsingContext {
                         if (!options.omitTypeAnnotations) {
                             parsed = new Coercion(expected, [parsed]);
                         }
-                    } else if (context.checkSubtype(context.expectedType, parsed.type)) {
+                    } else if (this.checkSubtype(this.expectedType, parsed.type)) {
                         return null;
                     }
                 }
@@ -122,21 +125,21 @@ class ParsingContext {
                     try {
                         parsed = new Literal(parsed.type, parsed.evaluate(ec));
                     } catch (e) {
-                        context.error(e.message);
+                        this.error(e.message);
                         return null;
                     }
                 }
 
                 return parsed;
             }
 
-            return context.error(`Unknown expression "${op}". If you wanted a literal array, use ["literal", [...]].`, 0);
+            return this.error(`Unknown expression "${op}". If you wanted a literal array, use ["literal", [...]].`, 0);
         } else if (typeof expr === 'undefined') {
-            return context.error(`'undefined' value invalid. Use null instead.`);
+            return this.error(`'undefined' value invalid. Use null instead.`);
         } else if (typeof expr === 'object') {
-            return context.error(`Bare objects invalid. Use ["literal", {...}] instead.`);
+            return this.error(`Bare objects invalid. Use ["literal", {...}] instead.`);
         } else {
-            return context.error(`Expected an array, but found ${typeof expr} instead.`);
+            return this.error(`Expected an array, but found ${typeof expr} instead.`);
         }
     }
 
@@ -187,16 +190,31 @@ export default ParsingContext;
 
 function isConstant(expression: Expression) {
     if (expression instanceof Var) {
-        return false;
+        return isConstant(expression.boundExpression);
     } else if (expression instanceof CompoundExpression && expression.name === 'error') {
         return false;
     }
 
-    let literalArgs = true;
-    expression.eachChild(arg => {
-        if (!(arg instanceof Literal)) { literalArgs = false; }
+    const isTypeAnnotation = expression instanceof Coercion ||
+        expression instanceof Assertion ||
+        expression instanceof ArrayAssertion;
+
+    let childrenConstant = true;
+    expression.eachChild(child => {
+        // 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 && child instanceof Literal;
+        }
     });
-    if (!literalArgs) {
+    if (!childrenConstant) {
         return false;
     }
 

test/integration/expression-tests/acos/basic/test.json

@@ -9,6 +9,6 @@
       "type": "number"
     },
     "outputs": [1.04719],
-    "serialized": 1.0471975511965976
+    "serialized": 1.0471975511965979
   }
 }

test/integration/expression-tests/asin/basic/test.json

@@ -9,6 +9,6 @@
       "type": "number"
     },
     "outputs": [0.523598],
-    "serialized": 0.5235987755982988
+    "serialized": 0.5235987755982989
   }
 }

test/integration/expression-tests/constant-folding/to-color-inferred/test.json

@@ -0,0 +1,15 @@
+{
+  "propertySpec": {"type": "color"},
+  "expression": "red",
+  "inputs": [[{}, {}]],
+  "expected": {
+    "compiled": {
+      "result": "success",
+      "isFeatureConstant": true,
+      "isZoomConstant": true,
+      "type": "color"
+    },
+    "outputs": [[1, 0, 0, 1]],
+    "serialized": ["rgba", 255, 0, 0, 1]
+  }
+}

test/integration/expression-tests/constant-folding/var/test.json

@@ -0,0 +1,21 @@
+{
+  "expression": [
+    "let",
+    "a",
+    1,
+    "b",
+    2,
+    ["+", ["+", ["var", "a"], ["var", "b"]], ["var", "a"]]
+  ],
+  "inputs": [[{}, {}]],
+  "expected": {
+    "compiled": {
+      "result": "success",
+      "isFeatureConstant": true,
+      "isZoomConstant": true,
+      "type": "number"
+    },
+    "outputs": [4],
+    "serialized": 4
+  }
+}

test/integration/expression-tests/length/invalid/test.json

@@ -10,7 +10,6 @@
           "error": "Expected argument of type string or array, but found number instead."
         }
       ]
-    },
-    "serialized": ["length", 0]
+    }
   }
 }

test/integration/expression-tests/let/basic/test.json

@@ -2,26 +2,26 @@
   "expression": [
     "let",
     "a",
-    1,
+    ["number", ["get", "a"]],
     "b",
-    2,
+    ["number", ["get", "b"]],
     ["+", ["+", ["var", "a"], ["var", "b"]], ["var", "a"]]
   ],
-  "inputs": [[{}, {}]],
+  "inputs": [[{}, {"properties": {"a": 1, "b": 2}}]],
   "expected": {
     "compiled": {
       "result": "success",
-      "isFeatureConstant": true,
+      "isFeatureConstant": false,
       "isZoomConstant": true,
       "type": "number"
     },
     "outputs": [4],
     "serialized": [
       "let",
       "a",
-      1,
+      ["number", ["get", "a"]],
       "b",
-      2,
+      ["number", ["get", "b"]],
       ["+", ["+", ["var", "a"], ["var", "b"]], ["var", "a"]]
     ]
   }

test/integration/expression-tests/let/nested/test.json

@@ -2,22 +2,22 @@
   "expression": [
     "let",
     "a",
-    1,
+    ["number", ["get", "a"]],
     ["let", "b", ["+", 1, ["var", "a"]], ["+", ["var", "a"], ["var", "b"]]]
   ],
-  "inputs": [[{}, {}]],
+  "inputs": [[{}, {"properties": {"a": 1}}]],
   "expected": {
     "compiled": {
       "result": "success",
-      "isFeatureConstant": true,
+      "isFeatureConstant": false,
       "isZoomConstant": true,
       "type": "number"
     },
     "outputs": [3],
     "serialized": [
       "let",
       "a",
-      1,
+      ["number", ["get", "a"]],
       ["let", "b", ["+", 1, ["var", "a"]], ["+", ["var", "a"], ["var", "b"]]]
     ]
   }

test/integration/expression-tests/let/shadow/test.json

@@ -1,14 +1,24 @@
 {
-  "expression": ["let", "a", 1, ["let", "a", 2, ["var", "a"]]],
-  "inputs": [[{}, {}]],
+  "expression": [
+    "let",
+    "a",
+    ["get", "one"],
+    ["let", "a", ["get", "two"], ["var", "a"]]
+  ],
+  "inputs": [[{}, {"properties": {"one": 1, "two": 2}}]],
   "expected": {
     "compiled": {
       "result": "success",
-      "isFeatureConstant": true,
+      "isFeatureConstant": false,
       "isZoomConstant": true,
-      "type": "number"
+      "type": "value"
     },
     "outputs": [2],
-    "serialized": ["let", "a", 1, ["let", "a", 2, ["var", "a"]]]
+    "serialized": [
+      "let",
+      "a",
+      ["get", "one"],
+      ["let", "a", ["get", "two"], ["var", "a"]]
+    ]
   }
 }

test/integration/expression-tests/let/zoom/test.json

@@ -2,9 +2,9 @@
   "expression": [
     "let",
     "z0_value",
-    10,
+    ["number", ["get", "a"]],
     "z20_value",
-    30,
+    ["number", ["get", "b"]],
     [
       "interpolate",
       ["linear"],
@@ -15,21 +15,21 @@
       ["var", "z20_value"]
     ]
   ],
-  "inputs": [[{"zoom": 10}, {}]],
+  "inputs": [[{"zoom": 10}, {"properties": {"a": 10, "b": 30}}]],
   "expected": {
     "compiled": {
       "result": "success",
-      "isFeatureConstant": true,
+      "isFeatureConstant": false,
       "isZoomConstant": false,
       "type": "number"
     },
     "outputs": [20],
     "serialized": [
       "let",
       "z0_value",
-      10,
+      ["number", ["get", "a"]],
       "z20_value",
-      30,
+      ["number", ["get", "b"]],
       [
         "interpolate",
         ["linear"],