RFR: 8355481: Clean up MHN_copyOutBootstrapArguments [v2]
Coleen Phillimore
coleenp at openjdk.org
Thu May 8 17:51:53 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
Okay, now I see it in context. Looks good to me.
-------------
Marked as reviewed by coleenp (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/24825#pullrequestreview-2825918319
More information about the hotspot-dev
mailing list