On Wed, 6 Sep 2023 13:54:28 GMT, ExE Boss <d...@openjdk.org> wrote:
>> Adam Sotona has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> magic moved to Util::isAttributeAllowed and fixed possible NPE
>
> src/java.base/share/classes/jdk/internal/classfile/impl/AbstractDirectBuilder.java
> line 59:
>
>> 57: if (a instanceof UnboundAttribute ||
>> 58: 5 - a.attributeMapper().attributeStability().ordinal()
>> 59: > context.attributesProcessingOption().ordinal()) {
>
> Maybe extract the `5 - a.attributeMapper().attributeStability().ordinal() >
> context.attributesProcessingOption().ordinal()` check into a helper method,
> as the current expression is too noisy:
>
> import jdk.internal.classfile.AttributeMapper.AttributeStability;
> import jdk.internal.classfile.Classfile.AttributesProcessingOption;
>
> private static final int ATTRIBUTE_STABILITY_COUNT =
> AttributeStability.values().length;
> public static boolean isAttributeAllowed(
> final AttributeStability stability,
> final AttributesProcessingOption processingOption
> ) {
> return ATTRIBUTE_STABILITY_COUNT - stability.ordinal() >
> processingOption.ordinal();
> }
>
>
> Also, the magic number should probably be replaced with a constant storing
> the value of `AttributeStability.values().length`.
Fixed, thanks!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15101#discussion_r1317422600
