From f6d89aee38331ebc91669f23acd814dada434873 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Wed, 26 Feb 2020 10:46:23 -0800 Subject: [PATCH] Remove constant folding in Painless user tree (#52783) This change removes constant folding from the Painless user tree. This accomplishes two goals. The first is to remove all optimizations from the user tree which will at a later time become optimization phases for the ir tree. The second is to make the user tree immutable, and this is a step toward that since we will no longer remove/modify/replace nodes in the user tree during the process of constant folding. One important note is that the conditional promotion code has changed, since the promoteConditional method considered constants (similarly to the JVM spec) that are now removed, but this code path was unreachable to begin with so the constants were never actually used to determine the appropriate promotion. --- .../painless/AnalyzerCaster.java | 126 ++------------- .../painless/node/AExpression.java | 106 +------------ .../elasticsearch/painless/node/EBinary.java | 143 ------------------ .../elasticsearch/painless/node/EBool.java | 10 -- .../elasticsearch/painless/node/EBoolean.java | 10 +- .../elasticsearch/painless/node/EComp.java | 128 ---------------- .../painless/node/EConditional.java | 13 +- .../painless/node/EConstant.java | 3 +- .../elasticsearch/painless/node/EDecimal.java | 10 +- .../elasticsearch/painless/node/EElvis.java | 8 +- .../elasticsearch/painless/node/ENumeric.java | 10 +- .../elasticsearch/painless/node/EString.java | 10 +- .../elasticsearch/painless/node/EUnary.java | 42 ----- .../org/elasticsearch/painless/node/SDo.java | 4 +- .../org/elasticsearch/painless/node/SFor.java | 4 +- .../org/elasticsearch/painless/node/SIf.java | 2 +- .../elasticsearch/painless/node/SIfElse.java | 2 +- .../elasticsearch/painless/node/SWhile.java | 4 +- .../painless/BaseClassTests.java | 1 - .../elasticsearch/painless/StringTests.java | 4 +- 20 files changed, 85 insertions(+), 555 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/AnalyzerCaster.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/AnalyzerCaster.java index b89f6ddcc4279..55432b2e586d3 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/AnalyzerCaster.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/AnalyzerCaster.java @@ -520,7 +520,7 @@ public static Class promoteEquality(Class from0, Class from1) { return Object.class; } - public static Class promoteConditional(Class from0, Class from1, Object const0, Object const1) { + public static Class promoteConditional(Class from0, Class from1) { if (from0 == from1) { return from0; } @@ -529,123 +529,29 @@ public static Class promoteConditional(Class from0, Class from1, Object return def.class; } - if (from0.isPrimitive() && from1.isPrimitive()) { - if (from0 == boolean.class && from1 == boolean.class) { - return boolean.class; - } - + if (from0.isPrimitive() && from0 != boolean.class && from1.isPrimitive() && from1 != boolean.class) { if (from0 == double.class || from1 == double.class) { return double.class; } else if (from0 == float.class || from1 == float.class) { return float.class; } else if (from0 == long.class || from1 == long.class) { return long.class; + } else if (from0 == int.class || from1 == int.class) { + return int.class; + } else if (from0 == char.class) { + if (from1 == short.class || from1 == byte.class) { + return int.class; + } else { + return null; + } + } else if (from1 == char.class) { + if (from0 == short.class || from0 == byte.class) { + return int.class; } else { - if (from0 == byte.class) { - if (from1 == byte.class) { - return byte.class; - } else if (from1 == short.class) { - if (const1 != null) { - final short constant = (short)const1; - - if (constant <= Byte.MAX_VALUE && constant >= Byte.MIN_VALUE) { - return byte.class; - } - } - - return short.class; - } else if (from1 == char.class) { - return int.class; - } else if (from1 == int.class) { - if (const1 != null) { - final int constant = (int)const1; - - if (constant <= Byte.MAX_VALUE && constant >= Byte.MIN_VALUE) { - return byte.class; - } - } - - return int.class; - } - } else if (from0 == short.class) { - if (from1 == byte.class) { - if (const0 != null) { - final short constant = (short)const0; - - if (constant <= Byte.MAX_VALUE && constant >= Byte.MIN_VALUE) { - return byte.class; - } - } - - return short.class; - } else if (from1 == short.class) { - return short.class; - } else if (from1 == char.class) { - return int.class; - } else if (from1 == int.class) { - if (const1 != null) { - final int constant = (int)const1; - - if (constant <= Short.MAX_VALUE && constant >= Short.MIN_VALUE) { - return short.class; - } - } - - return int.class; - } - } else if (from0 == char.class) { - if (from1 == byte.class) { - return int.class; - } else if (from1 == short.class) { - return int.class; - } else if (from1 == char.class) { - return char.class; - } else if (from1 == int.class) { - if (const1 != null) { - final int constant = (int)const1; - - if (constant <= Character.MAX_VALUE && constant >= Character.MIN_VALUE) { - return byte.class; - } - } - - return int.class; - } - } else if (from0 == int.class) { - if (from1 == byte.class) { - if (const0 != null) { - final int constant = (int)const0; - - if (constant <= Byte.MAX_VALUE && constant >= Byte.MIN_VALUE) { - return byte.class; - } - } - - return int.class; - } else if (from1 == short.class) { - if (const0 != null) { - final int constant = (int)const0; - - if (constant <= Short.MAX_VALUE && constant >= Short.MIN_VALUE) { - return byte.class; - } - } - - return int.class; - } else if (from1 == char.class) { - if (const0 != null) { - final int constant = (int)const0; - - if (constant <= Character.MAX_VALUE && constant >= Character.MIN_VALUE) { - return byte.class; - } - } - - return int.class; - } else if (from1 == int.class) { - return int.class; - } + return null; } + } else { + return null; } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/AExpression.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/AExpression.java index 723854dc0ce1e..ff53b661a9ba7 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/AExpression.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/AExpression.java @@ -25,7 +25,6 @@ import org.elasticsearch.painless.ir.ClassNode; import org.elasticsearch.painless.ir.ExpressionNode; import org.elasticsearch.painless.lookup.PainlessCast; -import org.elasticsearch.painless.lookup.PainlessLookupUtility; import org.elasticsearch.painless.symbol.ScriptRoot; import java.util.Objects; @@ -84,14 +83,6 @@ public abstract class AExpression extends ANode { */ boolean internal = false; - /** - * Set to the value of the constant this expression node represents if - * and only if the node represents a constant. If this is not null - * this node will be replaced by an {@link EConstant} during casting - * if it's not already one. - */ - Object constant = null; - /** * Set to true by {@link ENull} to represent a null value. */ @@ -134,97 +125,14 @@ AExpression cast(ScriptRoot scriptRoot, Scope scope) { PainlessCast cast = AnalyzerCaster.getLegalCast(location, actual, expected, explicit, internal); if (cast == null) { - if (constant == null || this instanceof EConstant) { - // For the case where a cast is not required and a constant is not set - // or the node is already an EConstant no changes are required to the tree. - - return this; - } else { - // For the case where a cast is not required but a - // constant is set, an EConstant replaces this node - // with the constant copied from this node. Note that - // for constants output data does not need to be copied - // from this node because the output data for the EConstant - // will already be the same. - - EConstant econstant = new EConstant(location, constant); - econstant.analyze(scriptRoot, scope); - - if (!expected.equals(econstant.actual)) { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - - return econstant; - } + return this; } else { - if (constant == null) { - // For the case where a cast is required and a constant is not set. - // Modify the tree to add an ECast between this node and its parent. - // The output data from this node is copied to the ECast for - // further reads done by the parent. - - ECast ecast = new ECast(location, this, cast); - ecast.statement = statement; - ecast.actual = expected; - ecast.isNull = isNull; - - return ecast; - } else { - if (PainlessLookupUtility.isConstantType(expected)) { - // For the case where a cast is required, a constant is set, - // and the constant can be immediately cast to the expected type. - // An EConstant replaces this node with the constant cast appropriately - // from the constant value defined by this node. Note that - // for constants output data does not need to be copied - // from this node because the output data for the EConstant - // will already be the same. - - constant = AnalyzerCaster.constCast(location, constant, cast); - - EConstant econstant = new EConstant(location, constant); - econstant.analyze(scriptRoot, scope); - - if (!expected.equals(econstant.actual)) { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - - return econstant; - } else if (this instanceof EConstant) { - // For the case where a cast is required, a constant is set, - // the constant cannot be immediately cast to the expected type, - // and this node is already an EConstant. Modify the tree to add - // an ECast between this node and its parent. Note that - // for constants output data does not need to be copied - // from this node because the output data for the EConstant - // will already be the same. - - ECast ecast = new ECast(location, this, cast); - ecast.actual = expected; - - return ecast; - } else { - // For the case where a cast is required, a constant is set, - // the constant cannot be immediately cast to the expected type, - // and this node is not an EConstant. Replace this node with - // an Econstant node copying the constant from this node. - // Modify the tree to add an ECast between the EConstant node - // and its parent. Note that for constants output data does not - // need to be copied from this node because the output data for - // the EConstant will already be the same. - - EConstant econstant = new EConstant(location, constant); - econstant.analyze(scriptRoot, scope); - - if (!actual.equals(econstant.actual)) { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - - ECast ecast = new ECast(location, econstant, cast); - ecast.actual = expected; - - return ecast; - } - } + ECast ecast = new ECast(location, this, cast); + ecast.statement = statement; + ecast.actual = expected; + ecast.isNull = isNull; + + return ecast; } } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBinary.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBinary.java index 2fc9dedfe144e..6deca2498b5c7 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBinary.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBinary.java @@ -116,20 +116,6 @@ private void analyzeMul(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - - if (left.constant != null && right.constant != null) { - if (promote == int.class) { - constant = (int)left.constant * (int)right.constant; - } else if (promote == long.class) { - constant = (long)left.constant * (long)right.constant; - } else if (promote == float.class) { - constant = (float)left.constant * (float)right.constant; - } else if (promote == double.class) { - constant = (double)left.constant * (double)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } } private void analyzeDiv(ScriptRoot scriptRoot, Scope variables) { @@ -160,24 +146,6 @@ private void analyzeDiv(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - - if (left.constant != null && right.constant != null) { - try { - if (promote == int.class) { - constant = (int)left.constant / (int)right.constant; - } else if (promote == long.class) { - constant = (long)left.constant / (long)right.constant; - } else if (promote == float.class) { - constant = (float)left.constant / (float)right.constant; - } else if (promote == double.class) { - constant = (double)left.constant / (double)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } catch (ArithmeticException exception) { - throw createError(exception); - } - } } private void analyzeRem(ScriptRoot scriptRoot, Scope variables) { @@ -208,24 +176,6 @@ private void analyzeRem(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - - if (left.constant != null && right.constant != null) { - try { - if (promote == int.class) { - constant = (int)left.constant % (int)right.constant; - } else if (promote == long.class) { - constant = (long)left.constant % (long)right.constant; - } else if (promote == float.class) { - constant = (float)left.constant % (float)right.constant; - } else if (promote == double.class) { - constant = (double)left.constant % (double)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } catch (ArithmeticException exception) { - throw createError(exception); - } - } } private void analyzeAdd(ScriptRoot scriptRoot, Scope variables) { @@ -268,23 +218,6 @@ private void analyzeAdd(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - - if (left.constant != null && right.constant != null) { - if (promote == int.class) { - constant = (int)left.constant + (int)right.constant; - } else if (promote == long.class) { - constant = (long)left.constant + (long)right.constant; - } else if (promote == float.class) { - constant = (float)left.constant + (float)right.constant; - } else if (promote == double.class) { - constant = (double)left.constant + (double)right.constant; - } else if (promote == String.class) { - constant = left.constant.toString() + right.constant.toString(); - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } - } private void analyzeSub(ScriptRoot scriptRoot, Scope variables) { @@ -315,20 +248,6 @@ private void analyzeSub(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - - if (left.constant != null && right.constant != null) { - if (promote == int.class) { - constant = (int)left.constant - (int)right.constant; - } else if (promote == long.class) { - constant = (long)left.constant - (long)right.constant; - } else if (promote == float.class) { - constant = (float)left.constant - (float)right.constant; - } else if (promote == double.class) { - constant = (double)left.constant - (double)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } } private void analyzeRegexOp(ScriptRoot scriptRoot, Scope variables) { @@ -381,16 +300,6 @@ private void analyzeLSH(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - - if (left.constant != null && right.constant != null) { - if (promote == int.class) { - constant = (int)left.constant << (int)right.constant; - } else if (promote == long.class) { - constant = (long)left.constant << (int)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } } private void analyzeRSH(ScriptRoot scriptRoot, Scope variables) { @@ -429,16 +338,6 @@ private void analyzeRSH(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - - if (left.constant != null && right.constant != null) { - if (promote == int.class) { - constant = (int)left.constant >> (int)right.constant; - } else if (promote == long.class) { - constant = (long)left.constant >> (int)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } } private void analyzeUSH(ScriptRoot scriptRoot, Scope variables) { @@ -477,16 +376,6 @@ private void analyzeUSH(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - - if (left.constant != null && right.constant != null) { - if (promote == int.class) { - constant = (int)left.constant >>> (int)right.constant; - } else if (promote == long.class) { - constant = (long)left.constant >>> (int)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } } private void analyzeBWAnd(ScriptRoot scriptRoot, Scope variables) { @@ -517,16 +406,6 @@ private void analyzeBWAnd(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - - if (left.constant != null && right.constant != null) { - if (promote == int.class) { - constant = (int)left.constant & (int)right.constant; - } else if (promote == long.class) { - constant = (long)left.constant & (long)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } } private void analyzeXor(ScriptRoot scriptRoot, Scope variables) { @@ -556,18 +435,6 @@ private void analyzeXor(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - - if (left.constant != null && right.constant != null) { - if (promote == boolean.class) { - constant = (boolean)left.constant ^ (boolean)right.constant; - } else if (promote == int.class) { - constant = (int)left.constant ^ (int)right.constant; - } else if (promote == long.class) { - constant = (long)left.constant ^ (long)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } } private void analyzeBWOr(ScriptRoot scriptRoot, Scope variables) { @@ -597,16 +464,6 @@ private void analyzeBWOr(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - - if (left.constant != null && right.constant != null) { - if (promote == int.class) { - constant = (int)left.constant | (int)right.constant; - } else if (promote == long.class) { - constant = (long)left.constant | (long)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } } @Override diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBool.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBool.java index d2bc1fcaf5233..ec2407dc92016 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBool.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBool.java @@ -55,16 +55,6 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { right.analyze(scriptRoot, scope); right = right.cast(scriptRoot, scope); - if (left.constant != null && right.constant != null) { - if (operation == Operation.AND) { - constant = (boolean)left.constant && (boolean)right.constant; - } else if (operation == Operation.OR) { - constant = (boolean)left.constant || (boolean)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } - actual = boolean.class; } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBoolean.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBoolean.java index 3964b3f989118..10d941fdce90f 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBoolean.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBoolean.java @@ -22,6 +22,7 @@ import org.elasticsearch.painless.Location; import org.elasticsearch.painless.Scope; import org.elasticsearch.painless.ir.ClassNode; +import org.elasticsearch.painless.ir.ConstantNode; import org.elasticsearch.painless.ir.ExpressionNode; import org.elasticsearch.painless.symbol.ScriptRoot; @@ -30,6 +31,8 @@ */ public final class EBoolean extends AExpression { + protected boolean constant; + public EBoolean(Location location, boolean constant) { super(location); @@ -47,7 +50,12 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { @Override ExpressionNode write(ClassNode classNode) { - throw createError(new IllegalStateException("Illegal tree structure.")); + ConstantNode constantNode = new ConstantNode(); + constantNode.setLocation(location); + constantNode.setExpressionType(actual); + constantNode.setConstant(constant); + + return constantNode; } @Override diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EComp.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EComp.java index f65aaf8a8e063..ff1ac49431da7 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EComp.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EComp.java @@ -100,26 +100,6 @@ private void analyzeEq(ScriptRoot scriptRoot, Scope variables) { throw createError(new IllegalArgumentException("Extraneous comparison of null constants.")); } - if ((left.constant != null || left.isNull) && (right.constant != null || right.isNull)) { - if (promotedType == boolean.class) { - constant = (boolean)left.constant == (boolean)right.constant; - } else if (promotedType == int.class) { - constant = (int)left.constant == (int)right.constant; - } else if (promotedType == long.class) { - constant = (long)left.constant == (long)right.constant; - } else if (promotedType == float.class) { - constant = (float)left.constant == (float)right.constant; - } else if (promotedType == double.class) { - constant = (double)left.constant == (double)right.constant; - } else if (!left.isNull) { - constant = left.constant.equals(right.constant); - } else if (!right.isNull) { - constant = right.constant.equals(null); - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } - actual = boolean.class; } @@ -145,22 +125,6 @@ private void analyzeEqR(ScriptRoot scriptRoot, Scope variables) { throw createError(new IllegalArgumentException("Extraneous comparison of null constants.")); } - if ((left.constant != null || left.isNull) && (right.constant != null || right.isNull)) { - if (promotedType == boolean.class) { - constant = (boolean)left.constant == (boolean)right.constant; - } else if (promotedType == int.class) { - constant = (int)left.constant == (int)right.constant; - } else if (promotedType == long.class) { - constant = (long)left.constant == (long)right.constant; - } else if (promotedType == float.class) { - constant = (float)left.constant == (float)right.constant; - } else if (promotedType == double.class) { - constant = (double)left.constant == (double)right.constant; - } else { - constant = left.constant == right.constant; - } - } - actual = boolean.class; } @@ -191,26 +155,6 @@ private void analyzeNE(ScriptRoot scriptRoot, Scope variables) { throw createError(new IllegalArgumentException("Extraneous comparison of null constants.")); } - if ((left.constant != null || left.isNull) && (right.constant != null || right.isNull)) { - if (promotedType == boolean.class) { - constant = (boolean)left.constant != (boolean)right.constant; - } else if (promotedType == int.class) { - constant = (int)left.constant != (int)right.constant; - } else if (promotedType == long.class) { - constant = (long)left.constant != (long)right.constant; - } else if (promotedType == float.class) { - constant = (float)left.constant != (float)right.constant; - } else if (promotedType == double.class) { - constant = (double)left.constant != (double)right.constant; - } else if (!left.isNull) { - constant = !left.constant.equals(right.constant); - } else if (!right.isNull) { - constant = !right.constant.equals(null); - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } - actual = boolean.class; } @@ -236,22 +180,6 @@ private void analyzeNER(ScriptRoot scriptRoot, Scope variables) { throw createError(new IllegalArgumentException("Extraneous comparison of null constants.")); } - if ((left.constant != null || left.isNull) && (right.constant != null || right.isNull)) { - if (promotedType == boolean.class) { - constant = (boolean)left.constant != (boolean)right.constant; - } else if (promotedType == int.class) { - constant = (int)left.constant != (int)right.constant; - } else if (promotedType == long.class) { - constant = (long)left.constant != (long)right.constant; - } else if (promotedType == float.class) { - constant = (float)left.constant != (float)right.constant; - } else if (promotedType == double.class) { - constant = (double)left.constant != (double)right.constant; - } else { - constant = left.constant != right.constant; - } - } - actual = boolean.class; } @@ -278,20 +206,6 @@ private void analyzeGTE(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - if (left.constant != null && right.constant != null) { - if (promotedType == int.class) { - constant = (int)left.constant >= (int)right.constant; - } else if (promotedType == long.class) { - constant = (long)left.constant >= (long)right.constant; - } else if (promotedType == float.class) { - constant = (float)left.constant >= (float)right.constant; - } else if (promotedType == double.class) { - constant = (double)left.constant >= (double)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } - actual = boolean.class; } @@ -318,20 +232,6 @@ private void analyzeGT(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - if (left.constant != null && right.constant != null) { - if (promotedType == int.class) { - constant = (int)left.constant > (int)right.constant; - } else if (promotedType == long.class) { - constant = (long)left.constant > (long)right.constant; - } else if (promotedType == float.class) { - constant = (float)left.constant > (float)right.constant; - } else if (promotedType == double.class) { - constant = (double)left.constant > (double)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } - actual = boolean.class; } @@ -358,20 +258,6 @@ private void analyzeLTE(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - if (left.constant != null && right.constant != null) { - if (promotedType == int.class) { - constant = (int)left.constant <= (int)right.constant; - } else if (promotedType == long.class) { - constant = (long)left.constant <= (long)right.constant; - } else if (promotedType == float.class) { - constant = (float)left.constant <= (float)right.constant; - } else if (promotedType == double.class) { - constant = (double)left.constant <= (double)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } - actual = boolean.class; } @@ -398,20 +284,6 @@ private void analyzeLT(ScriptRoot scriptRoot, Scope variables) { left = left.cast(scriptRoot, variables); right = right.cast(scriptRoot, variables); - if (left.constant != null && right.constant != null) { - if (promotedType == int.class) { - constant = (int)left.constant < (int)right.constant; - } else if (promotedType == long.class) { - constant = (long)left.constant < (long)right.constant; - } else if (promotedType == float.class) { - constant = (float)left.constant < (float)right.constant; - } else if (promotedType == double.class) { - constant = (double)left.constant < (double)right.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } - actual = boolean.class; } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EConditional.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EConditional.java index c4516aa26dcc5..2f3c04e8d899c 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EConditional.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EConditional.java @@ -24,6 +24,7 @@ import org.elasticsearch.painless.Scope; import org.elasticsearch.painless.ir.ClassNode; import org.elasticsearch.painless.ir.ConditionalNode; +import org.elasticsearch.painless.lookup.PainlessLookupUtility; import org.elasticsearch.painless.symbol.ScriptRoot; import java.util.Objects; @@ -51,10 +52,6 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { condition.analyze(scriptRoot, scope); condition = condition.cast(scriptRoot, scope); - if (condition.constant != null) { - throw createError(new IllegalArgumentException("Extraneous conditional statement.")); - } - left.expected = expected; left.explicit = explicit; left.internal = internal; @@ -67,7 +64,13 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { right.analyze(scriptRoot, scope); if (expected == null) { - Class promote = AnalyzerCaster.promoteConditional(left.actual, right.actual, left.constant, right.constant); + Class promote = AnalyzerCaster.promoteConditional(left.actual, right.actual); + + if (promote == null) { + throw createError(new ClassCastException("cannot apply a conditional operator [?:] to the types " + + "[" + PainlessLookupUtility.typeToCanonicalTypeName(left.actual) + "] and " + + "[" + PainlessLookupUtility.typeToCanonicalTypeName(right.actual) + "]")); + } left.expected = promote; right.expected = promote; diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EConstant.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EConstant.java index dbca094ba51b8..c02beca4ec044 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EConstant.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EConstant.java @@ -31,6 +31,8 @@ */ final class EConstant extends AExpression { + protected Object constant; + EConstant(Location location, Object constant) { super(location); @@ -65,7 +67,6 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { @Override ConstantNode write(ClassNode classNode) { ConstantNode constantNode = new ConstantNode(); - constantNode.setLocation(location); constantNode.setExpressionType(actual); constantNode.setConstant(constant); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EDecimal.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EDecimal.java index 1d62c0587f752..049e52799f1f7 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EDecimal.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EDecimal.java @@ -22,6 +22,7 @@ import org.elasticsearch.painless.Location; import org.elasticsearch.painless.Scope; import org.elasticsearch.painless.ir.ClassNode; +import org.elasticsearch.painless.ir.ConstantNode; import org.elasticsearch.painless.ir.ExpressionNode; import org.elasticsearch.painless.symbol.ScriptRoot; @@ -34,6 +35,8 @@ public final class EDecimal extends AExpression { private final String value; + protected Object constant; + public EDecimal(Location location, String value) { super(location); @@ -69,7 +72,12 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { @Override ExpressionNode write(ClassNode classNode) { - throw createError(new IllegalStateException("Illegal tree structure.")); + ConstantNode constantNode = new ConstantNode(); + constantNode.setLocation(location); + constantNode.setExpressionType(actual); + constantNode.setConstant(constant); + + return constantNode; } @Override diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EElvis.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EElvis.java index 45cfdab5e8c34..89eabb439ba72 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EElvis.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EElvis.java @@ -61,7 +61,11 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { if (lhs.isNull) { throw createError(new IllegalArgumentException("Extraneous elvis operator. LHS is null.")); } - if (lhs.constant != null) { + if (lhs instanceof EBoolean + || lhs instanceof ENumeric + || lhs instanceof EDecimal + || lhs instanceof EString + || lhs instanceof EConstant) { throw createError(new IllegalArgumentException("Extraneous elvis operator. LHS is a constant.")); } if (lhs.actual.isPrimitive()) { @@ -72,7 +76,7 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { } if (expected == null) { - Class promote = AnalyzerCaster.promoteConditional(lhs.actual, rhs.actual, lhs.constant, rhs.constant); + Class promote = AnalyzerCaster.promoteConditional(lhs.actual, rhs.actual); lhs.expected = promote; rhs.expected = promote; diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENumeric.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENumeric.java index 0699475adbf1c..5c689f090acaf 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENumeric.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENumeric.java @@ -22,6 +22,7 @@ import org.elasticsearch.painless.Location; import org.elasticsearch.painless.Scope; import org.elasticsearch.painless.ir.ClassNode; +import org.elasticsearch.painless.ir.ConstantNode; import org.elasticsearch.painless.ir.ExpressionNode; import org.elasticsearch.painless.symbol.ScriptRoot; @@ -35,6 +36,8 @@ public final class ENumeric extends AExpression { private final String value; private int radix; + protected Object constant; + public ENumeric(Location location, String value, int radix) { super(location); @@ -111,7 +114,12 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { @Override ExpressionNode write(ClassNode classNode) { - throw createError(new IllegalStateException("Illegal tree structure.")); + ConstantNode constantNode = new ConstantNode(); + constantNode.setLocation(location); + constantNode.setExpressionType(actual); + constantNode.setConstant(constant); + + return constantNode; } @Override diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EString.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EString.java index 62367437dc648..423ff49e521b9 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EString.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EString.java @@ -22,6 +22,7 @@ import org.elasticsearch.painless.Location; import org.elasticsearch.painless.Scope; import org.elasticsearch.painless.ir.ClassNode; +import org.elasticsearch.painless.ir.ConstantNode; import org.elasticsearch.painless.ir.ExpressionNode; import org.elasticsearch.painless.symbol.ScriptRoot; @@ -32,6 +33,8 @@ */ public final class EString extends AExpression { + protected String constant; + public EString(Location location, String string) { super(location); @@ -49,7 +52,12 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { @Override ExpressionNode write(ClassNode classNode) { - throw new IllegalStateException("Illegal tree structure."); + ConstantNode constantNode = new ConstantNode(); + constantNode.setLocation(location); + constantNode.setExpressionType(actual); + constantNode.setConstant(constant); + + return constantNode; } @Override diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EUnary.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EUnary.java index 5d10612d1f6ab..93d0969da3c91 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EUnary.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EUnary.java @@ -72,10 +72,6 @@ void analyzeNot(ScriptRoot scriptRoot, Scope variables) { child.analyze(scriptRoot, variables); child = child.cast(scriptRoot, variables); - if (child.constant != null) { - constant = !(boolean)child.constant; - } - actual = boolean.class; } @@ -92,16 +88,6 @@ void analyzeBWNot(ScriptRoot scriptRoot, Scope variables) { child.expected = promote; child = child.cast(scriptRoot, variables); - if (child.constant != null) { - if (promote == int.class) { - constant = ~(int)child.constant; - } else if (promote == long.class) { - constant = ~(long)child.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } - if (promote == def.class && expected != null) { actual = expected; } else { @@ -122,20 +108,6 @@ void analyzerAdd(ScriptRoot scriptRoot, Scope variables) { child.expected = promote; child = child.cast(scriptRoot, variables); - if (child.constant != null) { - if (promote == int.class) { - constant = +(int)child.constant; - } else if (promote == long.class) { - constant = +(long)child.constant; - } else if (promote == float.class) { - constant = +(float)child.constant; - } else if (promote == double.class) { - constant = +(double)child.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } - if (promote == def.class && expected != null) { actual = expected; } else { @@ -156,20 +128,6 @@ void analyzerSub(ScriptRoot scriptRoot, Scope variables) { child.expected = promote; child = child.cast(scriptRoot, variables); - if (child.constant != null) { - if (promote == int.class) { - constant = -(int)child.constant; - } else if (promote == long.class) { - constant = -(long)child.constant; - } else if (promote == float.class) { - constant = -(float)child.constant; - } else if (promote == double.class) { - constant = -(double)child.constant; - } else { - throw createError(new IllegalStateException("Illegal tree structure.")); - } - } - if (promote == def.class && expected != null) { actual = expected; } else { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SDo.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SDo.java index 1e8873f259056..41afc52f878b3 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SDo.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SDo.java @@ -64,8 +64,8 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { condition.analyze(scriptRoot, scope); condition = condition.cast(scriptRoot, scope); - if (condition.constant != null) { - continuous = (boolean)condition.constant; + if (condition instanceof EBoolean) { + continuous = ((EBoolean)condition).constant; if (!continuous) { throw createError(new IllegalArgumentException("Extraneous do while loop.")); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFor.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFor.java index 9d8d3e3006936..970ba8d877f48 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFor.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFor.java @@ -79,8 +79,8 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { condition.analyze(scriptRoot, scope); condition = condition.cast(scriptRoot, scope); - if (condition.constant != null) { - continuous = (boolean)condition.constant; + if (condition instanceof EBoolean) { + continuous = ((EBoolean)condition).constant; if (!continuous) { throw createError(new IllegalArgumentException("Extraneous for loop.")); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SIf.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SIf.java index 5b13be46b254b..f00866a6ef422 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SIf.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SIf.java @@ -48,7 +48,7 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { condition.analyze(scriptRoot, scope); condition = condition.cast(scriptRoot, scope); - if (condition.constant != null) { + if (condition instanceof EBoolean) { throw createError(new IllegalArgumentException("Extraneous if statement.")); } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SIfElse.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SIfElse.java index 38b258557a289..518094a10c1a4 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SIfElse.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SIfElse.java @@ -53,7 +53,7 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { condition.analyze(scriptRoot, scope); condition = condition.cast(scriptRoot, scope); - if (condition.constant != null) { + if (condition instanceof EBoolean) { throw createError(new IllegalArgumentException("Extraneous if statement.")); } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SWhile.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SWhile.java index 5192e06793f73..47909b6ded674 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SWhile.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SWhile.java @@ -52,8 +52,8 @@ void analyze(ScriptRoot scriptRoot, Scope scope) { condition.analyze(scriptRoot, scope); condition = condition.cast(scriptRoot, scope); - if (condition.constant != null) { - continuous = (boolean)condition.constant; + if (condition instanceof EBoolean) { + continuous = ((EBoolean)condition).constant; if (!continuous) { throw createError(new IllegalArgumentException("Extraneous while loop.")); diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BaseClassTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BaseClassTests.java index d4679787806ce..578efb25dbff9 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BaseClassTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BaseClassTests.java @@ -557,7 +557,6 @@ public void testReturnsPrimitiveDouble() throws Exception { .newInstance().execute(), 0); String debug = Debugger.toString(ReturnsPrimitiveDouble.class, "1", new CompilerSettings()); - assertThat(debug, containsString("DCONST_1")); // The important thing here is that we have the bytecode for returning a double instead of an object assertThat(debug, containsString("DRETURN")); diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/StringTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/StringTests.java index 31870b9125cb3..c04c1d9304425 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/StringTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/StringTests.java @@ -167,12 +167,12 @@ public void testStringAndCharacter() { assertEquals('c', exec("String s = \"c\"; (char)s")); assertEquals('c', exec("String s = 'c'; (char)s")); - ClassCastException expected = expectScriptThrows(ClassCastException.class, false, () -> { + ClassCastException expected = expectScriptThrows(ClassCastException.class, () -> { assertEquals("cc", exec("return (String)(char)\"cc\"")); }); assertTrue(expected.getMessage().contains("cannot cast java.lang.String with length not equal to one to char")); - expected = expectScriptThrows(ClassCastException.class, false, () -> { + expected = expectScriptThrows(ClassCastException.class, () -> { assertEquals("cc", exec("return (String)(char)'cc'")); }); assertTrue(expected.getMessage().contains("cannot cast java.lang.String with length not equal to one to char"));