Skip to content

Commit

Permalink
Painless: improve error message on non-constant
Browse files Browse the repository at this point in the history
As of elastic#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.
  • Loading branch information
nik9000 committed Feb 4, 2021
1 parent e686e18 commit 497e810
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit 497e810

Please sign in to comment.