You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by he...@apache.org on 2019/11/04 10:32:06 UTC
[commons-jexl] branch master updated: JEXL-307: added hoisted
variables stacks to solve shading and frame state preservation Task
#JEXL-307 - Variable redeclaration option
This is an automated email from the ASF dual-hosted git repository.
henrib pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
The following commit(s) were added to refs/heads/master by this push:
new 94e6097 JEXL-307: added hoisted variables stacks to solve shading and frame state preservation Task #JEXL-307 - Variable redeclaration option
94e6097 is described below
commit 94e6097c7bce16996e6bfb94aa4ef7a3854c115a
Author: Henri Biestro <hb...@gmail.com>
AuthorDate: Mon Nov 4 11:31:54 2019 +0100
JEXL-307: added hoisted variables stacks to solve shading and frame state preservation
Task #JEXL-307 - Variable redeclaration option
---
.../org/apache/commons/jexl3/internal/Frame.java | 4 +-
.../apache/commons/jexl3/internal/Interpreter.java | 28 +++----
.../commons/jexl3/internal/LexicalFrame.java | 87 ++++++++++++++++++++++
.../commons/jexl3/internal/LexicalScope.java | 23 ++----
.../org/apache/commons/jexl3/internal/Scope.java | 43 ++++++-----
.../jexl3/internal/TemplateInterpreter.java | 5 +-
.../java/org/apache/commons/jexl3/LambdaTest.java | 11 ++-
.../java/org/apache/commons/jexl3/LexicalTest.java | 22 +++++-
8 files changed, 165 insertions(+), 58 deletions(-)
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Frame.java b/src/main/java/org/apache/commons/jexl3/internal/Frame.java
index 8de92e6..cb27170 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Frame.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Frame.java
@@ -90,7 +90,7 @@ public final class Frame {
* @return true if this symbol has been assigned a value, false otherwise
*/
boolean has(int s) {
- return s >= 0 && s < stack.length && stack[s] != Scope.UNDEFINED;
+ return s >= 0 && s < stack.length && stack[s] != Scope.UNDECLARED;
}
/**
@@ -122,5 +122,5 @@ public final class Frame {
}
return this;
}
-
+
}
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
index 6c0d432..a673f56 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
@@ -119,7 +119,7 @@ public class Interpreter extends InterpreterBase {
/** Symbol values. */
protected final Frame frame;
/** Block micro-frames. */
- protected LexicalScope block = null;
+ protected LexicalFrame block = null;
/**
* The thread local interpreter.
@@ -169,6 +169,9 @@ public class Interpreter extends InterpreterBase {
* @throws JexlException if any error occurs during interpretation.
*/
public Object interpret(JexlNode node) {
+ return interpret(node, false);
+ }
+ public Object interpret(JexlNode node, boolean rethrow) {
JexlContext.ThreadLocal tcontext = null;
JexlEngine tjexl = null;
Interpreter tinter = null;
@@ -202,7 +205,7 @@ public class Interpreter extends InterpreterBase {
throw xcancel.clean();
}
} catch (JexlException xjexl) {
- if (!isSilent()) {
+ if (rethrow || !isSilent()) {
throw xjexl.clean();
}
if (logger.isWarnEnabled()) {
@@ -623,12 +626,11 @@ public class Interpreter extends InterpreterBase {
if (!options.isLexical() || cnt <= 0) {
return visitBlock(node, data);
}
- LexicalScope lexical = block;
try {
- block = new LexicalScope(lexical);
+ block = new LexicalFrame(frame, block);
return visitBlock(node, data);
} finally {
- block = lexical;
+ block = block.pop();
}
}
@@ -672,14 +674,14 @@ public class Interpreter extends InterpreterBase {
ASTReference loopReference = (ASTReference) node.jjtGetChild(0);
ASTIdentifier loopVariable = (ASTIdentifier) loopReference.jjtGetChild(0);
final int symbol = loopVariable.getSymbol();
- final LexicalScope lexical = block;
+ final LexicalFrame lexical = block;
if (options.isLexical()) {
// the iteration variable can not be declared in parent block
if (symbol >= 0 && block.hasSymbol(symbol)) {
return redefinedVariable(node, loopVariable.getName());
}
// create lexical frame
- block = new LexicalScope(lexical);
+ block = new LexicalFrame(frame, lexical);
}
Object forEach = null;
try {
@@ -982,13 +984,12 @@ public class Interpreter extends InterpreterBase {
*/
protected Object runClosure(Closure closure, Object data) {
ASTJexlScript script = closure.getScript();
- final LexicalScope lexical = block;
- block = new LexicalScope(frame, null);
+ block = new LexicalFrame(frame, block).declareArgs();
try {
JexlNode body = script.jjtGetChild(script.jjtGetNumChildren() - 1);
- return interpret(body);
+ return interpret(body, true);
} finally {
- block = lexical;
+ block = block.pop();
}
}
@@ -997,8 +998,7 @@ public class Interpreter extends InterpreterBase {
if (script instanceof ASTJexlLambda && !((ASTJexlLambda) script).isTopLevel()) {
return new Closure(this, (ASTJexlLambda) script);
} else {
- final LexicalScope lexical = block;
- block = new LexicalScope(frame, null);
+ block = new LexicalFrame(frame, block).declareArgs();
try {
final int numChildren = script.jjtGetNumChildren();
Object result = null;
@@ -1009,7 +1009,7 @@ public class Interpreter extends InterpreterBase {
}
return result;
} finally {
- block = lexical;
+ block = block.pop();
}
}
}
diff --git a/src/main/java/org/apache/commons/jexl3/internal/LexicalFrame.java b/src/main/java/org/apache/commons/jexl3/internal/LexicalFrame.java
new file mode 100644
index 0000000..1deb14a
--- /dev/null
+++ b/src/main/java/org/apache/commons/jexl3/internal/LexicalFrame.java
@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jexl3.internal;
+
+import java.util.ArrayDeque;
+import java.util.Deque;
+
+/**
+ * The set of valued symbols declared in a lexical scope.
+ * <p>The symbol identifiers are determined by the functional scope.
+ */
+public class LexicalFrame extends LexicalScope {
+ /** The script frame. */
+ private final Frame frame;
+ /** The stack of values in the lexical frame. */
+ private Deque<Object> stack = null;
+ /**
+ * Lexical frame ctor.
+ * @param frame the script frame
+ * @param previous the previous lexical frame
+ */
+ public LexicalFrame(Frame frame, LexicalFrame previous) {
+ super(previous);
+ this.frame = frame;
+ }
+
+ public LexicalFrame declareArgs() {
+ if (frame != null) {
+ int argc = frame.getScope().getArgCount();
+ for(int a = 0; a < argc; ++a) {
+ super.registerSymbol(a);
+ }
+ }
+ return this;
+ }
+
+ @Override
+ public boolean declareSymbol(int symbol) {
+ boolean declared = super.declareSymbol(symbol);
+ if (declared && frame.getScope().isHoistedSymbol(symbol)) {
+ if (stack == null) {
+ stack = new ArrayDeque<Object>() ;
+ }
+ stack.push(symbol);
+ Object value = frame.get(symbol);
+ if (value == null) {
+ value = this;
+ }
+ stack.push(value);
+ }
+ return declared;
+ }
+
+ /**
+ * Pops back values and lexical frame.
+ * @return the previous frame
+ */
+ public LexicalFrame pop() {
+ if (stack != null) {
+ while(!stack.isEmpty()) {
+ Object value = stack.pop();
+ if (value == Scope.UNDECLARED) {
+ value = Scope.UNDEFINED;
+ } else if (value == this) {// || value == Scope.UNDEFINED) {
+ value = null;
+ }
+ int symbol = (Integer) stack.pop();
+ frame.set(symbol, value);
+ }
+ }
+ return (LexicalFrame) previous;
+ }
+}
diff --git a/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java b/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java
index b9fc4bb..a6e6fff 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java
@@ -22,7 +22,7 @@ import java.util.BitSet;
* The set of symbols declared in a lexical scope.
* <p>The symbol identifiers are determined by the functional scope.
*/
-public final class LexicalScope {
+public class LexicalScope {
/** Number of bits in a long. */
protected static final int LONGBITS = 64;
/** The mask of symbols in the frame. */
@@ -32,26 +32,13 @@ public final class LexicalScope {
/** Previous block. */
protected final LexicalScope previous;
- /**
- * Default ctor.
- * @param scope the previous scope
- */
- public LexicalScope(LexicalScope scope) {
- this(null, scope);
- }
/**
* Create a scope.
* @param frame the current execution frame
* @param scope the previous scope
*/
- public LexicalScope(Frame frame, LexicalScope scope) {
- if (frame != null) {
- int argc = frame.getScope().getArgCount();
- for(int a = 0; a < argc; ++a) {
- declareSymbol(a);
- }
- }
+ public LexicalScope(LexicalScope scope) {
previous = scope;
}
@@ -65,7 +52,7 @@ public final class LexicalScope {
}
return moreSymbols;
}
-
+
/**
* Checks whether a symbol has already been declared.
* @param symbol the symbol
@@ -93,6 +80,10 @@ public final class LexicalScope {
}
walk = walk.previous;
}
+ return registerSymbol(symbol);
+ }
+
+ protected final boolean registerSymbol(int symbol) {
if (symbol < LONGBITS) {
if ((symbols & (1L << symbol)) != 0L) {
return false;
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Scope.java b/src/main/java/org/apache/commons/jexl3/internal/Scope.java
index 51cf842..0016b8a 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Scope.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Scope.java
@@ -16,8 +16,10 @@
*/
package org.apache.commons.jexl3.internal;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.LinkedHashMap;
+import java.util.List;
import java.util.Map;
/**
@@ -25,7 +27,15 @@ import java.util.Map;
* <p>This also acts as the functional scope and variable definition store.
* @since 3.0
*/
-public final class Scope {
+public final class Scope {
+ /**
+ * The value of an as-yet undeclared but variable, for instance: x; before var x;.
+ */
+ static final Object UNDECLARED = new Object() {
+ @Override public String toString() {
+ return "??";
+ }
+ };
/**
* The value of a declared but undefined variable, for instance: var x;.
*/
@@ -187,16 +197,16 @@ public final class Scope {
register = namedVariables.size();
namedVariables.put(name, register);
vars += 1;
-// // check if local is redefining hoisted
-// if (parent != null) {
-// Integer pr = parent.getSymbol(name, true);
-// if (pr != null) {
-// if (hoistedVariables == null) {
-// hoistedVariables = new LinkedHashMap<Integer, Integer>();
-// }
-// hoistedVariables.put(register, pr);
-// }
-// }
+ // check if local is redefining hoisted
+ if (parent != null) {
+ Integer pr = parent.getSymbol(name, true);
+ if (pr != null) {
+ if (hoistedVariables == null) {
+ hoistedVariables = new LinkedHashMap<Integer, Integer>();
+ }
+ hoistedVariables.put(register, pr);
+ }
+ }
}
return register;
}
@@ -211,7 +221,7 @@ public final class Scope {
public Frame createFrame(Frame frame, Object...args) {
if (namedVariables != null) {
Object[] arguments = new Object[namedVariables.size()];
- Arrays.fill(arguments, UNDEFINED);
+ Arrays.fill(arguments, UNDECLARED);
if (frame != null && hoistedVariables != null && parent != null) {
for (Map.Entry<Integer, Integer> hoist : hoistedVariables.entrySet()) {
Integer target = hoist.getKey();
@@ -266,7 +276,7 @@ public final class Scope {
public String[] getParameters() {
return getParameters(0);
}
-
+
/**
* Gets this script parameters.
* @param bound number of known bound parameters (curry)
@@ -295,15 +305,14 @@ public final class Scope {
*/
public String[] getLocalVariables() {
if (namedVariables != null && vars > 0) {
- String[] pa = new String[parms - (hoistedVariables == null? 0 : hoistedVariables.size())];
- int p = 0;
+ List<String> locals = new ArrayList<String>(vars);
for (Map.Entry<String, Integer> entry : namedVariables.entrySet()) {
int symnum = entry.getValue();
if (symnum >= parms && (hoistedVariables == null || !hoistedVariables.containsKey(symnum))) {
- pa[p++] = entry.getKey();
+ locals.add(entry.getKey());
}
}
- return pa;
+ return locals.toArray(new String[locals.size()]);
} else {
return EMPTY_STRS;
}
diff --git a/src/main/java/org/apache/commons/jexl3/internal/TemplateInterpreter.java b/src/main/java/org/apache/commons/jexl3/internal/TemplateInterpreter.java
index 295d900..3ae8017 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/TemplateInterpreter.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/TemplateInterpreter.java
@@ -153,8 +153,7 @@ public class TemplateInterpreter extends Interpreter {
}
};
}
- final LexicalScope lexical = block;
- block = new LexicalScope(frame, null);
+ block = new LexicalFrame(frame, block).declareArgs();
try {
// otherwise...
final int numChildren = node.jjtGetNumChildren();
@@ -166,7 +165,7 @@ public class TemplateInterpreter extends Interpreter {
}
return result;
} finally {
- block = lexical;
+ block = block.pop();
}
}
diff --git a/src/test/java/org/apache/commons/jexl3/LambdaTest.java b/src/test/java/org/apache/commons/jexl3/LambdaTest.java
index 930377c..03d9339 100644
--- a/src/test/java/org/apache/commons/jexl3/LambdaTest.java
+++ b/src/test/java/org/apache/commons/jexl3/LambdaTest.java
@@ -168,7 +168,9 @@ public class LambdaTest extends JexlTestCase {
Assert.assertEquals(1, hvars.size());
// declaring a local that overrides hoisted
- strs = "(x)->{ (y)->{ var x; x + y } }";
+ // in 3.1, such a local was considered local
+ // per 3.2, this local is considered hoisted
+ strs = "(x)->{ (y)->{ var z = 169; var x; x + y } }";
s42 = jexl.createScript(strs);
result = s42.execute(ctx, 15);
Assert.assertTrue(result instanceof JexlScript);
@@ -177,7 +179,10 @@ public class LambdaTest extends JexlTestCase {
Assert.assertNotNull(localv);
Assert.assertEquals(1, localv.length);
hvars = s15.getVariables();
- Assert.assertEquals(0, hvars.size());
+ Assert.assertEquals(1, hvars.size());
+ // evidence this is not (strictly) a local since it inherited a hoisted value
+ result = ((JexlScript) s15).execute(ctx, 27);
+ Assert.assertEquals(42, result);
}
@Test
@@ -343,7 +348,7 @@ public class LambdaTest extends JexlTestCase {
Object result = y.execute(null, 2, 3);
Assert.assertEquals(8, result);
}
-
+
// redefining an hoisted var is not resolved correctly in left hand side;
// declare the var in local frame, resolved in local frame instead of parent
// @Test
diff --git a/src/test/java/org/apache/commons/jexl3/LexicalTest.java b/src/test/java/org/apache/commons/jexl3/LexicalTest.java
index 30ff319..fff27bd 100644
--- a/src/test/java/org/apache/commons/jexl3/LexicalTest.java
+++ b/src/test/java/org/apache/commons/jexl3/LexicalTest.java
@@ -230,13 +230,13 @@ public class LexicalTest {
@Test
public void testLexical2a() throws Exception {
runLexical2(true);
- }
-
+ }
+
@Test
public void testLexical2b() throws Exception {
runLexical2(false);
}
-
+
protected void runLexical2(boolean lexical) throws Exception {
JexlEngine jexl = new JexlBuilder().strict(true).lexical(lexical).create();
JexlContext ctxt = new MapContext();
@@ -280,4 +280,20 @@ public class LexicalTest {
String ctl = "<report>\n\n3\n</report>\n";
Assert.assertEquals(ctl, output);
}
+
+ @Test
+ public void testLexical5() throws Exception {
+ JexlEngine jexl = new JexlBuilder().strict(true).lexical(true).create();
+ JexlContext ctxt = new MapContext();
+ JexlScript script;
+ Object result;
+ script = jexl.createScript("var x = 42; var z = 169; var y = () -> { {var x = -42; }; return x; }; y()");
+ try {
+ result = script.execute(ctxt);
+ Assert.assertEquals(42, result);
+ } catch (JexlException xany) {
+ String ww = xany.toString();
+ Assert.fail(ww);
+ }
+ }
}