Skip to content

Commit

Permalink
Simplify constant handling in Painless (#52612)
Browse files Browse the repository at this point in the history
This change makes constant handling simpler by doing the following:
* Removes the ir tree node RegexNode entirely in favor or using more 
generic nodes such as ConstantNode and MemberFieldLoadNode 
generated by the user tree node ERegex. This requires a clinit method 
added to the ClassNode, but allows for additional removals. This clinit 
method can also be used for potential optimization passes.
* Removes the Constant class as with the changes to regex nodes it's no 
longer necessary.
* Removes the Globals class from the ir tree write phase. With the previous 
removals, Globals is no longer necessary.
  • Loading branch information
jdconrad authored Feb 25, 2020
1 parent 5ca628a commit 37e3391
Show file tree
Hide file tree
Showing 67 changed files with 518 additions and 517 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ public static int buildAccess(int modifiers, boolean synthetic) {

protected final org.objectweb.asm.ClassWriter classWriter;
protected final ClassVisitor classVisitor;
protected MethodWriter clinitWriter = null;

public ClassWriter(CompilerSettings compilerSettings, BitSet statements, Printer debugStream,
Class<?> baseClass, int classFrames, int classAccess, String className, String[] classInterfaces) {
Expand Down Expand Up @@ -93,30 +92,12 @@ public ClassVisitor getClassVisitor() {
return classVisitor;
}

/**
* Lazy loads the {@link MethodWriter} for clinit, so that if it's not
* necessary the method is never created for the class.
*/
public MethodWriter getClinitWriter() {
if (clinitWriter == null) {
clinitWriter = new MethodWriter(Opcodes.ACC_STATIC, WriterConstants.CLINIT, classVisitor, statements, compilerSettings);
clinitWriter.visitCode();
}

return clinitWriter;
}

public MethodWriter newMethodWriter(int access, Method method) {
return new MethodWriter(access, method, classVisitor, statements, compilerSettings);
}

@Override
public void close() {
if (clinitWriter != null) {
clinitWriter.returnValue();
clinitWriter.endMethod();
}

classVisitor.visitEnd();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import org.elasticsearch.painless.ir.ClassNode;
import org.elasticsearch.painless.ir.FieldNode;
import org.elasticsearch.painless.ir.FunctionNode;
import org.elasticsearch.painless.ir.MemberFieldNode;
import org.elasticsearch.painless.ir.MemberFieldLoadNode;
import org.elasticsearch.painless.ir.ReturnNode;
import org.elasticsearch.painless.ir.StaticNode;
import org.elasticsearch.painless.ir.VariableNode;
Expand Down Expand Up @@ -121,50 +121,50 @@ protected static void injectDefBootstrapMethod(ClassNode classNode) {
callSubNode.setLocation(internalLocation);
callSubNode.setExpressionType(CallSite.class);
callSubNode.setMethod(new PainlessMethod(
DefBootstrap.class.getMethod("bootstrap",
PainlessLookup.class,
FunctionTable.class,
Lookup.class,
String.class,
MethodType.class,
int.class,
int.class,
Object[].class),
DefBootstrap.class,
CallSite.class,
Arrays.asList(
PainlessLookup.class,
FunctionTable.class,
Lookup.class,
String.class,
MethodType.class,
int.class,
int.class,
Object[].class),
null,
null,
null
DefBootstrap.class.getMethod("bootstrap",
PainlessLookup.class,
FunctionTable.class,
Lookup.class,
String.class,
MethodType.class,
int.class,
int.class,
Object[].class),
DefBootstrap.class,
CallSite.class,
Arrays.asList(
PainlessLookup.class,
FunctionTable.class,
Lookup.class,
String.class,
MethodType.class,
int.class,
int.class,
Object[].class),
null,
null,
null
)
);
callSubNode.setBox(DefBootstrap.class);

callNode.setRightNode(callSubNode);

MemberFieldNode memberFieldNode = new MemberFieldNode();
memberFieldNode.setLocation(internalLocation);
memberFieldNode.setExpressionType(PainlessLookup.class);
memberFieldNode.setName("$DEFINITION");
memberFieldNode.setStatic(true);
MemberFieldLoadNode memberFieldLoadNode = new MemberFieldLoadNode();
memberFieldLoadNode.setLocation(internalLocation);
memberFieldLoadNode.setExpressionType(PainlessLookup.class);
memberFieldLoadNode.setName("$DEFINITION");
memberFieldLoadNode.setStatic(true);

callSubNode.addArgumentNode(memberFieldNode);
callSubNode.addArgumentNode(memberFieldLoadNode);

memberFieldNode = new MemberFieldNode();
memberFieldNode.setLocation(internalLocation);
memberFieldNode.setExpressionType(FunctionTable.class);
memberFieldNode.setName("$FUNCTIONS");
memberFieldNode.setStatic(true);
memberFieldLoadNode = new MemberFieldLoadNode();
memberFieldLoadNode.setLocation(internalLocation);
memberFieldLoadNode.setExpressionType(FunctionTable.class);
memberFieldLoadNode.setName("$FUNCTIONS");
memberFieldLoadNode.setStatic(true);

callSubNode.addArgumentNode(memberFieldNode);
callSubNode.addArgumentNode(memberFieldLoadNode);

VariableNode variableNode = new VariableNode();
variableNode.setLocation(internalLocation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import org.elasticsearch.painless.ir.FieldNode;
import org.elasticsearch.painless.ir.FunctionNode;
import org.elasticsearch.painless.ir.MemberCallNode;
import org.elasticsearch.painless.ir.MemberFieldNode;
import org.elasticsearch.painless.ir.MemberFieldLoadNode;
import org.elasticsearch.painless.ir.ReturnNode;
import org.elasticsearch.painless.ir.StatementNode;
import org.elasticsearch.painless.ir.StaticNode;
Expand Down Expand Up @@ -131,13 +131,13 @@ protected static void injectStaticFieldsAndGetters(ClassNode classNode) {

blockNode.addStatementNode(returnNode);

MemberFieldNode memberFieldNode = new MemberFieldNode();
memberFieldNode.setLocation(internalLocation);
memberFieldNode.setExpressionType(String.class);
memberFieldNode.setName("$NAME");
memberFieldNode.setStatic(true);
MemberFieldLoadNode memberFieldLoadNode = new MemberFieldLoadNode();
memberFieldLoadNode.setLocation(internalLocation);
memberFieldLoadNode.setExpressionType(String.class);
memberFieldLoadNode.setName("$NAME");
memberFieldLoadNode.setStatic(true);

returnNode.setExpressionNode(memberFieldNode);
returnNode.setExpressionNode(memberFieldLoadNode);

functionNode = new FunctionNode();
functionNode.setLocation(internalLocation);
Expand All @@ -162,13 +162,13 @@ protected static void injectStaticFieldsAndGetters(ClassNode classNode) {

blockNode.addStatementNode(returnNode);

memberFieldNode = new MemberFieldNode();
memberFieldNode.setLocation(internalLocation);
memberFieldNode.setExpressionType(String.class);
memberFieldNode.setName("$SOURCE");
memberFieldNode.setStatic(true);
memberFieldLoadNode = new MemberFieldLoadNode();
memberFieldLoadNode.setLocation(internalLocation);
memberFieldLoadNode.setExpressionType(String.class);
memberFieldLoadNode.setName("$SOURCE");
memberFieldLoadNode.setStatic(true);

returnNode.setExpressionNode(memberFieldNode);
returnNode.setExpressionNode(memberFieldLoadNode);

functionNode = new FunctionNode();
functionNode.setLocation(internalLocation);
Expand All @@ -193,13 +193,13 @@ protected static void injectStaticFieldsAndGetters(ClassNode classNode) {

blockNode.addStatementNode(returnNode);

memberFieldNode = new MemberFieldNode();
memberFieldNode.setLocation(internalLocation);
memberFieldNode.setExpressionType(BitSet.class);
memberFieldNode.setName("$STATEMENTS");
memberFieldNode.setStatic(true);
memberFieldLoadNode = new MemberFieldLoadNode();
memberFieldLoadNode.setLocation(internalLocation);
memberFieldLoadNode.setExpressionType(BitSet.class);
memberFieldLoadNode.setName("$STATEMENTS");
memberFieldLoadNode.setStatic(true);

returnNode.setExpressionNode(memberFieldNode);
returnNode.setExpressionNode(memberFieldLoadNode);
}

// convert gets methods to a new set of inserted ir nodes as necessary -
Expand Down Expand Up @@ -338,11 +338,11 @@ protected static void injectSandboxExceptions(FunctionNode functionNode) {
memberCallNode.setLocation(internalLocation);
memberCallNode.setExpressionType(ScriptException.class);
memberCallNode.setLocalFunction(new LocalFunction(
"convertToScriptException",
ScriptException.class,
Arrays.asList(Throwable.class, Map.class),
true,
false
"convertToScriptException",
ScriptException.class,
Arrays.asList(Throwable.class, Map.class),
true,
false
)
);

Expand Down Expand Up @@ -382,17 +382,18 @@ protected static void injectSandboxExceptions(FunctionNode functionNode) {
null,
null,
null
));
)
);

callNode.setRightNode(callSubNode);

MemberFieldNode memberFieldNode = new MemberFieldNode();
memberFieldNode.setLocation(internalLocation);
memberFieldNode.setExpressionType(PainlessLookup.class);
memberFieldNode.setName("$DEFINITION");
memberFieldNode.setStatic(true);
MemberFieldLoadNode memberFieldLoadNode = new MemberFieldLoadNode();
memberFieldLoadNode.setLocation(internalLocation);
memberFieldLoadNode.setExpressionType(PainlessLookup.class);
memberFieldLoadNode.setName("$DEFINITION");
memberFieldLoadNode.setStatic(true);

callSubNode.addArgumentNode(memberFieldNode);
callSubNode.addArgumentNode(memberFieldLoadNode);

for (Class<?> throwable : new Class<?>[] {
PainlessError.class, BootstrapMethodError.class, OutOfMemoryError.class, StackOverflowError.class, Exception.class}) {
Expand Down Expand Up @@ -429,11 +430,11 @@ protected static void injectSandboxExceptions(FunctionNode functionNode) {
memberCallNode.setLocation(internalLocation);
memberCallNode.setExpressionType(ScriptException.class);
memberCallNode.setLocalFunction(new LocalFunction(
"convertToScriptException",
ScriptException.class,
Arrays.asList(Throwable.class, Map.class),
true,
false
"convertToScriptException",
ScriptException.class,
Arrays.asList(Throwable.class, Map.class),
true,
false
)
);

Expand Down Expand Up @@ -470,7 +471,8 @@ protected static void injectSandboxExceptions(FunctionNode functionNode) {
null,
null,
null
));
)
);

callNode.setRightNode(callSubNode);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

import org.elasticsearch.painless.ClassWriter;
import org.elasticsearch.painless.DefBootstrap;
import org.elasticsearch.painless.Globals;
import org.elasticsearch.painless.MethodWriter;
import org.elasticsearch.painless.Operation;
import org.elasticsearch.painless.lookup.PainlessCast;
Expand Down Expand Up @@ -114,7 +113,7 @@ public PainlessCast getBack() {
/* ---- end node data ---- */

@Override
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
protected void write(ClassWriter classWriter, MethodWriter methodWriter, ScopeTable scopeTable) {
methodWriter.writeDebugInfo(location);

// For the case where the assignment represents a String concatenation
Expand All @@ -129,18 +128,18 @@ protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals
}

// call the setup method on the lhs to prepare for a load/store operation
getLeftNode().setup(classWriter, methodWriter, globals, scopeTable);
getLeftNode().setup(classWriter, methodWriter, scopeTable);

if (cat) {
// Handle the case where we are doing a compound assignment
// representing a String concatenation.

methodWriter.writeDup(getLeftNode().accessElementCount(), catElementStackSize); // dup the top element and insert it
// before concat helper on stack
getLeftNode().load(classWriter, methodWriter, globals, scopeTable); // read the current lhs's value
getLeftNode().load(classWriter, methodWriter, scopeTable); // read the current lhs's value
methodWriter.writeAppendStrings(getLeftNode().getExpressionType()); // append the lhs's value using the StringBuilder

getRightNode().write(classWriter, methodWriter, globals, scopeTable); // write the bytecode for the rhs
getRightNode().write(classWriter, methodWriter, scopeTable); // write the bytecode for the rhs

// check to see if the rhs has already done a concatenation
if (getRightNode() instanceof BinaryMathNode == false || ((BinaryMathNode)getRightNode()).getCat() == false) {
Expand All @@ -158,14 +157,15 @@ protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals
}

// store the lhs's value from the stack in its respective variable/field/array
getLeftNode().store(classWriter, methodWriter, globals, scopeTable);
getLeftNode().store(classWriter, methodWriter, scopeTable);
} else if (operation != null) {
// Handle the case where we are doing a compound assignment that
// does not represent a String concatenation.

methodWriter.writeDup(getLeftNode().accessElementCount(), 0); // if necessary, dup the previous lhs's value
// to be both loaded from and stored to
getLeftNode().load(classWriter, methodWriter, globals, scopeTable); // load the current lhs's value

getLeftNode().load(classWriter, methodWriter, scopeTable); // load the current lhs's value

if (read && post) {
// dup the value if the lhs is also read from and is a post increment
Expand All @@ -175,7 +175,8 @@ protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals

methodWriter.writeCast(there); // if necessary cast the current lhs's value
// to the promotion type between the lhs and rhs types
getRightNode().write(classWriter, methodWriter, globals, scopeTable); // write the bytecode for the rhs

getRightNode().write(classWriter, methodWriter, scopeTable); // write the bytecode for the rhs

// XXX: fix these types, but first we need def compound assignment tests.
// its tricky here as there are possibly explicit casts, too.
Expand All @@ -196,11 +197,11 @@ protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals
}

// store the lhs's value from the stack in its respective variable/field/array
getLeftNode().store(classWriter, methodWriter, globals, scopeTable);
getLeftNode().store(classWriter, methodWriter, scopeTable);
} else {
// Handle the case for a simple write.

getRightNode().write(classWriter, methodWriter, globals, scopeTable); // write the bytecode for the rhs rhs
getRightNode().write(classWriter, methodWriter, scopeTable); // write the bytecode for the rhs rhs

if (read) {
// dup the value if the lhs is also read from
Expand All @@ -209,7 +210,7 @@ protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals
}

// store the lhs's value from the stack in its respective variable/field/array
getLeftNode().store(classWriter, methodWriter, globals, scopeTable);
getLeftNode().store(classWriter, methodWriter, scopeTable);
}
}
}
Loading

0 comments on commit 37e3391

Please sign in to comment.