RFR: 8257912: Convert enum iteration to use range-based for loops

Gerard Ziemski gziemski at openjdk.java.net
Wed Dec 9 19:47:32 UTC 2020


On Wed, 9 Dec 2020 12:09:41 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> * For JDK 17*
>> 
>> Now that [range-based for loops](https://github.com/openjdk/jdk/pull/1488) are allowed by the [HotSpot Style Guide](https://github.com/openjdk/jdk/blob/52ab72127ddc3ca6575d9d58503ec39c5dff7ab1/doc/hotspot-style.md), I have converted the more verbose syntax:
>> 
>> for (vmIntrinsicsIterator it = vmIntrinsicsRange.begin(); it != vmIntrinsicsRange.end(); ++it) {
>>   vmIntrinsicID index = *it;
>>   nt[as_int(index)] = string;
>> }
>> 
>> to
>> 
>>   for (vmIntrinsicID index : EnumRange<vmIntrinsicID>{}) {
>>     nt[as_int(index)] = string;
>>   }
>> 
>> I also removed the "convenient" declarations such as `vmIntrinsicsRange` and `vmIntrinsicsIterator` -- these are useful only when writing a traditional for loop, and are probably confusing to everyone else.  I've left examples in enumIterator.hpp in case anyone wants to write a traditional for loop.
>> 
>> I also added gtest cases for using range-based for loops with `EnumRange<>`.
>
> Lgtm

I like the change very much.

Did you consider leaving the convenience symbol "vmSymbolsIterator" as is and then using it in the loop, so instead of:

`for (vmSymbolID index : EnumRange<vmSymbolID>{}) {`

we would have:

`for (vmSymbolID index : vmSymbolsIterator) {`

which would be more concise (but less self-documenting I guess) and more similar to:

`for (JVMFlagOrigin origin : range) {`

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

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


More information about the hotspot-dev mailing list