From 497e810cd442436e570724d36c50d3b03e106b63 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 4 Feb 2021 08:20:58 -0500 Subject: [PATCH] Painless: improve error message on non-constant As of #68088 painless can have methods where all parameters must be constant. This improves the error message when the parameter isn't. It's still not super great, but its better and its what we can easilly give at that point in the compiler. --- .../DefaultConstantFoldingOptimizationPhase.java | 11 ++++++++--- .../org/elasticsearch/painless/BindingsTests.java | 4 ++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/phase/DefaultConstantFoldingOptimizationPhase.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/phase/DefaultConstantFoldingOptimizationPhase.java index adcfa3cc0fe52..6b83ba9288aea 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/phase/DefaultConstantFoldingOptimizationPhase.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/phase/DefaultConstantFoldingOptimizationPhase.java @@ -841,12 +841,17 @@ private void replaceCallWithConstant( Object[] args = new Object[irInvokeCallMemberNode.getArgumentNodes().size()]; for (int i = 0; i < irInvokeCallMemberNode.getArgumentNodes().size(); i++) { ExpressionNode argNode = irInvokeCallMemberNode.getArgumentNodes().get(i); - if (argNode instanceof ConstantNode == false) { + IRDConstant constantDecoration = argNode.getDecoration(IRDConstant.class); + if (constantDecoration == null) { // TODO find a better string to output throw irInvokeCallMemberNode.getLocation() - .createError(new IllegalArgumentException("all arguments must be constant but the [" + (i + 1) + "] argument isn't")); + .createError( + new IllegalArgumentException( + "all arguments to [" + javaMethod.getName() + "] must be constant but the [" + (i + 1) + "] argument isn't" + ) + ); } - args[i] = ((ConstantNode) argNode).getDecorationValue(IRDConstant.class); + args[i] = constantDecoration.getValue(); } Object result; try { diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java index b6c8f1afb4987..b525035886332 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java @@ -211,7 +211,7 @@ public void testClassMethodCompileTimeOnlyVariableParams() { IllegalArgumentException.class, () -> scriptEngine.compile(null, "def a = 2; classMul(2, a)", BindingsTestScript.CONTEXT, Collections.emptyMap()) ); - assertThat(e.getMessage(), equalTo("all arguments must be constant but the [2] argument isn't")); + assertThat(e.getMessage(), equalTo("all arguments to [classMul] must be constant but the [2] argument isn't")); } public void testClassMethodCompileTimeOnlyThrows() { @@ -240,7 +240,7 @@ public void testInstanceMethodCompileTimeOnlyVariableParams() { IllegalArgumentException.class, () -> scriptEngine.compile(null, "def a = 2; instanceMul(a, 2)", BindingsTestScript.CONTEXT, Collections.emptyMap()) ); - assertThat(e.getMessage(), equalTo("all arguments must be constant but the [1] argument isn't")); + assertThat(e.getMessage(), equalTo("all arguments to [instanceMul] must be constant but the [1] argument isn't")); } public void testCompileTimeOnlyParameterFoldedToConstant() {