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

[Liam Miller-Cushon]

On Tue, 12 Oct 2021 07:53:42 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> How about code like:
>> 
>>         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?
>
>> ```
>>         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?

@shipilev this is also blocked on the CSR, I think the next step is for it to be 'reviewed by at least one engineer familiar with that technology area', is that something you'd be able to help with? https://bugs.openjdk.java.net/browse/JDK-8274863

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

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