RFR: 8355481: Clean up MHN_copyOutBootstrapArguments [v2]

Coleen Phillimore coleenp at openjdk.org
Thu May 8 13:08:52 UTC 2025


On Thu, 24 Apr 2025 12:24:10 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Hi,
>> 
>> I'd like to integrate this simplification of the code for this loop.
>> 
>> We used to have:
>> 
>> ```c++
>> if (start < 0) {
>>   for (int pseudo_index = -4; pseudo_index < 0; pseudo_index++) {
>>     if (start == pseudo_index) {
>>       if (start >= end || 0 > pos || pos >= buf->length())  break;
>>       // ...
>>     }
>>     start++;
>>   }
>> }
>> 
>> 
>> That's exactly the same as:
>> 
>> 
>> int min_end = MIN2(0, end);
>> while (-4 <= start && start < min_end) {
>>   if (pos >= buf->length()) break;
>>   // ...
>>   start++;
>> }
>> 
>> 
>> but the latter looks like a conventional loop.
>> 
>> I'd consider this a basic cleanup, which is worth doing in the name of maintainability.
>> 
>> I would have liked to change the `-4` to `-1` into actual names, but I've no clue where those come from. It doesn't seem worth it to change them if they just happen to be a kludge relying on internal details, or something like that.
>> 
>> Testing: GHA Tier1
>
> Johan Sjölen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove whitespace

It looks like @PaulSandoz wrote this code and could help review your change.  I wish -4 etc had some const names instead and both versions look the same to me.

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

PR Review: https://git.openjdk.org/jdk/pull/24825#pullrequestreview-2825077033


More information about the hotspot-dev mailing list