RFR(S): JDK-8247938: Change various JVM enums like LinkInfo::AccessCheck and Klass::DefaultsLookupMode to enum class
Kim Barrett
kim.barrett at oracle.com
Sat Aug 1 09:47:13 UTC 2020
> On Jul 31, 2020, at 1:55 PM, Lois Foltan <lois.foltan at oracle.com> wrote:
>
> Please review the webrev to change the following unscoped enumeration declarations to scoped enumerations.
>
> Klass::DefaultsLookupMode
> Klass::OverpassLookupMode
> Klass::StaticLookupMode
> Klass::PrivateLookupMode
> LinkInfo::AccessCheck
>
> With C++11/14 enum class provides the added type safety to prevent 0 (integral) or false (boolean) to be implicitly converted to AccessCheck::needs_access_check for example.
>
> open webrev at:http://cr.openjdk.java.net/~lfoltan/bug_jdk8247938.0/webrev/ <http://cr.openjdk.java.net/~lfoltan/bug_jdk8247938.0/webrev/index.html>
> bug link: https://bugs.openjdk.java.net/browse/JDK-8247938
>
> Testing: hs-tier1 & 2 complete, hs-tier3-6 in progress
>
> Thanks,
> Lois
Looks good.
For what it's worth, I didn't notice any cases here that seemed like they
would significantly benefit from C++20 "using enum":
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1099r5.html
(Though it could have been used to signficantly reduce the size of the change.)
However, it does seem like some of the enumerators might benefit from a name
change, since they are now referred to by names that are qualified by the
enum type name. For example, instead of this:
199 enum class DefaultsLookupMode { find_defaults, skip_defaults };
200 enum class OverpassLookupMode { find_overpass, skip_overpass };
201 enum class StaticLookupMode { find_static, skip_static };
202 enum class PrivateLookupMode { find_private, skip_private };
maybe use this:
199 enum class DefaultsLookupMode { find, skip };
200 enum class OverpassLookupMode { find, skip };
201 enum class StaticLookupMode { find, skip };
202 enum class PrivateLookupMode { find, skip };
since (for example) DefaultsLookupMode::find is obviously distinct from
StaticLookupMode::find, with the numerator suffixes being redundant.
Similarly, in class LinkInfo, perhaps:
enum class AccessCheck { required, skip };
But that sort of thing can be done separately, assuming it's deemed
desirable. Getting the benfit of the improved type safety is the important
part of this change.
More information about the hotspot-runtime-dev
mailing list