RFR: JDK-8273914: Indy string concat changes order of operations [v3]

[Aleksey Shipilev]

On Tue, 12 Oct 2021 07:23:32 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

> ```
>         StringBuilder builder2 = new StringBuilder("foo");
>         Object oo = builder2;
>         oo += "" + builder2.append("bar"); 
> ```
> 
> Should that be covered as well? From looking at the patch (not really trying it), it does not seem to be covered?

Tried it, still not correct:


$ cat Concat.java 
public class Concat {
	public static void main(String... args) {
	        StringBuilder builder2 = new StringBuilder("foo");
	        Object oo = builder2;
        	oo += "" + builder2.append("bar");
		System.out.println(oo);
	}
}

$ build/linux-x86_64-server-fastdebug/images/jdk/bin/javac Concat.java
$ build/linux-x86_64-server-fastdebug/images/jdk/bin/java Concat
foobarfoobar


I believe `if (shouldConvertToStringEagerly(argType))` branch should be handled for first argument as well, i.e. code should be shaped as:


if (!first || generateFirstArg) {
    genExpr(arg, arg.type).load();
}
if (shouldConvertToStringEagerly(argType)) {
    gen.callMethod(pos, syms.stringType, names.valueOf, List.of(syms.objectType), true);
    argType = syms.stringType;
}
dynamicArgs.add(argType);


This produces the correct result:


$ build/linux-x86_64-server-fastdebug/images/jdk/bin/java Concat
foofoobar


@cushon, could you do this change and add a relevant test case?

-------------

PR: https://git.openjdk.java.net/jdk/pull/5844