On Tue, 29 Apr 2025 20:01:23 GMT, Chen Liang <li...@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
