On Wed, 27 Sep 2023 15:04:12 GMT, Alan Bateman <[email protected]> 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
