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