On Wed, 6 Dec 2023 14:06:00 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org>
wrote:
>> This PR proposes to change the exception type for exceptions thrown for
>> certain methods with a parameter of type `MemorySegment` when it is
>> `MemorySegment::isReadOnly`. Previously an `UnsupportedOperationException`
>> was specified but in some cases, in reality, an `IllegalArgumentException`
>> was thrown.
>>
>> The principle used in this PR is that operations acting on an MS where the
>> MS is `this` should throw an `UnsupportedOperationException` whereas in
>> cases where the MS is a *parameter* an `IllegalArgumentException` should be
>> thrown.
>>
>> It should be noted that this PR retains the previous behavior for MS
>> VarHandle access (even though the MS is a parameter to the accessor methods,
>> the first parameter can be said to represent `this`).
>
> src/java.base/share/classes/java/lang/invoke/X-VarHandleSegmentView.java.template
> line 153:
>
>> 151: static void set(VarHandle ob, Object obb, long base, $type$ value) {
>> 152: VarHandleSegmentViewBase handle = (VarHandleSegmentViewBase)ob;
>> 153: AbstractMemorySegmentImpl bb = checkAddress(obb, base,
>> handle.length, ON_WRITE_UOE);
>
> I don't think this should throw UOE. It should throw IAE, because the bad
> segment is passed to the var handle. I also realize that this exception is
> not captured in the documentation of MemoryLayout::varHandle. So that should
> be fixed as well.
This is actually tricky: for var handles it seems like the right exception is
IAE - but then for MemorySegment::get the right exception is UOE (because the
receiver itself is read-only). We might need to give up consistency a bit, as
otherwise I don't think memory segment getter/setter can reuse the VarHandle
implementation.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16993#discussion_r1417386458
