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

[David Holmes]

On 8/10/2021 6:02 am, Liam Miller-Cushon wrote:
> On Thu, 7 Oct 2021 01:20:04 GMT, David Holmes <dholmes at openjdk.org> wrote:
> 
>>> Liam Miller-Cushon has updated the pull request incrementally with two additional commits since the last revision:
>>>
>>>   - Also test evaluation order for 'inline'
>>>   - Remove sharpestAccessible logic, since types are now passed as strings
>>
>> Can you explain the changes made in relation to the runtime test:
>>
>>   test/hotspot/jtreg/runtime/modules/AccessCheck/MethodAccessReadTwice.java
>>
>> please. The connection to this fix is not at all apparent, nor what the conversion to jasm is achieving.
>>
>> Thanks,
>> David
> 
> @dholmes-ora
> 
>> Can you explain the changes made in relation to the runtime test:
>>
>> test/hotspot/jtreg/runtime/modules/AccessCheck/MethodAccessReadTwice.java
>>
>> please. The connection to this fix is not at all apparent, nor what the conversion to jasm is achieving.
> 
> Thanks, good question:
> 
> That test is for [JDK-8174954](https://bugs.openjdk.java.net/browse/JDK-8174954), which checks for an expected `IllegalAccessError` when the parameter type of an invokedynamic is inaccessible. It's assuming that given `"" + param`, javac generates an invokedynamic that uses the specific type of `param`. This change make javac eagerly convert `param` to a `String` before passing it to the `invokedynamic` call, which avoids the accessibility issue the test is trying to exercise.
> 
> Using `jasm` preserves the existing bytecode (including the signature of the `invokedynamic`) that the test is expecting.

Okay, can you please add a comment to the jasm file that explains that. 
It is useful to know what Java code a jasm file represents and where/why 
it differs from how that Java code would compile.

Thanks,
David
-----

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