Re: RFR: 8308645: Javadoc of FFM API needs to be refreshed [v3]
On Fri, 26 May 2023 18:11:14 GMT, Maurizio Cimadamore
wrote:
>> As the FFM API evolved over time, some parts of the javadoc went out of
>> sync. Now that most of the API is stable, it is a good time to look again at
>> the javadoc as a whole, and bring some more consistency.
>>
>> While most of the changes in this PR are stylistic, I'd like to call out few
>> things which resulted in API changes:
>>
>> * `MemorySegment::asSlice(long, long, long)` did not (incorrectly) specified
>> requirement that its alignment parameter must be a power of two.
>>
>> * `MemoryLayout::sliceHandle` did not require the absence of dereference
>> paths in the provided layout path. While that is technically possible to
>> allow, currently the method is specified as returning a "slice"
>> corresponding to some "nested layout", so following pointers seems
>> incompatible with the spec. I have decided to disallow for now - we can
>> always compatibly enable it later, if required.
>>
>> * `MemorySegment::copy` - most of the overloads did not specify that
>> `UnsupportedOperationException` is thrown if the destination segment is
>> read-only.
>>
>> * In several places, an extra `@throws IllegalArgumentException` or `@throws
>> IllegalArgumentException` has been added to capture cases where element size
>> * index computation can overflow. This is true for all the element-wise
>> segment copy methods, for the indexed accessors in memory segment (e.g.
>> `MemorySegment.getAtIndex(ValueLayout.OfInt, long)`), as well as for
>> `SegmentAllocator::allocateArray(MemoryLayout, long)`.
>>
>> In all these cases (except for overflows), new tests have been added to
>> cover the additional API changes (a CSR will also be filed to cover these).
>>
>> The class with most changes is `MemoryLayout`. I realized that the javadoc
>> there was a bit sloppy around the definition of "layout paths". Now there
>> are several subsection in the class javadoc, which explain how different
>> kinds of paths can be used. I have introduced the notion of "open path
>> elements" to denote those path elements that are not fully specified, and
>> result in additional var handle coordinates to be added. There is also now a
>> definition of what it means for a layout path to be "well-formed", so that
>> all methods accepting a layout path can simply refer to it (this definition
>> is tricky to give inline in the javadoc of the various path-accepting
>> methods, as it depends on many factors).
>>
>> Another biggie change was in `MemorySegment` as now all dereference methods
>> state precisely their spatial checks requirements, as well also specifying
>> when the API can th...
>
> Maurizio Cimadamore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Improve javadoc on supported linker layouts
src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 187:
> 185: * ).withName("point")
> 186: * )
> 187: * ).withName("points")
Wrong indentation:
Suggestion:
* )
* ).withName("points")
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14098#discussion_r1207238878
Re: RFR: 8308645: Javadoc of FFM API needs to be refreshed [v3]
> As the FFM API evolved over time, some parts of the javadoc went out of sync. > Now that most of the API is stable, it is a good time to look again at the > javadoc as a whole, and bring some more consistency. > > While most of the changes in this PR are stylistic, I'd like to call out few > things which resulted in API changes: > > * `MemorySegment::asSlice(long, long, long)` did not (incorrectly) specified > requirement that its alignment parameter must be a power of two. > > * `MemoryLayout::sliceHandle` did not require the absence of dereference > paths in the provided layout path. While that is technically possible to > allow, currently the method is specified as returning a "slice" corresponding > to some "nested layout", so following pointers seems incompatible with the > spec. I have decided to disallow for now - we can always compatibly enable it > later, if required. > > * `MemorySegment::copy` - most of the overloads did not specify that > `UnsupportedOperationException` is thrown if the destination segment is > read-only. > > * In several places, an extra `@throws IllegalArgumentException` or `@throws > IllegalArgumentException` has been added to capture cases where element size > * index computation can overflow. This is true for all the element-wise > segment copy methods, for the indexed accessors in memory segment (e.g. > `MemorySegment.getAtIndex(ValueLayout.OfInt, long)`), as well as for > `SegmentAllocator::allocateArray(MemoryLayout, long)`. > > In all these cases (except for overflows), new tests have been added to cover > the additional API changes (a CSR will also be filed to cover these). > > The class with most changes is `MemoryLayout`. I realized that the javadoc > there was a bit sloppy around the definition of "layout paths". Now there are > several subsection in the class javadoc, which explain how different kinds of > paths can be used. I have introduced the notion of "open path elements" to > denote those path elements that are not fully specified, and result in > additional var handle coordinates to be added. There is also now a definition > of what it means for a layout path to be "well-formed", so that all methods > accepting a layout path can simply refer to it (this definition is tricky to > give inline in the javadoc of the various path-accepting methods, as it > depends on many factors). > > Another biggie change was in `MemorySegment` as now all dereference methods > state precisely their spatial checks requirements, as well also specifying > when the API can throw because of an ... Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Improve javadoc on supported linker layouts - Changes: - all: https://git.openjdk.org/jdk/pull/14098/files - new: https://git.openjdk.org/jdk/pull/14098/files/df6c1239..6703eee9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14098=02 - incr: https://webrevs.openjdk.org/?repo=jdk=14098=01-02 Stats: 69 lines in 3 files changed: 45 ins; 3 del; 21 mod Patch: https://git.openjdk.org/jdk/pull/14098.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14098/head:pull/14098 PR: https://git.openjdk.org/jdk/pull/14098
