On Mon, 29 Jul 2024 23:30:48 GMT, Chen Liang <li...@openjdk.org> wrote:

>> Removes 6 `AccessFlags` factories that do not take class-file versions as 
>> its arguments.
>> 
>> `AccessFlags` is a wrapper around a bit mask to support modifier streaming 
>> in ClassFile API. It additionally supports advanced validation based on 
>> location.
>> 
>> However, as class file versions evolve, we may also need a class file 
>> version argument to ensure that the flags are correctly constructed. For 
>> example, a pre-valhalla class modifier without `ACC_SUPER` should not be 
>> interpreted as a value class. The current factories cannot find good default 
>> class file versions, and if they always assume latest, they will fail in 
>> this scenario.
>> 
>> As a result, we should remove these 6 factories; note that users can still 
>> set the flags via `XxxBuilder::withFlags` with either `int` or 
>> `AccessFlag...` flags. In contrast, these builder APIs can fetch the 
>> previously-passed class file versions, and correctly validate or interpret 
>> these flags. Same story goes for parsing, which can also construct the right 
>> flags with available information.
>> 
>> This enables us to add methods to interpret the logical flags with 
>> version-specific information. If there's need, we can always add a new 
>> `AccessFlags.of(int, AccessFlag.Location, ClassFileFormatVersion)` factory, 
>> given the flexibility from this removal.
>
> Chen Liang has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains three commits:
> 
>  - Reapply import changes after merge
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 
> fix/accessflags-factory
>  - 8337219: AccessFlags factories do not require necessary arguments

Marked as reviewed by asotona (Reviewer).

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

PR Review: https://git.openjdk.org/jdk/pull/20341#pullrequestreview-2207034646

Reply via email to