[foreign-memaccess] RFR 8234581: Cleanup use of exceptions in the memory access API

[Paul Sandoz]

Looks good,
Paul.

> On Nov 21, 2019, at 3:51 PM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
> 
> I did another pass - here's the delta webrev:
> 
> http://cr.openjdk.java.net/~mcimadamore/panama/8234581_v2_delta/ <http://cr.openjdk.java.net/~mcimadamore/panama/8234581_v2_delta/>
> and here's the new javadoc:
> 
> http://cr.openjdk.java.net/~mcimadamore/panama/8234581_v2_javadoc/jdk/incubator/foreign/package-summary.html <http://cr.openjdk.java.net/~mcimadamore/panama/8234581_v2_javadoc/jdk/incubator/foreign/package-summary.html>
> Thanks
> Maurizio
> 
> 
> 
> On 21/11/2019 17:57, Paul Sandoz wrote:
>> 
>> 
>>> On Nov 21, 2019, at 9:49 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com <mailto:maurizio.cimadamore at oracle.com>> wrote:
>>> 
>>> 
>>> On 21/11/2019 17:45, Paul Sandoz wrote:
>>>> Good cleanup.
>>> thanks
>>>> 
>>>> 
>>>> You might be able to use Objects.checkFromIndexSize in MemorySegmentImpl instead of your own checkBounds method (it’s possible to customize the exception message if you use the internal Preconditions.checkFromIndexSize).
>>> That works on ints doesn't it? The implemented one is on longs. So, while maybe doable (e.g. splitting the check in two) doesn't seem that straightforward?
>> 
>> Doh, I missed types! (We should add long accepting check* methods to Objects as this API matures).
>> 
>> 
>>>> 
>>>> 
>>>> I generally try to avoid the use of < etc because I find it makes it harder to read the doc in source (I tend to read doc more in source than in the generated HTML).
>>>> 
>>>> * @throws IllegalArgumentException if the start index is < 0, or if the step is = 0.
>>>> ->
>>>> * @throws IllegalArgumentException if {@code start < 0} or {@code step == 0}.
>>>> 
>>>>  174      * @throws IndexOutOfBoundsException if the new segment bounds are illegal; this can happen because:
>>>>  175      * <ul>
>>>>  176      * <li>{@code offset} is negative or greater than {@code byteSize()}</li>
>>>>  177      * <li>{@code newSize} is negative or {@code newSize > byteSize() - offset}</li>
>>>>  178      * </ul>
>>>> ->
>>>> 
>>>> @throws IndexOutOfBoundsException if {@code offset < 0}, {@code offset > byteSize()}, {@code newSize < 0}, or  {@code newSize > byteSize() - offset}
>>>> 
>>>> (Since we are using IndexOutOfBoundsException we can be a little more terse.)
>>>> 
>>>> If you are ok with that I am happy to make such a change after your patch has settled down and pushed.
>>> 
>>> I can do that too - I plan to do another pass anyway to remove the 'throws' clauses which mention unchecked exceptions
>>> 
>> 
>> Ok.
>> 
>> Thanks,
>> Paul.
>>