Another revision which addresses some additional CSR feedback:

* SequenceLayout::withElementCount should throw if new element count < 0
* MemoryLayout::hasSize should clarify that certain layout (e.g. ValueLayout) always have a size

Webrev:

http://cr.openjdk.java.net/~mcimadamore/8235837_v3

delta from previous iteration:

http://cr.openjdk.java.net/~mcimadamore/8235837_v3_delta

javadoc

http://cr.openjdk.java.net/~mcimadamore/8235837_v3_javadoc

specdiff

http://cr.openjdk.java.net/~mcimadamore/8235837_v3_specdiff/overview-summary.html

Cheers
Maurizio

On 09/01/2020 18:36, Maurizio Cimadamore wrote:
New revision:

http://cr.openjdk.java.net/~mcimadamore/8235837_v2

delta from previous iteration:

http://cr.openjdk.java.net/~mcimadamore/8235837_v2_delta

javadoc

http://cr.openjdk.java.net/~mcimadamore/8235837_v2_javadoc

specdiff

http://cr.openjdk.java.net/~mcimadamore/8235837_v2_specdiff/overview-summary.html

Cheers
Maurizio

On 09/01/2020 14:36, Maurizio Cimadamore wrote:
Hi,
following the JEP 370 code review and CSR, certain comments that have not been addressed have been added to the tracker issue:

https://bugs.openjdk.java.net/browse/JDK-8235837

For further evaluation. After some discussion in the Panama channel (see [1] and [2]), the Panama team has come to the following conclusions:

* the MemoryAddress::offset(long) method should be renamed to MemoryAddress::addOffset(long) * the MemorySegment::isAccessible() predicate should be dropped and replaced with the MemorySegment::ownerThread() accessor * a new predicate should be added to MemoryLayout, namely MemoryLayout::hasSize() to allow clients to establish as to whether a layout contains some unsized parts * a new method should be added to SequenceLayout, namely SequenceLayout::withElementCount, to create a new sequence layout with new element count
* 2 new helper methods should be added to MemoryLayout, namely:
   - MemoryLayout::map - to allow transformation of a layout into a new layout (by replacing the sub-layout at given path with a new layout)    - MemoryLayout::select - to allow selection of a sub-layout given a layout path

Overall, we think this set of change strikes a good compromise between usability, readability and minimality.

The javadoc for the layout path-accepting methods (both old and new ones) has been significantly refactored and improved - and the toplevel javadoc section on layout paths has been enhanced to cover all the concepts. The implementation for these methods has also been consolidated greatly, which revealed subtle bugs in the old implementation which have now been rectified (new tests have been added).

Webrev is available here:

http://cr.openjdk.java.net/~mcimadamore/8235837/

Specdiff for changes here:

http://cr.openjdk.java.net/~mcimadamore/8235837_specdiff/overview-summary.html

A draft CSR for this issue is available here (I will finalize once the review converges on the API changes):

https://bugs.openjdk.java.net/browse/JDK-8236853

I plan to push this to JDK 14 under the late enhancement process [3].

Cheers
Maurizio

[1] - https://mail.openjdk.java.net/pipermail/panama-dev/2019-December/006873.html [2] - https://mail.openjdk.java.net/pipermail/panama-dev/2019-December/006885.html
[3] - https://openjdk.java.net/jeps/3#Late-Enhancement-Request-Process





Reply via email to