RFR: 8253402: Convert vmSymbols::SID to enum class [v2]
Coleen Phillimore
coleenp at openjdk.java.net
Fri Sep 25 23:03:43 UTC 2020
On Fri, 25 Sep 2020 22:23:34 GMT, Ioi Lam <iklam at openjdk.org> wrote:
>> Convert `vmSymbols::SID` to an `enum class` to provide better type safety.
>>
>> - The original enum type `vmSymbols::SID` cannot be forward-declared. I moved it out of the `vmSymbols` class and
>> renamed, so now it can be forward-declared as `enum class vmSymbolID : int;`, without including the large vmSymbols.hpp
>> file.
>> - This also breaks the mutual dependency between the `vmSymbols` and `vmIntrinsics` classes. Now the declaration of
>> `vmIntrinsics` can be moved from vmSymbols.hpp to vmIntrinsics.hpp, where it naturally belongs.
>> - Type-safe enumeration (contributed by Kim Barrett)
>> for (vmSymbolID index : vmSymbolsIterator()) {
>> vm_symbol_index[as_int(index)] = index;
>> }
>> - I moved `vmSymbols::_symbols[]` to `Symbol::_vm_symbols[]`, and made it accessible via `Symbol::vm_symbol_at()`. This
>> way, header files (e.g. fieldInfo.hpp) that need to convert from `vmSymbolID` to `Symbol*` don't need to include the
>> large vmSymbols.hpp file.
>> - I changed the `VM_SYMBOL_ENUM_NAME` macro so that the users don't need to explicitly add the `vmSymbolID::` scope.
>> - I removed many unnecessary casts between `int` and `vmSymbolID`.
>> - The remaining casts are done via `vmSymbol::as_int()` and `vmSymbols::as_SID()` with range checks.
>>
>> -----
>> If this is successful, I will do the same for `vmIntrinsics::ID`.
>
> Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes
> the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last
> revision:
> - more vmEnums.hpp fixes; fixed minimal VM build
> - Merge branch 'master' into 8253402-convert-vmsymbols-sid-to-enum-class
> - Moved forward declaration of vmSymbolID to vmEnums.hpp
> - clean up whitespaces and removed useless comment
> - removed unnecessary include
> - 8253402: Convert vmSymbols::SID to enum class
src/hotspot/share/utilities/vmEnums.hpp line 33:
> 31:
> 32: enum JVMFlagsEnum : int;
> 33: enum class vmSymbolID : int;
I don't like that you mixed two different enums that are used in different places here. Can you go back?
-------------
PR: https://git.openjdk.java.net/jdk/pull/276
More information about the hotspot-dev
mailing list