RFR: JDK-8266670: Better modeling of access flags in core reflection [v12]

[ExE Boss]

On Mon, 7 Mar 2022 11:22:29 GMT, Peter Levart <plevart at openjdk.org> wrote:

>>> It does because of the AccessFlags.MODULE constant.
>> 
>> But that’s exactly what the unqualified `MODULE` identifier refers to, and there’s no other bare `MODULE` identifier in scope that would shadow the `AccessFlag.MODULE` constant from the POV of `AccessFlag.LocationToFlags`.
>
> This is just implementation detail, but I think the reverse mapping from Location to AccessFlag(s) could be established implicitly during the initialization of the AccessFlag enum since it is only used from within code of that enum class. Like this:
> 
> 
> Index: src/java.base/share/classes/java/lang/reflect/AccessFlag.java
> IDEA additional info:
> Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
> <+>UTF-8
> ===================================================================
> diff --git a/src/java.base/share/classes/java/lang/reflect/AccessFlag.java b/src/java.base/share/classes/java/lang/reflect/AccessFlag.java
> --- a/src/java.base/share/classes/java/lang/reflect/AccessFlag.java	(revision 1498060544413cb67de8b1a82fbbf15d388d62c3)
> +++ b/src/java.base/share/classes/java/lang/reflect/AccessFlag.java	(date 1646651355828)
> @@ -26,8 +26,13 @@
>  package java.lang.reflect;
>  
>  import java.util.Collections;
> +import java.util.EnumMap;
> +import java.util.EnumSet;
> +import java.util.HashMap;
>  import java.util.Map;
>  import java.util.Set;
> +import java.util.function.Function;
> +
>  import static java.util.Map.entry;
>  
>  /**
> @@ -248,6 +253,14 @@
>          this.mask = mask;
>          this.sourceModifier = sourceModifier;
>          this.locations = locations;
> +        for (var location : locations) {
> +            LocationToFlags.MAP.computeIfAbsent(location, new Function<>() {
> +                @Override
> +                public Set<AccessFlag> apply(Location location) {
> +                    return EnumSet.noneOf(AccessFlag.class);
> +                }
> +            }).add(this);
> +        }
>      }
>  
>      /**
> @@ -283,7 +296,7 @@
>       */
>      public static Set<AccessFlag> maskToAccessFlags(int mask, Location location) {
>          Set<AccessFlag> result = java.util.EnumSet.noneOf(AccessFlag.class);
> -        for (var accessFlag : LocationToFlags.locationToFlags.get(location)) {
> +        for (var accessFlag : LocationToFlags.MAP.get(location)) {
>              int accessMask = accessFlag.mask();
>              if ((mask &  accessMask) != 0) {
>                  result.add(accessFlag);
> @@ -363,34 +376,7 @@
>      }
>  
>      private static class LocationToFlags {
> -        private static Map<Location, Set<AccessFlag>> locationToFlags =
> -            Map.ofEntries(entry(Location.CLASS,
> -                                Set.of(PUBLIC, FINAL, SUPER,
> -                                       INTERFACE, ABSTRACT,
> -                                       SYNTHETIC, ANNOTATION,
> -                                       ENUM, AccessFlag.MODULE)),
> -                          entry(Location.FIELD,
> -                                Set.of(PUBLIC, PRIVATE, PROTECTED,
> -                                       STATIC, FINAL, VOLATILE,
> -                                       TRANSIENT, SYNTHETIC, ENUM)),
> -                          entry(Location.METHOD,
> -                                Set.of(PUBLIC, PRIVATE, PROTECTED,
> -                                       STATIC, FINAL, SYNCHRONIZED,
> -                                       BRIDGE, VARARGS, NATIVE,
> -                                       ABSTRACT, STRICT, SYNTHETIC)),
> -                          entry(Location.INNER_CLASS,
> -                                Set.of(PUBLIC, PRIVATE, PROTECTED,
> -                                       STATIC, FINAL, INTERFACE, ABSTRACT,
> -                                       SYNTHETIC, ANNOTATION, ENUM)),
> -                          entry(Location.METHOD_PARAMETER,
> -                                Set.of(FINAL, SYNTHETIC, MANDATED)),
> -                          entry(Location.MODULE,
> -                                Set.of(OPEN, SYNTHETIC, MANDATED)),
> -                          entry(Location.MODULE_REQUIRES,
> -                                Set.of(TRANSITIVE, STATIC_PHASE, SYNTHETIC, MANDATED)),
> -                          entry(Location.MODULE_EXPORTS,
> -                                Set.of(SYNTHETIC, MANDATED)),
> -                          entry(Location.MODULE_OPENS,
> -                                Set.of(SYNTHETIC, MANDATED)));
> +        private static final Map<Location, Set<AccessFlag>> MAP =
> +            new EnumMap<>(Location.class);
>      }
>  }

@plevart
In your code, you might want to avoid allocating a new `Function` instance for every call of `LocationToFlags.MAP.computeIfAbsent(…)`.

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

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