RFR: 8297271: AccessFlag.maskToAccessFlags should be specific to class file version [v4]

Roger Riggs rriggs at openjdk.org
Tue Apr 29 20:14:53 UTC 2025


On Tue, 29 Apr 2025 20:01:23 GMT, Chen Liang <liach at openjdk.org> wrote:

>> src/java.base/share/classes/java/lang/reflect/AccessFlag.java line 401:
>> 
>>> 399:     public static Set<AccessFlag> maskToAccessFlags(int mask, Location location, ClassFileFormatVersion cffv) {
>>> 400:         var definition = findDefinition(location);
>>> 401:         int unmatchedMask = mask & (~location.flagsMask(cffv)); // implicit null check
>> 
>> Implicit null check is actually in `findDefinition`.
>> There's little harm is looking up the definition before checking for unmatched mask bits but it could be delayed and inlined as the argument to `new AccessFlagSet`.
>
> Oops, this was the implicit null check for cffv. Should I make all of the checks explicit?
> 
> Also, I split out `findDefinition` as in value objects, it will become `findDefinition(location, cffv)`, when the valhalla cffv is passed, findDefinition can return differently for Location.CLASS.

Explicit null checks are better if they provide if they name the caller supplied argument that was null.
Checking for caller errors usually precedes the methods semantics.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24760#discussion_r2067327110


More information about the core-libs-dev mailing list