Skip to content

Commit

Permalink
Fix reserved variable availability in lambdas in Painless
Browse files Browse the repository at this point in the history
  • Loading branch information
@jdconrad
jdconrad committed Nov 17, 2016
1 parent 151d6b7 commit e2dae05
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 66 deletions.
77 changes: 45 additions & 32 deletions modules/lang-painless/src/main/java/org/elasticsearch/painless/Locals.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
* Tracks user defined methods and variables across compilation phases.
*/
public final class Locals {

/** Reserved word: params map parameter */
public static final String PARAMS = "params";
/** Reserved word: Lucene scorer parameter */
Expand All @@ -53,25 +53,35 @@ public final class Locals {
public static final String THIS = "#this";
/** Reserved word: unused */
public static final String DOC = "doc";

/** Map of always reserved keywords */
public static final Set<String> KEYWORDS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
THIS,PARAMS,SCORER,DOC,VALUE,SCORE,CTX,LOOP

/** Map of always reserved keywords for the main scope */
public static final Set<String> MAIN_KEYWORDS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
THIS,PARAMS,SCORER,DOC,VALUE,SCORE,CTX,LOOP
)));

/** Map of always reserved keywords for a function scope */
public static final Set<String> FUNCTION_KEYWORDS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
THIS,LOOP
)));


/** Map of always reserved keywords for a lambda scope */
public static final Set<String> LAMBDA_KEYWORDS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
THIS,LOOP
)));

