RFR: 8321387: SegmentAllocator:allocateFrom(AddressLayout, MemorySegment) does not throw stated UnsupportedOperationException

[Maurizio Cimadamore]

On Wed, 6 Dec 2023 14:09:31 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> 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.

After offline discussion: the distinction between UOE and IAE is mostly fictional and doesn't add much. In fact, such distinction is harmful because it would mean that code written using var handles cannot cleanly migrate to use memory segment getter/setter (and vice-versa). As such, I recommend that the implementation is left alone, and IAE is thrown by the var handle accessors for read-only segments. Memory segment setters should similarly throw IAE for read-only, because the setters are just thin easy to use wrappers around the var handle.

A similar argument holds for MemorySegment::copyFrom - here throwing UOE would be more apt, but given we explain this method as calling the static MemorySegment::copy (which throws IAE) I would again avoid exception mismatch which would be detrimental to refactoring.

At which point we're left with MemorySegment::fill. While UOE would be better here, it would be the only method in MS issuing UOE for a read-only issue, so I think it would be just best to regularize and use IAE here too.

tl;dr; the implementation is already throwing the correct set of exceptions (which is not perfect, but better than the alternatives). The javadoc should be updated to reflect that.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16993#discussion_r1417426024