On Tue, 20 Aug 2024 21:13:29 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org>
wrote:
>> src/java.base/share/classes/jdk/internal/foreign/Utils.java line 245:
>>
>>> 243:
>>> 244: @ForceInline
>>> 245: public static void checkEnclosingLayout(MemorySegment segment,
>>> long offset, MemoryLayout enclosing, boolean readOnly) {
>>
>> Can't the first argument be `AbstractMemorySegmentImpl`? The new call site
>> already has an `AbstractMemorySegmentImpl` and the private static method
>> site can do the cast instead.
>
> I suppose it could yes - any reason as to why moving the cast around is
> better?
Just stylistic - you cast `segment` to `AbstractMemorySegmentImpl` twice here,
and if you count here
https://github.com/openjdk/jdk/pull/20647/files#diff-e483572155b915ded5f6290c0e91fcf3feeaadf117865ea744920b9b9bbbec45R103
you have already casted 3 times in var handles. You can change the type here
and add one new cast here:
https://github.com/openjdk/jdk/pull/20647/files#diff-8b4feba9593ad63edaad23970fff28004f916bfa2bf45970f63fad83fb46cd92R289
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20647#discussion_r1724002020
