On Tue, 12 Dec 2023 13:58:42 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org>
wrote:
>> Per Minborg has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Revert change in allocateNoInit
>
> src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java line 396:
>
>> 394: * @throws IllegalArgumentException if {@code elementCount *
>> sourceElementLayout.byteSize()} overflows
>> 395: * @throws IndexOutOfBoundsException if {@code sourceOffset >
>> source.byteSize() - (elementCount * sourceElementLayout.byteSize())}
>> 396: * @throws IndexOutOfBoundsException if either {@code sourceOffset}
>> or {@code elementCount} are {@code < 0}
>
> I think that if elementCount < 0, we should throw IAE, not IOOBE. Note that
> we explain this method as:
>
> MemorySegment dest = this.allocate(elementLayout, elementCount);
> MemorySegment.copy(source, sourceElementLayout, sourceOffset, dest,
> elementLayout, 0, elementCount);
>
>
> So, the set of exceptions thrown by these two methods should be preserved.
> This means that `allocate(MemoryLayout, long)` gets a first pass at checking
> - this means IAE for overflows and also for negative element count.
>
> After that, the exceptions we can have are those specified in
> MemorySegment.copy.
(as we did in other patches, I think it's important here to remain true to the
semantics of the "desugared" code, because that will guarantee that developers
can refactor their code w/o worrying about exceptions changing from under them)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17079#discussion_r1424034360
