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

Maurizio Cimadamore mcimadamore at openjdk.org
Wed Dec 6 15:41:39 UTC 2023


On Wed, 6 Dec 2023 15:32:54 GMT, Per Minborg <pminborg at 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`).
>
> Per Minborg has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update throws docs fror SegmentAllocator

src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java line 95:

> 93:      * @param str the Java string to be converted into a C string
> 94:      * @return a new native segment containing the converted C string
> 95:      * @throws IllegalArgumentException if the allocated segment is

I don't think the changes here are useful. What does it mean for an allocated segment to be read-only? I think all these conditions are tied to `prefixAllocator` blindly accepting read-only segments, which should NOT be the case. I suggest to revert all the chnages here and document (and throw) a new exception for when a prefix allocator is created from a read-only segment.

src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java line 367:

> 365: 
> 366:     @ForceInline
> 367:     public void checkAccess(long offset, long length, AccessConstraint access) {

These changes should be reverted, I don't think we use the UOE mode anymore. Just revert the impl to what it was before (also true for VarHandle templates).

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

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


More information about the core-libs-dev mailing list