/** Creates a new local variable scope (e.g. loop) inside the current scope */
public static Locals newLocalScope(Locals currentScope) {
return new Locals(currentScope);
}
/**

/**
* Creates a new lambda scope inside the current scope
* <p>
* This is just like {@link #newFunctionScope}, except the captured parameters are made read-only.
*/
public static Locals newLambdaScope(Locals programScope, Type returnType, List<Parameter> parameters,
public static Locals newLambdaScope(Locals programScope, Type returnType, List<Parameter> parameters,
int captureCount, int maxLoopCounter) {
Locals locals = new Locals(programScope, returnType);
Locals locals = new Locals(programScope, returnType, LAMBDA_KEYWORDS);
for (int i = 0; i < parameters.size(); i++) {
Parameter parameter = parameters.get(i);
// TODO: allow non-captures to be r/w:
Expand All @@ -87,10 +97,10 @@ public static Locals newLambdaScope(Locals programScope, Type returnType, List<P
}
return locals;
}

/** Creates a new function scope inside the current scope */
public static Locals newFunctionScope(Locals programScope, Type returnType, List<Parameter> parameters, int maxLoopCounter) {
Locals locals = new Locals(programScope, returnType);
Locals locals = new Locals(programScope, returnType, FUNCTION_KEYWORDS);
for (Parameter parameter : parameters) {
locals.addVariable(parameter.location, parameter.type, parameter.name, false);
}
Expand All @@ -100,10 +110,10 @@ public static Locals newFunctionScope(Locals programScope, Type returnType, List
}
return locals;
}

/** Creates a new main method scope */
public static Locals newMainMethodScope(Locals programScope, boolean usesScore, boolean usesCtx, int maxLoopCounter) {
Locals locals = new Locals(programScope, Definition.OBJECT_TYPE);
Locals locals = new Locals(programScope, Definition.OBJECT_TYPE, MAIN_KEYWORDS);
// This reference. Internal use only.
locals.defineVariable(null, Definition.getType("Object"), THIS, true);

Expand Down Expand Up @@ -137,16 +147,16 @@ public static Locals newMainMethodScope(Locals programScope, boolean usesScore,
}
return locals;
}

/** Creates a new program scope: the list of methods. It is the parent for all methods */
public static Locals newProgramScope(Collection<Method> methods) {
Locals locals = new Locals(null, null);
Locals locals = new Locals(null, null, null);
for (Method method : methods) {
locals.addMethod(method);
}
return locals;
}

/** Checks if a variable exists or not, in this scope or any parents. */
public boolean hasVariable(String name) {
Variable variable = lookupVariable(null, name);
Expand All @@ -158,7 +168,7 @@ public boolean hasVariable(String name) {
}
return false;
}

/** Accesses a variable. This will throw IAE if the variable does not exist */
public Variable getVariable(Location location, String name) {
Variable variable = lookupVariable(location, name);
Expand All @@ -170,7 +180,7 @@ public Variable getVariable(Location location, String name) {
}
throw location.createError(new IllegalArgumentException("Variable [" + name + "] is not defined."));
}

/** Looks up a method. Returns null if the method does not exist. */
public Method getMethod(MethodKey key) {
Method method = lookupMethod(key);
Expand All @@ -182,23 +192,23 @@ public Method getMethod(MethodKey key) {
}
return null;
}

/** Creates a new variable. Throws IAE if the variable has already been defined (even in a parent) or reserved. */
public Variable addVariable(Location location, Type type, String name, boolean readonly) {
if (hasVariable(name)) {
throw location.createError(new IllegalArgumentException("Variable [" + name + "] is already defined."));
}
if (KEYWORDS.contains(name)) {
if (keywords.contains(name)) {
throw location.createError(new IllegalArgumentException("Variable [" + name + "] is reserved."));
}
return defineVariable(location, type, name, readonly);
}

/** Return type of this scope (e.g. int, if inside a function that returns int) */
public Type getReturnType() {
return returnType;
}

/** Returns the top-level program scope. */
public Locals getProgramScope() {
Locals locals = this;
Expand All @@ -207,13 +217,15 @@ public Locals getProgramScope() {
}
return locals;
}

///// private impl

// parent scope
private final Locals parent;
// return type of this scope
private final Type returnType;
// keywords for this scope
private final Set<String> keywords;
// next slot number to assign
private int nextSlotNumber;
// variable name -> variable
Expand All @@ -225,15 +237,16 @@ public Locals getProgramScope() {
* Create a new Locals
*/
private Locals(Locals parent) {
this(parent, parent.getReturnType());
this(parent, parent.returnType, parent.keywords);
}

/**
* Create a new Locals with specified return type
*/
private Locals(Locals parent, Type returnType) {
private Locals(Locals parent, Type returnType, Set<String> keywords) {
this.parent = parent;
this.returnType = returnType;
this.keywords = keywords;
if (parent == null) {
this.nextSlotNumber = 0;
} else {
Expand Down Expand Up @@ -262,7 +275,7 @@ private Method lookupMethod(MethodKey key) {
return methods.get(key);
}


/** Defines a variable at this scope internally. */
private Variable defineVariable(Location location, Type type, String name, boolean readonly) {
if (variables == null) {
Expand All @@ -273,7 +286,7 @@ private Variable defineVariable(Location location, Type type, String name, boole
nextSlotNumber += type.type.getSize();
return variable;
}

private void addMethod(Method method) {
if (methods == null) {
methods = new HashMap<>();
Expand All @@ -293,20 +306,20 @@ public static final class Variable {
public final Type type;
public final boolean readonly;
private final int slot;

public Variable(Location location, String name, Type type, int slot, boolean readonly) {
this.location = location;
this.name = name;
this.type = type;
this.slot = slot;
this.readonly = readonly;
}

public int getSlot() {
return slot;
}
}

public static final class Parameter {
public final Location location;
public final String name;
Expand Down
7 changes: 2 additions & 5 deletions modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,14 @@
*/
public final class SFunction extends AStatement {
public static final class FunctionReserved implements Reserved {
public static final String THIS = "#this";
public static final String LOOP = "#loop";

private int maxLoopCounter = 0;

public void markReserved(String name) {
// Do nothing.
}

public boolean isReserved(String name) {
return name.equals(THIS) || name.equals(LOOP);
return Locals.FUNCTION_KEYWORDS.contains(name);
}

@Override
Expand Down Expand Up @@ -173,7 +170,7 @@ void analyze(Locals locals) {
}

if (reserved.getMaxLoopCounter() > 0) {
loop = locals.getVariable(null, FunctionReserved.LOOP);
loop = locals.getVariable(null, Locals.LOOP);
}
}

Expand Down
2 changes: 1 addition & 1 deletion modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public void markReserved(String name) {

@Override
public boolean isReserved(String name) {
return Locals.KEYWORDS.contains(name);
return Locals.MAIN_KEYWORDS.contains(name);
}

public boolean usesScore() {
Expand Down
Loading

0 comments on commit e2dae05

Please sign in to comment.