Merge pull request #18933 from uschindler/painless-issue-18929

Fix compound assignment with string concats

modules/lang-painless/src/main/java/org/elasticsearch/painless/MethodWriter.java

@@ -209,15 +209,20 @@ public final class MethodWriter extends GeneratorAdapter {
         }
     }
 
-    public void writeNewStrings() {
+    /** Starts a new string concat.
+     * @return the size of arguments pushed to stack (the object that does string concats, e.g. a StringBuilder)
+     */
+    public int writeNewStrings() {
         if (INDY_STRING_CONCAT_BOOTSTRAP_HANDLE != null) {
             // Java 9+: we just push our argument collector onto deque
             stringConcatArgs.push(new ArrayList<>());
+            return 0; // nothing added to stack
         } else {
             // Java 8: create a StringBuilder in bytecode
             newInstance(STRINGBUILDER_TYPE);
             dup();
             invokeConstructor(STRINGBUILDER_TYPE, STRINGBUILDER_CONSTRUCTOR);
+            return 1; // StringBuilder on stack
         }
     }
 

modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java

@@ -131,7 +131,7 @@ public final class WriterConstants {
             // not Java 9 - we set it null, so MethodWriter uses StringBuilder:
             bs = null;
         }
-        INDY_STRING_CONCAT_BOOTSTRAP_HANDLE = null; // Disabled until https://github.com/elastic/elasticsearch/issues/18929
+        INDY_STRING_CONCAT_BOOTSTRAP_HANDLE = bs;
     }
 
     public final static int MAX_INDY_STRING_CONCAT_ARGS = 200;

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EChain.java

@@ -294,8 +294,9 @@ public final class EChain extends AExpression {
         // track types going onto the stack.  This must be done before the
         // links in the chain are read because we need the StringBuilder to
         // be placed on the stack ahead of any potential concatenation arguments.
+        int catElementStackSize = 0;
         if (cat) {
-            writer.writeNewStrings();
+            catElementStackSize = writer.writeNewStrings();
         }
 
         ALink last = links.get(links.size() - 1);
@@ -312,7 +313,7 @@ public final class EChain extends AExpression {
                     // Handle the case where we are doing a compound assignment
                     // representing a String concatenation.
 
-                    writer.writeDup(link.size, 1);         // dup the StringBuilder
+                    writer.writeDup(link.size, catElementStackSize);  // dup the top element and insert it before concat helper on stack
                     link.load(writer);                     // read the current link's value
                     writer.writeAppendStrings(link.after); // append the link's value using the StringBuilder
 
@@ -323,7 +324,7 @@ public final class EChain extends AExpression {
                         writer.writeAppendStrings(expression.actual); // append the expression's value unless it's also a concatenation
                     }
 
-                    writer.writeToStrings(); // put the value of the StringBuilder on the stack
+                    writer.writeToStrings(); // put the value for string concat onto the stack
                     writer.writeCast(back);  // if necessary, cast the String to the lhs actual type
 
                     if (link.load) {