Update Painless AST Catch Node (#50044)

This makes two changes to the catch node:

1. Use SDeclaration to replace independent variable usage.
2. Use a DType to set a "minimum" exception type - this allows us to require 
users to continue using Exception as "minimum" type for catch blocks, but 
for us to internally catch Error/Throwable. This is a required step to 
removing custom try/catch blocks from SClass.
This commit is contained in:
Jack Conradson 2019-12-10 12:55:06 -08:00
parent ba9526ec4f
commit eb20db8a1c
5 changed files with 37 additions and 29 deletions

View File

@ -109,6 +109,7 @@ import org.elasticsearch.painless.lookup.PainlessLookup;
import org.elasticsearch.painless.node.AExpression;
import org.elasticsearch.painless.node.ANode;
import org.elasticsearch.painless.node.AStatement;
import org.elasticsearch.painless.node.DResolvedType;
import org.elasticsearch.painless.node.DUnresolvedType;
import org.elasticsearch.painless.node.EAssignment;
import org.elasticsearch.painless.node.EBinary;
@ -232,6 +233,10 @@ public final class Walker extends PainlessParserBaseVisitor<ANode> {
return new Location(sourceName, ctx.getStart().getStartIndex());
}
private Location location(TerminalNode tn) {
return new Location(sourceName, tn.getSymbol().getStartIndex());
}
@Override
public ANode visitSource(SourceContext ctx) {
List<SFunction> functions = new ArrayList<>();
@ -503,7 +508,8 @@ public final class Walker extends PainlessParserBaseVisitor<ANode> {
String name = ctx.ID().getText();
SBlock block = (SBlock)visit(ctx.block());
return new SCatch(location(ctx), type, name, block);
return new SCatch(location(ctx), new DResolvedType(location(ctx), Exception.class),
new SDeclaration(location(ctx.TYPE()), new DUnresolvedType(location(ctx.TYPE()), type), name, null), block);
}
@Override

View File

@ -22,10 +22,10 @@ package org.elasticsearch.painless.node;
import org.elasticsearch.painless.ClassWriter;
import org.elasticsearch.painless.Globals;
import org.elasticsearch.painless.Locals;
import org.elasticsearch.painless.Locals.Variable;
import org.elasticsearch.painless.Location;
import org.elasticsearch.painless.MethodWriter;
import org.elasticsearch.painless.ScriptRoot;
import org.elasticsearch.painless.lookup.PainlessLookupUtility;
import org.objectweb.asm.Label;
import org.objectweb.asm.Opcodes;
@ -37,27 +37,25 @@ import java.util.Set;
*/
public final class SCatch extends AStatement {
private final String type;
private final String name;
private final DType baseException;
private final SDeclaration declaration;
private final SBlock block;
private Variable variable = null;
Label begin = null;
Label end = null;
Label exception = null;
public SCatch(Location location, String type, String name, SBlock block) {
public SCatch(Location location, DType baseException, SDeclaration declaration, SBlock block) {
super(location);
this.type = Objects.requireNonNull(type);
this.name = Objects.requireNonNull(name);
this.baseException = Objects.requireNonNull(baseException);
this.declaration = Objects.requireNonNull(declaration);
this.block = block;
}
@Override
void extractVariables(Set<String> variables) {
variables.add(name);
declaration.extractVariables(variables);
if (block != null) {
block.extractVariables(variables);
@ -66,18 +64,17 @@ public final class SCatch extends AStatement {
@Override
void analyze(ScriptRoot scriptRoot, Locals locals) {
Class<?> clazz = scriptRoot.getPainlessLookup().canonicalTypeNameToType(this.type);
declaration.analyze(scriptRoot, locals);
if (clazz == null) {
throw createError(new IllegalArgumentException("Not a type [" + this.type + "]."));
Class<?> baseType = baseException.resolveType(scriptRoot.getPainlessLookup()).getType();
Class<?> type = declaration.variable.clazz;
if (baseType.isAssignableFrom(type) == false) {
throw createError(new ClassCastException(
"cannot cast from [" + PainlessLookupUtility.typeToCanonicalTypeName(type) + "] " +
"to [" + PainlessLookupUtility.typeToCanonicalTypeName(baseType) + "]"));
}
if (!Exception.class.isAssignableFrom(clazz)) {
throw createError(new ClassCastException("Not an exception type [" + this.type + "]."));
}
variable = locals.addVariable(location, clazz, name, true);
if (block != null) {
block.lastSource = lastSource;
block.inLoop = inLoop;
@ -100,7 +97,8 @@ public final class SCatch extends AStatement {
Label jump = new Label();
methodWriter.mark(jump);
methodWriter.visitVarInsn(MethodWriter.getType(variable.clazz).getOpcode(Opcodes.ISTORE), variable.getSlot());
methodWriter.visitVarInsn(
MethodWriter.getType(declaration.variable.clazz).getOpcode(Opcodes.ISTORE), declaration.variable.getSlot());
if (block != null) {
block.continu = continu;
@ -108,7 +106,7 @@ public final class SCatch extends AStatement {
block.write(classWriter, methodWriter, globals);
}
methodWriter.visitTryCatchBlock(begin, end, jump, MethodWriter.getType(variable.clazz).getInternalName());
methodWriter.visitTryCatchBlock(begin, end, jump, MethodWriter.getType(declaration.variable.clazz).getInternalName());
if (exception != null && (block == null || !block.allEscape)) {
methodWriter.goTo(exception);
@ -117,6 +115,6 @@ public final class SCatch extends AStatement {
@Override
public String toString() {
return singleLineToString(type, name, block);
return singleLineToString(baseException, declaration, block);
}
}

View File

@ -40,7 +40,7 @@ public final class SDeclaration extends AStatement {
private final String name;
private AExpression expression;
private Variable variable = null;
Variable variable = null;
public SDeclaration(Location location, DType type, String name, AExpression expression) {
super(location);

View File

@ -853,7 +853,8 @@ public class NodeToStringTests extends ESTestCase {
public void testSTryAndSCatch() {
assertToString(
"(SClass (STry (SBlock (SReturn (ENumeric 1)))\n"
+ " (SCatch Exception e (SBlock (SReturn (ENumeric 2))))))",
+ " (SCatch (DResolvedType [java.lang.Exception]) (SDeclaration (DUnresolvedType [Exception]) e) " +
"(SBlock (SReturn (ENumeric 2))))))",
"try {\n"
+ " return 1\n"
+ "} catch (Exception e) {\n"
@ -863,7 +864,8 @@ public class NodeToStringTests extends ESTestCase {
"(SClass (STry (SBlock\n"
+ " (SDeclBlock (SDeclaration (DUnresolvedType [int]) i (ENumeric 1)))\n"
+ " (SReturn (ENumeric 1)))\n"
+ " (SCatch Exception e (SBlock (SReturn (ENumeric 2))))))",
+ " (SCatch (DResolvedType [java.lang.Exception]) (SDeclaration (DUnresolvedType [Exception]) e) " +
"(SBlock (SReturn (ENumeric 2))))))",
"try {\n"
+ " int i = 1;"
+ " return 1\n"
@ -872,7 +874,7 @@ public class NodeToStringTests extends ESTestCase {
+ "}");
assertToString(
"(SClass (STry (SBlock (SReturn (ENumeric 1)))\n"
+ " (SCatch Exception e (SBlock\n"
+ " (SCatch (DResolvedType [java.lang.Exception]) (SDeclaration (DUnresolvedType [Exception]) e) (SBlock\n"
+ " (SDeclBlock (SDeclaration (DUnresolvedType [int]) i (ENumeric 1)))\n"
+ " (SReturn (ENumeric 2))))))",
"try {\n"
@ -883,8 +885,10 @@ public class NodeToStringTests extends ESTestCase {
+ "}");
assertToString(
"(SClass (STry (SBlock (SReturn (ENumeric 1)))\n"
+ " (SCatch NullPointerException e (SBlock (SReturn (ENumeric 2))))\n"
+ " (SCatch Exception e (SBlock (SReturn (ENumeric 3))))))",
+ " (SCatch (DResolvedType [java.lang.Exception]) (SDeclaration (DUnresolvedType [NullPointerException]) e) " +
"(SBlock (SReturn (ENumeric 2))))\n"
+ " (SCatch (DResolvedType [java.lang.Exception]) (SDeclaration (DUnresolvedType [Exception]) e) " +
"(SBlock (SReturn (ENumeric 3))))))",
"try {\n"
+ " return 1\n"
+ "} catch (NullPointerException e) {\n"