RFR: 8312522: Implementation of Foreign Function & Memory API [v28]

Jorn Vernee jvernee at openjdk.org
Wed Sep 27 16:15:30 UTC 2023


On Wed, 27 Sep 2023 15:04:12 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Jorn Vernee has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Fix visibility issues
>>    
>>    Reviewed-by: mcimadamore
>>  - Review comments
>
> src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 1103:
> 
>> 1101:      * @throws WrongThreadException          if this method is called from a thread {@code T},
>> 1102:      *                                       such that {@code isAccessibleBy(T) == false}.
>> 1103:      * @throws UnsupportedOperationException if {@code charset} is not a {@linkplain StandardCharsets standard charset}.
> 
> The caller can fix/avoid the exception by providing another value for the argument so I think IAE is the unchecked exception for this case rather than UOE.

I agree. I'll make the change for the following `CharSet` accepting methods: `MemorySegment::getString(long,Charset)`, `MemorySegment::setString(long,String,Charset)`, and `SegmentAllocator::allocateFrom(String,Charset)`. (Which should be all of them).

> src/java.base/share/classes/sun/launcher/LauncherHelper.java line 640:
> 
>> 638:                 if (!enableNativeAccess.equals("ALL-UNNAMED")) {
>> 639:                     throw new IllegalArgumentException("Only ALL-UNNAMED allowed as value for " + ENABLE_NATIVE_ACCESS);
>> 640:                 }
> 
> I don't think throwing IAE is right here. It should call abort with a key for the error message. The value of enableNativeAccess can be used as the parameter for the message.

Thanks for the suggestion! I'll switch this to using `abort` instead.

Side note: I don't believe I have to add all the different error message translations right? Only the English version?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15103#discussion_r1338855648
PR Review Comment: https://git.openjdk.org/jdk/pull/15103#discussion_r1338857229


More information about the build-dev mailing list