Skip to content

Commit

Permalink
Remove toString from Painless user tree nodes (elastic#54117)
Browse files Browse the repository at this point in the history
* remove isNull from AExpression

* remove explicit cast optimization

* remove modification of semantic tree for casting

* remove ECast node

* start of input/output in expressions

* partial change for input and output in expression nodes

* add input/output objects for expressions

* fix shift bug in EBinary

* add input/output to statements

* response to pr comment

* updated expression nodes to remove member state

* update statements to remove most mutable state

* fix def optimization

* remove SField

* fix def optimization in assignment

* move debug stream

* add methods to generate class scope

* move info around

* remove toString on user nodes

* fix imports

* response to pr comments

* response to PR comments

* fix more todos

* fix issue number

* fix space

Co-authored-by: Jack Conradson <[email protected]>
Co-authored-by: Jack Conradson <[email protected]>
3 people authored Mar 27, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 0c52a92 commit 1bb36d7
Showing 72 changed files with 121 additions and 1,490 deletions.
Original file line number Diff line number Diff line change
@@ -207,9 +207,10 @@ private static void addFactoryMethod(Map<String, Class<?>> additionalClasses, Cl
* @return The ScriptRoot used to compile
*/
ScriptRoot compile(Loader loader, String name, String source, CompilerSettings settings) {
String scriptName = Location.computeSourceName(name);
ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass);
SClass root = Walker.buildPainlessTree(scriptClassInfo, name, source, settings, painlessLookup, null);
ScriptRoot scriptRoot = new ScriptRoot(painlessLookup, settings, scriptClassInfo, root);
SClass root = Walker.buildPainlessTree(scriptClassInfo, scriptName, source, settings, painlessLookup);
ScriptRoot scriptRoot = new ScriptRoot(painlessLookup, settings, scriptClassInfo, scriptName, source);
ClassNode classNode = root.writeClass(scriptRoot);
DefBootstrapInjectionPhase.phase(classNode);
ScriptInjectionPhase.phase(scriptRoot, classNode);
@@ -236,10 +237,12 @@ ScriptRoot compile(Loader loader, String name, String source, CompilerSettings s
* @return The bytes for compilation.
*/
byte[] compile(String name, String source, CompilerSettings settings, Printer debugStream) {
String scriptName = Location.computeSourceName(name);
ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass);
SClass root = Walker.buildPainlessTree(scriptClassInfo, name, source, settings, painlessLookup, debugStream);
ScriptRoot scriptRoot = new ScriptRoot(painlessLookup, settings, scriptClassInfo, root);
SClass root = Walker.buildPainlessTree(scriptClassInfo, scriptName, source, settings, painlessLookup);
ScriptRoot scriptRoot = new ScriptRoot(painlessLookup, settings, scriptClassInfo, scriptName, source);
ClassNode classNode = root.writeClass(scriptRoot);
classNode.setDebugStream(debugStream);
DefBootstrapInjectionPhase.phase(classNode);
ScriptInjectionPhase.phase(scriptRoot, classNode);

Original file line number Diff line number Diff line change
@@ -159,7 +159,6 @@
import org.elasticsearch.painless.node.SThrow;
import org.elasticsearch.painless.node.STry;
import org.elasticsearch.painless.node.SWhile;
import org.objectweb.asm.util.Printer;

import java.util.ArrayList;
import java.util.List;
@@ -170,26 +169,21 @@
public final class Walker extends PainlessParserBaseVisitor<ANode> {

public static SClass buildPainlessTree(ScriptClassInfo mainMethod, String sourceName,
String sourceText, CompilerSettings settings, PainlessLookup painlessLookup,
Printer debugStream) {
return new Walker(mainMethod, sourceName, sourceText, settings, painlessLookup, debugStream).source;
String sourceText, CompilerSettings settings, PainlessLookup painlessLookup) {
return new Walker(mainMethod, sourceName, sourceText, settings, painlessLookup).source;
}

private final ScriptClassInfo scriptClassInfo;
private final SClass source;
private final CompilerSettings settings;
private final Printer debugStream;
private final String sourceName;
private final String sourceText;
private final PainlessLookup painlessLookup;

private Walker(ScriptClassInfo scriptClassInfo, String sourceName, String sourceText,
CompilerSettings settings, PainlessLookup painlessLookup, Printer debugStream) {
CompilerSettings settings, PainlessLookup painlessLookup) {
this.scriptClassInfo = scriptClassInfo;
this.debugStream = debugStream;
this.settings = settings;
this.sourceName = Location.computeSourceName(sourceName);
this.sourceText = sourceText;
this.sourceName = sourceName;
this.painlessLookup = painlessLookup;
this.source = (SClass)visit(buildAntlrTree(sourceText));
}
@@ -279,7 +273,7 @@ public ANode visitSource(SourceContext ctx) {
location(ctx), statements), true, false, false, true);
functions.add(execute);

return new SClass(scriptClassInfo, sourceName, sourceText, debugStream, location(ctx), functions);
return new SClass(location(ctx), functions);
}

@Override
Original file line number Diff line number Diff line change
@@ -69,36 +69,9 @@ public BlockNode getClinitBlockNode() {

/* ---- end tree structure, begin node data ---- */

private ScriptClassInfo scriptClassInfo;
private String name;
private String sourceText;
private Printer debugStream;
private ScriptRoot scriptRoot;

public void setScriptClassInfo(ScriptClassInfo scriptClassInfo) {
this.scriptClassInfo = scriptClassInfo;
}

public ScriptClassInfo getScriptClassInfo() {
return scriptClassInfo;
}

public void setName(String name) {
this.name = name;
}

public String getName() {
return name;
}

public void setSourceText(String sourceText) {
this.sourceText = sourceText;
}

public String getSourceText() {
return sourceText;
}

public void setDebugStream(Printer debugStream) {
this.debugStream = debugStream;
}
@@ -125,7 +98,8 @@ public ClassNode() {
}

public byte[] write() {
BitSet statements = new BitSet(sourceText.length());
ScriptClassInfo scriptClassInfo = scriptRoot.getScriptClassInfo();
BitSet statements = new BitSet(scriptRoot.getScriptSource().length());
scriptRoot.addStaticConstant("$STATEMENTS", statements);

// Create the ClassWriter.
@@ -139,7 +113,7 @@ public byte[] write() {
ClassWriter classWriter = new ClassWriter(scriptRoot.getCompilerSettings(), statements, debugStream,
scriptClassInfo.getBaseClass(), classFrames, classAccess, className, classInterfaces);
ClassVisitor classVisitor = classWriter.getClassVisitor();
classVisitor.visitSource(Location.computeSourceName(name), null);
classVisitor.visitSource(Location.computeSourceName(scriptRoot.getScriptName()), null);

org.objectweb.asm.commons.Method init;

Original file line number Diff line number Diff line change
@@ -21,17 +21,8 @@

import org.elasticsearch.painless.Location;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;

import static java.lang.Math.max;
import static java.util.Collections.emptyList;

/**
* The superclass for all nodes.
*/
@@ -40,7 +31,7 @@ public abstract class ANode {
/**
* The identifier of the script and character offset used for debugging and errors.
*/
final Location location;
protected final Location location;

/**
* Standard constructor with location used for error tracking.
@@ -55,81 +46,4 @@ public abstract class ANode {
RuntimeException createError(RuntimeException exception) {
return location.createError(exception);
}

/**
* Subclasses should implement this with a method like {@link #singleLineToString(Object...)} or
* {@link #multilineToString(Collection, Collection)}.
*/
public abstract String toString();

// Below are utilities for building the toString

/**
* Build {@link #toString()} for a node without inserting line breaks between the sub-nodes.
*/
protected String singleLineToString(Object... subs) {
return singleLineToString(Arrays.asList(subs));
}

/**
* Build {@link #toString()} for a node without inserting line breaks between the sub-nodes.
*/
protected String singleLineToString(Collection<? extends Object> subs) {
return joinWithName(getClass().getSimpleName(), subs, emptyList());
}

/**
* Build {@link #toString()} for a node that optionally ends in {@code (Args some arguments here)}. Usually function calls.
*/
protected String singleLineToStringWithOptionalArgs(Collection<? extends ANode> arguments, Object... restOfSubs) {
List<Object> subs = new ArrayList<>();
Collections.addAll(subs, restOfSubs);
if (false == arguments.isEmpty()) {
subs.add(joinWithName("Args", arguments, emptyList()));
}
return singleLineToString(subs);
}

/**
* Build {@link #toString()} for a node that should have new lines after some of its sub-nodes.
*/
protected String multilineToString(Collection<? extends Object> sameLine, Collection<? extends Object> ownLine) {
return joinWithName(getClass().getSimpleName(), sameLine, ownLine);
}

/**
* Zip two (potentially uneven) lists together into for {@link #toString()}.
*/
protected List<String> pairwiseToString(Collection<? extends Object> lefts, Collection<? extends Object> rights) {
List<String> pairs = new ArrayList<>(max(lefts.size(), rights.size()));
Iterator<? extends Object> left = lefts.iterator();
Iterator<? extends Object> right = rights.iterator();
while (left.hasNext() || right.hasNext()) {
pairs.add(joinWithName("Pair",
Arrays.asList(left.hasNext() ? left.next() : "<uneven>", right.hasNext() ? right.next() : "<uneven>"),
emptyList()));
}
return pairs;
}

/**
* Build a {@link #toString()} for some expressions. Usually best to use {@link #singleLineToString(Object...)} or
* {@link #multilineToString(Collection, Collection)} instead because they include the name of the node by default.
*/
protected String joinWithName(String name, Collection<? extends Object> sameLine,
Collection<? extends Object> ownLine) {
StringBuilder b = new StringBuilder();
b.append('(').append(name);
for (Object sub : sameLine) {
b.append(' ').append(sub);
}
if (ownLine.size() == 1 && sameLine.isEmpty()) {
b.append(' ').append(ownLine.iterator().next());
} else {
for (Object sub : ownLine) {
b.append("\n ").append(Objects.toString(sub).replace("\n", "\n "));
}
}
return b.append(')').toString();
}
}
Original file line number Diff line number Diff line change
@@ -73,9 +73,4 @@ public DResolvedType resolveType(PainlessLookup painlessLookup) {
public Class<?> getType() {
return type;
}

@Override
public String toString() {
return "(DResolvedType [" + PainlessLookupUtility.typeToCanonicalTypeName(type) + "])";
}
}
Original file line number Diff line number Diff line change
@@ -52,10 +52,5 @@ public DResolvedType resolveType(PainlessLookup painlessLookup) {

return new DResolvedType(location, type);
}

@Override
public String toString() {
return "(DUnresolvedType [" + typeName + "])";
}
}

Original file line number Diff line number Diff line change
@@ -36,8 +36,6 @@
import org.elasticsearch.painless.lookup.def;
import org.elasticsearch.painless.symbol.ScriptRoot;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

/**
@@ -251,24 +249,4 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in

return output;
}

@Override
public String toString() {
List<Object> subs = new ArrayList<>();
subs.add(lhs);
if (rhs != null) {
// Make sure "=" is in the symbol so this is easy to read at a glance
subs.add(operation == null ? "=" : operation.symbol + "=");
subs.add(rhs);
return singleLineToString(subs);
}
subs.add(operation.symbol);
if (pre) {
subs.add("pre");
}
if (post) {
subs.add("post");
}
return singleLineToString(subs);
}
}
Original file line number Diff line number Diff line change
@@ -151,9 +151,4 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in

return output;
}

@Override
public String toString() {
return singleLineToString(left, operation.symbol, right);
}
}
Original file line number Diff line number Diff line change
@@ -74,9 +74,4 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in

return output;
}

@Override
public String toString() {
return singleLineToString(left, operation.symbol, right);
}
}
Original file line number Diff line number Diff line change
@@ -57,9 +57,4 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in

return output;
}

@Override
public String toString() {
return singleLineToString(constant);
}
}
Original file line number Diff line number Diff line change
@@ -22,8 +22,8 @@
import org.elasticsearch.painless.Location;
import org.elasticsearch.painless.Scope;
import org.elasticsearch.painless.ir.ClassNode;
import org.elasticsearch.painless.ir.MemberCallNode;
import org.elasticsearch.painless.ir.FieldNode;
import org.elasticsearch.painless.ir.MemberCallNode;
import org.elasticsearch.painless.lookup.PainlessClassBinding;
import org.elasticsearch.painless.lookup.PainlessInstanceBinding;
import org.elasticsearch.painless.lookup.PainlessMethod;
@@ -188,9 +188,4 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in

return output;
}

@Override
public String toString() {
return singleLineToStringWithOptionalArgs(arguments, name);
}
}
Original file line number Diff line number Diff line change
@@ -100,9 +100,4 @@ public String getPointer() {
public List<Class<?>> getCaptures() {
return Collections.singletonList(captured.getType());
}

@Override
public String toString() {
return singleLineToString(variable, call);
}
}
Original file line number Diff line number Diff line change
@@ -107,9 +107,4 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in

return output;
}

@Override
public String toString() {
return singleLineToString(left, operation.symbol, right);
}
}
Original file line number Diff line number Diff line change
@@ -99,9 +99,4 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in

return output;
}

@Override
public String toString() {
return singleLineToString(condition, left, right);
}
}
Original file line number Diff line number Diff line change
@@ -77,13 +77,4 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in

return output;
}

@Override
public String toString() {
String c = constant.toString();
if (constant instanceof String) {
c = "'" + c + "'";
}
return singleLineToString(constant.getClass().getSimpleName(), c);
}
}
Original file line number Diff line number Diff line change
@@ -79,9 +79,4 @@ Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input in

return output;
}

@Override
public String toString() {
return singleLineToString(value);
}
}
Loading

0 comments on commit 1bb36d7

Please sign in to comment.