Skip to content

Commit 9920396

Browse files
jdconradjdconrad
authored
Remove several Painless user tree nodes (#58129)
This PR contains the following clean up: * Remove the EConstant node from the user tree. This is no longer necessary as we don't do constant folding as part of the semantic phase anymore. * Remove DType nodes from the user tree. These are no longer necessary as they were part of a plan to add flexibility to Painless under a single tree architecture. * Remove SDeclaration from SCatch. Catch statements are unique enough that using and SDeclaration node for the variable declaration was an overly complex indirection. * Rename ScriptRoot to ScriptScope and rename Scope to SemanticScope. ScriptRoot is just the scope for the overall compilation of the script. With multiple phases is makes sense to rename Scope to SemanticScope as this will only be used during the semantic phase and may overlap otherwise. * Make ScriptScope a child of SemanticScope so that we can have only one argument passed in during phases as this will eventually be refactored into a generic base visitor.
1 parent bf46468 commit 9920396

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

61 files changed

+438
-743
lines changed

‎modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import org.elasticsearch.painless.lookup.PainlessLookup;
2626
import org.elasticsearch.painless.node.SClass;
2727
import org.elasticsearch.painless.spi.Whitelist;
28-
import org.elasticsearch.painless.symbol.ScriptRoot;
28+
import org.elasticsearch.painless.symbol.ScriptScope;
2929
import org.objectweb.asm.util.Printer;
3030

3131
import java.lang.reflect.Method;
@@ -204,26 +204,26 @@ private static void addFactoryMethod(Map<String, Class<?>> additionalClasses, Cl
204204
* @param name The name of the script.
205205
* @param source The source code for the script.
206206
* @param settings The CompilerSettings to be used during the compilation.
207-
* @return The ScriptRoot used to compile
207+
* @return The ScriptScope used to compile
208208
*/
209-
ScriptRoot compile(Loader loader, String name, String source, CompilerSettings settings) {
209+
ScriptScope compile(Loader loader, String name, String source, CompilerSettings settings) {
210210
String scriptName = Location.computeSourceName(name);
211211
ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass);
212212
SClass root = Walker.buildPainlessTree(scriptClassInfo, scriptName, source, settings);
213-
ScriptRoot scriptRoot = new ScriptRoot(painlessLookup, settings, scriptClassInfo, scriptName, source);
214-
ClassNode classNode = root.writeClass(scriptRoot);
213+
ScriptScope scriptScope = new ScriptScope(painlessLookup, settings, scriptClassInfo, scriptName, source);
214+
ClassNode classNode = root.analyze(scriptScope);
215215
DefBootstrapInjectionPhase.phase(classNode);
216-
ScriptInjectionPhase.phase(scriptRoot, classNode);
216+
ScriptInjectionPhase.phase(scriptScope, classNode);
217217
byte[] bytes = classNode.write();
218218

219219
try {
220220
Class<? extends PainlessScript> clazz = loader.defineScript(CLASS_NAME, bytes);
221221

222-
for (Map.Entry<String, Object> staticConstant : scriptRoot.getStaticConstants().entrySet()) {
222+
for (Map.Entry<String, Object> staticConstant : scriptScope.getStaticConstants().entrySet()) {
223223
clazz.getField(staticConstant.getKey()).set(null, staticConstant.getValue());
224224
}
225225

226-
return scriptRoot;
226+
return scriptScope;
227227
} catch (Exception exception) {
228228
// Catch everything to let the user know this is something caused internally.
229229
throw new IllegalStateException("An internal error occurred attempting to define the script [" + name + "].", exception);
@@ -240,11 +240,11 @@ byte[] compile(String name, String source, CompilerSettings settings, Printer de
240240
String scriptName = Location.computeSourceName(name);
241241
ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass);
242242
SClass root = Walker.buildPainlessTree(scriptClassInfo, scriptName, source, settings);
243-
ScriptRoot scriptRoot = new ScriptRoot(painlessLookup, settings, scriptClassInfo, scriptName, source);
244-
ClassNode classNode = root.writeClass(scriptRoot);
243+
ScriptScope scriptScope = new ScriptScope(painlessLookup, settings, scriptClassInfo, scriptName, source);
244+
ClassNode classNode = root.analyze(scriptScope);
245245
classNode.setDebugStream(debugStream);
246246
DefBootstrapInjectionPhase.phase(classNode);
247-
ScriptInjectionPhase.phase(scriptRoot, classNode);
247+
ScriptInjectionPhase.phase(scriptScope, classNode);
248248

249249
return classNode.write();
250250
}

‎modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import org.elasticsearch.painless.lookup.PainlessLookup;
2626
import org.elasticsearch.painless.lookup.PainlessLookupBuilder;
2727
import org.elasticsearch.painless.spi.Whitelist;
28-
import org.elasticsearch.painless.symbol.ScriptRoot;
28+
import org.elasticsearch.painless.symbol.ScriptScope;
2929
import org.elasticsearch.script.ScriptContext;
3030
import org.elasticsearch.script.ScriptEngine;
3131
import org.elasticsearch.script.ScriptException;
@@ -141,12 +141,12 @@ public Loader run() {
141141
}
142142
});
143143

144-
ScriptRoot scriptRoot = compile(contextsToCompilers.get(context), loader, scriptName, scriptSource, params);
144+
ScriptScope scriptScope = compile(contextsToCompilers.get(context), loader, scriptName, scriptSource, params);
145145

146146
if (context.statefulFactoryClazz != null) {
147-
return generateFactory(loader, context, generateStatefulFactory(loader, context, scriptRoot), scriptRoot);
147+
return generateFactory(loader, context, generateStatefulFactory(loader, context, scriptScope), scriptScope);
148148
} else {
149-
return generateFactory(loader, context, WriterConstants.CLASS_TYPE, scriptRoot);
149+
return generateFactory(loader, context, WriterConstants.CLASS_TYPE, scriptScope);
150150
}
151151
}
152152

@@ -169,7 +169,7 @@ public Set<ScriptContext<?>> getSupportedContexts() {
169169
private <T> Type generateStatefulFactory(
170170
Loader loader,
171171
ScriptContext<T> context,
172-
ScriptRoot scriptRoot
172+
ScriptScope scriptScope
173173
) {
174174
int classFrames = ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS;
175175
int classAccess = Opcodes.ACC_PUBLIC | Opcodes.ACC_SUPER | Opcodes.ACC_FINAL;
@@ -251,7 +251,7 @@ private <T> Type generateStatefulFactory(
251251
adapter.returnValue();
252252
adapter.endMethod();
253253

254-
writeNeedsMethods(context.statefulFactoryClazz, writer, scriptRoot.getUsedVariables());
254+
writeNeedsMethods(context.statefulFactoryClazz, writer, scriptScope.getUsedVariables());
255255
writer.visitEnd();
256256

257257
loader.defineFactory(className.replace('/', '.'), writer.toByteArray());
@@ -268,15 +268,15 @@ private <T> Type generateStatefulFactory(
268268
* @param context The {@link ScriptContext}'s semantics are used to define the factory class.
269269
* @param classType The type to be instaniated in the newFactory or newInstance method. Depends
270270
* on whether a {@link ScriptContext#statefulFactoryClazz} is specified.
271-
* @param scriptRoot the {@link ScriptRoot} used to do the compilation
271+
* @param scriptScope the {@link ScriptScope} used to do the compilation
272272
* @param <T> The factory class.
273273
* @return A factory class that will return script instances.
274274
*/
275275
private <T> T generateFactory(
276276
Loader loader,
277277
ScriptContext<T> context,
278278
Type classType,
279-
ScriptRoot scriptRoot
279+
ScriptScope scriptScope
280280
) {
281281
int classFrames = ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS;
282282
int classAccess = Opcodes.ACC_PUBLIC | Opcodes.ACC_SUPER| Opcodes.ACC_FINAL;
@@ -328,7 +328,7 @@ private <T> T generateFactory(
328328
adapter.returnValue();
329329
adapter.endMethod();
330330

331-
writeNeedsMethods(context.factoryClazz, writer, scriptRoot.getUsedVariables());
331+
writeNeedsMethods(context.factoryClazz, writer, scriptScope.getUsedVariables());
332332

333333
String methodName = "isResultDeterministic";
334334
org.objectweb.asm.commons.Method isResultDeterministic = new org.objectweb.asm.commons.Method(methodName,
@@ -337,7 +337,7 @@ private <T> T generateFactory(
337337
GeneratorAdapter deterAdapter = new GeneratorAdapter(Opcodes.ASM5, isResultDeterministic,
338338
writer.visitMethod(Opcodes.ACC_PUBLIC, methodName, isResultDeterministic.getDescriptor(), null, null));
339339
deterAdapter.visitCode();
340-
deterAdapter.push(scriptRoot.isDeterministic());
340+
deterAdapter.push(scriptScope.isDeterministic());
341341
deterAdapter.returnValue();
342342
deterAdapter.endMethod();
343343

@@ -374,14 +374,14 @@ private void writeNeedsMethods(Class<?> clazz, ClassWriter writer, Set<String> e
374374
}
375375
}
376376

377-
ScriptRoot compile(Compiler compiler, Loader loader, String scriptName, String source, Map<String, String> params) {
377+
ScriptScope compile(Compiler compiler, Loader loader, String scriptName, String source, Map<String, String> params) {
378378
final CompilerSettings compilerSettings = buildCompilerSettings(params);
379379

380380
try {
381381
// Drop all permissions to actually compile the code itself.
382-
return AccessController.doPrivileged(new PrivilegedAction<ScriptRoot>() {
382+
return AccessController.doPrivileged(new PrivilegedAction<ScriptScope>() {
383383
@Override
384-
public ScriptRoot run() {
384+
public ScriptScope run() {
385385
String name = scriptName == null ? source : scriptName;
386386
return compiler.compile(loader, name, source, compilerSettings);
387387
}

‎modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptInjectionPhase.java

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
import org.elasticsearch.painless.lookup.PainlessLookup;
4040
import org.elasticsearch.painless.lookup.PainlessMethod;
4141
import org.elasticsearch.painless.symbol.FunctionTable.LocalFunction;
42-
import org.elasticsearch.painless.symbol.ScriptRoot;
42+
import org.elasticsearch.painless.symbol.ScriptScope;
4343
import org.elasticsearch.script.ScriptException;
4444
import org.objectweb.asm.Opcodes;
4545
import org.objectweb.asm.commons.Method;
@@ -58,7 +58,7 @@
5858
*/
5959
public class ScriptInjectionPhase {
6060

61-
public static void phase(ScriptRoot scriptRoot, ClassNode classNode) {
61+
public static void phase(ScriptScope scriptScope, ClassNode classNode) {
6262
FunctionNode executeFunctionNode = null;
6363

6464
// look up the execute method for decoration
@@ -74,8 +74,8 @@ public static void phase(ScriptRoot scriptRoot, ClassNode classNode) {
7474
}
7575

7676
injectStaticFieldsAndGetters(classNode);
77-
injectGetsDeclarations(scriptRoot, executeFunctionNode);
78-
injectNeedsMethods(scriptRoot, classNode);
77+
injectGetsDeclarations(scriptScope, executeFunctionNode);
78+
injectNeedsMethods(scriptScope, classNode);
7979
injectSandboxExceptions(executeFunctionNode);
8080
}
8181

@@ -206,7 +206,7 @@ protected static void injectStaticFieldsAndGetters(ClassNode classNode) {
206206
// requires the gets method name be modified from "getExample" to "example"
207207
// if a get method variable isn't used it's declaration node is removed from
208208
// the ir tree permanently so there is no frivolous variable slotting
209-
protected static void injectGetsDeclarations(ScriptRoot scriptRoot, FunctionNode functionNode) {
209+
protected static void injectGetsDeclarations(ScriptScope scriptScope, FunctionNode functionNode) {
210210
Location internalLocation = new Location("$internal$ScriptInjectionPhase$injectGetsDeclarations", 0);
211211
BlockNode blockNode = functionNode.getBlockNode();
212212
int statementIndex = 0;
@@ -218,14 +218,14 @@ protected static void injectGetsDeclarations(ScriptRoot scriptRoot, FunctionNode
218218
DeclarationNode declarationNode = (DeclarationNode)statementNode;
219219
boolean isRemoved = false;
220220

221-
for (int getIndex = 0; getIndex < scriptRoot.getScriptClassInfo().getGetMethods().size(); ++getIndex) {
222-
Class<?> returnType = scriptRoot.getScriptClassInfo().getGetReturns().get(getIndex);
223-
Method getMethod = scriptRoot.getScriptClassInfo().getGetMethods().get(getIndex);
221+
for (int getIndex = 0; getIndex < scriptScope.getScriptClassInfo().getGetMethods().size(); ++getIndex) {
222+
Class<?> returnType = scriptScope.getScriptClassInfo().getGetReturns().get(getIndex);
223+
Method getMethod = scriptScope.getScriptClassInfo().getGetMethods().get(getIndex);
224224
String name = getMethod.getName().substring(3);
225225
name = Character.toLowerCase(name.charAt(0)) + name.substring(1);
226226

227227
if (name.equals(declarationNode.getName())) {
228-
if (scriptRoot.getUsedVariables().contains(name)) {
228+
if (scriptScope.getUsedVariables().contains(name)) {
229229
MemberCallNode memberCallNode = new MemberCallNode();
230230
memberCallNode.setLocation(internalLocation);
231231
memberCallNode.setExpressionType(declarationNode.getDeclarationType());
@@ -251,10 +251,10 @@ protected static void injectGetsDeclarations(ScriptRoot scriptRoot, FunctionNode
251251
}
252252

253253
// injects needs methods as defined by ScriptClassInfo
254-
protected static void injectNeedsMethods(ScriptRoot scriptRoot, ClassNode classNode) {
254+
protected static void injectNeedsMethods(ScriptScope scriptScope, ClassNode classNode) {
255255
Location internalLocation = new Location("$internal$ScriptInjectionPhase$injectNeedsMethods", 0);
256256

257-
for (org.objectweb.asm.commons.Method needsMethod : scriptRoot.getScriptClassInfo().getNeedsMethods()) {
257+
for (org.objectweb.asm.commons.Method needsMethod : scriptScope.getScriptClassInfo().getNeedsMethods()) {
258258
String name = needsMethod.getName();
259259
name = name.substring(5);
260260
name = Character.toLowerCase(name.charAt(0)) + name.substring(1);
@@ -285,7 +285,7 @@ protected static void injectNeedsMethods(ScriptRoot scriptRoot, ClassNode classN
285285
ConstantNode constantNode = new ConstantNode();
286286
constantNode.setLocation(internalLocation);
287287
constantNode.setExpressionType(boolean.class);
288-
constantNode.setConstant(scriptRoot.getUsedVariables().contains(name));
288+
constantNode.setConstant(scriptScope.getUsedVariables().contains(name));
289289

290290
returnNode.setExpressionNode(constantNode);
291291
}
@@ -311,17 +311,11 @@ protected static void injectSandboxExceptions(FunctionNode functionNode) {
311311

312312
CatchNode catchNode = new CatchNode();
313313
catchNode.setLocation(internalLocation);
314+
catchNode.setExceptionType(PainlessExplainError.class);
315+
catchNode.setSymbol("#painlessExplainError");
314316

315317
tryNode.addCatchNode(catchNode);
316318

317-
DeclarationNode declarationNode = new DeclarationNode();
318-
declarationNode.setLocation(internalLocation);
319-
declarationNode.setDeclarationType(PainlessExplainError.class);
320-
declarationNode.setName("#painlessExplainError");
321-
declarationNode.setRequiresDefault(false);
322-
323-
catchNode.setDeclarationNode(declarationNode);
324-
325319
BlockNode catchBlockNode = new BlockNode();
326320
catchBlockNode.setLocation(internalLocation);
327321
catchBlockNode.setAllEscape(true);
@@ -403,17 +397,11 @@ protected static void injectSandboxExceptions(FunctionNode functionNode) {
403397

404398
catchNode = new CatchNode();
405399
catchNode.setLocation(internalLocation);
400+
catchNode.setExceptionType(throwable);
401+
catchNode.setSymbol(name);
406402

407403
tryNode.addCatchNode(catchNode);
408404

409-
declarationNode = new DeclarationNode();
410-
declarationNode.setLocation(internalLocation);
411-
declarationNode.setDeclarationType(throwable);
412-
declarationNode.setName(name);
413-
declarationNode.setRequiresDefault(false);
414-
415-
catchNode.setDeclarationNode(declarationNode);
416-
417405
catchBlockNode = new BlockNode();
418406
catchBlockNode.setLocation(internalLocation);
419407
catchBlockNode.setAllEscape(true);

‎modules/lang-painless/src/main/java/org/elasticsearch/painless/antlr/Walker.java

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,6 @@
111111
import org.elasticsearch.painless.node.AExpression;
112112
import org.elasticsearch.painless.node.ANode;
113113
import org.elasticsearch.painless.node.AStatement;
114-
import org.elasticsearch.painless.node.DResolvedType;
115-
import org.elasticsearch.painless.node.DUnresolvedType;
116114
import org.elasticsearch.painless.node.EAssignment;
117115
import org.elasticsearch.painless.node.EBinary;
118116
import org.elasticsearch.painless.node.EBool;
@@ -122,7 +120,6 @@
122120
import org.elasticsearch.painless.node.ECallLocal;
123121
import org.elasticsearch.painless.node.EComp;
124122
import org.elasticsearch.painless.node.EConditional;
125-
import org.elasticsearch.painless.node.EConstant;
126123
import org.elasticsearch.painless.node.EDecimal;
127124
import org.elasticsearch.painless.node.EDot;
128125
import org.elasticsearch.painless.node.EElvis;
@@ -257,7 +254,7 @@ public ANode visitSource(SourceContext ctx) {
257254
name = Character.toLowerCase(name.charAt(0)) + name.substring(1);
258255

259256
statements.add(new SDeclaration(nextIdentifier(), location(ctx),
260-
new DResolvedType(location(ctx), scriptClassInfo.getGetReturns().get(index), false), name, false, null));
257+
PainlessLookupUtility.typeToCanonicalTypeName(scriptClassInfo.getGetReturns().get(index)), name, null));
261258
}
262259

263260
for (StatementContext statement : ctx.statement()) {
@@ -512,9 +509,7 @@ public ANode visitDeclaration(DeclarationContext ctx) {
512509
for (DeclvarContext declvar : ctx.declvar()) {
513510
String name = declvar.ID().getText();
514511
AExpression expression = declvar.expression() == null ? null : (AExpression)visit(declvar.expression());
515-
DUnresolvedType unresolvedType = new DUnresolvedType(location(declvar), type);
516-
517-
declarations.add(new SDeclaration(nextIdentifier(), location(declvar), unresolvedType, name, true, expression));
512+
declarations.add(new SDeclaration(nextIdentifier(), location(declvar), type, name, expression));
518513
}
519514

520515
return new SDeclBlock(nextIdentifier(), location(ctx), declarations);
@@ -541,9 +536,7 @@ public ANode visitTrap(TrapContext ctx) {
541536
String name = ctx.ID().getText();
542537
SBlock block = (SBlock)visit(ctx.block());
543538

544-
return new SCatch(nextIdentifier(), location(ctx), Exception.class,
545-
new SDeclaration(nextIdentifier(), location(ctx.type()),
546-
new DUnresolvedType(location(ctx.type()), type), name, false, null), block);
539+
return new SCatch(nextIdentifier(), location(ctx), Exception.class, type, name, block);
547540
}
548541

549542
@Override
@@ -719,7 +712,7 @@ public ANode visitPre(PreContext ctx) {
719712
}
720713

721714
return new EAssignment(nextIdentifier(), location(ctx), expression,
722-
new EConstant(nextIdentifier(), location(ctx), 1), false, operation);
715+
new ENumeric(nextIdentifier(), location(ctx), "1", 10), false, operation);
723716
}
724717

725718
@Override
@@ -764,7 +757,7 @@ public ANode visitPost(PostContext ctx) {
764757
}
765758

766759
return new EAssignment(nextIdentifier(), location(ctx), expression,
767-
new EConstant(nextIdentifier(), location(ctx), 1), true, operation);
760+
new ENumeric(nextIdentifier(), location(ctx), "1", 10), true, operation);
768761
}
769762

770763
@Override
@@ -794,15 +787,15 @@ public ANode visitPrimordefcast(PainlessParser.PrimordefcastContext ctx) {
794787
String type = ctx.primordefcasttype().getText();
795788
AExpression child = (AExpression)visit(ctx.unary());
796789

797-
return new EExplicit(nextIdentifier(), location(ctx), new DUnresolvedType(location(ctx.primordefcasttype()), type), child);
790+
return new EExplicit(nextIdentifier(), location(ctx), type, child);
798791
}
799792

800793
@Override
801794
public ANode visitRefcast(PainlessParser.RefcastContext ctx) {
802795
String type = ctx.refcasttype().getText();
803796
AExpression child = (AExpression)visit(ctx.unarynotaddsub());
804797

805-
return new EExplicit(nextIdentifier(), location(ctx), new DUnresolvedType(location(ctx.refcasttype()), type), child);
798+
return new EExplicit(nextIdentifier(), location(ctx), type, child);
806799
}
807800

808801
@Override

0 commit comments

Comments
 (0)