Simpler regex constants in painless #68486
Conversation
nik9000
added
>non-issue
:Core/Infra/Scripting
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
Replaces the double `Pattern.compile` invocations in painless scripts with the fancy constant injection we added in elastic#68088. This caused one of the tests to fail. It turns out that we weren't fully iterating the IR tree during the constant folding phases. I started experimenting and added a ton of tests that failed. Then I fixed them by changing the IR tree walking code.
|
I've marked this a non-issue because it fixes a bug in unreleased code - the |
| assertEquals(true, exec("return 'foo' ==~ /foo/ ? true : false")); | ||
| assertEquals(1, exec("def i = 0; i += 'foo' ==~ /foo/ ? 1 : 1; return i")); | ||
| assertEquals(1, exec("def i = 0; i += 'foo' ==~ /foo/ ? 1 : 0; return i")); |
There was a problem hiding this comment.
This looks like a mistake.
| @@ -232,6 +232,30 @@ public void testReplaceFirstQuoteReplacement() { | |||
| exec("'the quick brown fox'.replaceFirst(/[aeiou]/, m -> '$' + m.group().toUpperCase(Locale.ROOT))")); | |||
| } | |||
|
|
|||
| public void testStoreInMap() { | |||
There was a problem hiding this comment.
I think it might be useful to have similar tests in the constant folding tests that verify that we fold constants in these positions.
There was a problem hiding this comment.
We could use the bytecode assertion for that kind of test.
| @@ -1216,6 +1216,10 @@ public void visitConstant(ConstantNode irConstantNode, WriteScope writeScope) { | |||
| */ | |||
| String fieldName = irConstantNode.getDecorationValue(IRDConstantFieldName.class); | |||
| Type asmFieldType = MethodWriter.getType(irConstantNode.getDecorationValue(IRDExpressionType.class)); | |||
| if (asmFieldType == null) { | |||
| throw irConstantNode.getLocation() | |||
There was a problem hiding this comment.
Is it correct to say this should never happen outside of bugs?
There was a problem hiding this comment.
Correct. I just wanted to get a better error message for my debugging. Is ok? Should I throw the exception in another way?
There was a problem hiding this comment.
It's good. I just wanted to make sure I wasn't missing something user related.
Replaces the double `Pattern.compile` invocations in painless scripts with the fancy constant injection we added in elastic#68088. This caused one of the tests to fail. It turns out that we weren't fully iterating the IR tree during the constant folding phases. I started experimenting and added a ton of tests that failed. Then I fixed them by changing the IR tree walking code.
Replaces the double `Pattern.compile` invocations in painless scripts with the fancy constant injection we added in #68088. This caused one of the tests to fail. It turns out that we weren't fully iterating the IR tree during the constant folding phases. I started experimenting and added a ton of tests that failed. Then I fixed them by changing the IR tree walking code.
Replaces the double
Pattern.compileinvocations in painless scriptswith the fancy constant injection we added in #68088. This caused one of
the tests to fail. It turns out that we weren't fully iterating the IR
tree during the constant folding phases. I started experimenting and
added a ton of tests that failed. Then I fixed them by changing the IR
tree walking code.