Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Lambdas in Painless to be Able to Use Top-Level Variables Such as params and doc #21635

Merged
merged 3 commits into from
Nov 17, 2016
Merged

Fix Lambdas in Painless to be Able to Use Top-Level Variables Such as params and doc #21635

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7dad654
Add a utility comparison method to support Kibana using a script in
jdconrad Nov 17, 2016
19cdc49
Removed unnecessary compare function. Can be built in the script.
jdconrad Nov 17, 2016
5581318
Rename test method.
jdconrad Nov 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 45 additions & 32 deletions modules/lang-painless/src/main/java/org/elasticsearch/painless/Locals.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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