[foreign-memaccess+abi] RFR: 8291639: Improve the ability to visualize a MemorySegment in human readable forms [v18]

Per Minborg pminborg at openjdk.org
Wed Jun 7 11:43:29 UTC 2023


On Mon, 5 Jun 2023 15:27:28 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Per Minborg has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Clean up code
>
> src/java.base/share/classes/java/lang/foreign/GroupLayout.java line 81:
> 
>> 79: 
>> 80:     /**
>> 81:      * {@return a {@link Function} that can project {@linkplain MemorySegment MemorySegments} onto new instances
> 
> Since a new record is returned, 'into' seems more correct. ('onto' sounds like there is an existing record instance).
> Suggestion:
> 
>      * {@return a {@link Function} that can project {@linkplain MemorySegment MemorySegments} into new instances

"project ... onto" makes sense I think. But maybe we can reword here.

> src/java.base/share/classes/java/lang/foreign/GroupLayout.java line 217:
> 
>> 215:      * @since 21
>> 216:      */
>> 217:     <R extends Record> Function<MemorySegment, R> recordMapper(Class<R> type);
> 
> I feel like the documentation is a bit _too_ formal, and will go over some people's heads. In general, I suggest using plain English as the main structure, and then weaving in the formal definitions instead.
> 
> I think by re-ordering some of the information, you can end up with simpler/smaller paragraphs. I think there are 3 main steps that are useful to describe:
> 1. The `recordMapper` function itself finds, for each record component, a corresponding member layout with the same name in this group layout. There are restrictions on the layout type of each corresponding member layout based on the type of the record component (e.g. a java primitive maps to a ValueLayout, MemorySegment maps to an AddressLayout, etc.)
> 2. An extraction function is determined based on the component/layout type combination (and then have a list with the process for each type combination). In this part you can lean on the information defined in 1, and use language like "the corresponding member layout"/"the corresponding record component", rather than `c_a` and `e_b`.
> 3. The canonical constructor of the record type is called with the result of the extraction functions.
> 
> I think 1 & 2 are too mixed up right now in the big formal definition of the extraction functions.

I am happy to simplify. What do you think @mcimadamore ?

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

PR Review Comment: https://git.openjdk.org/panama-foreign/pull/833#discussion_r1221437016
PR Review Comment: https://git.openjdk.org/panama-foreign/pull/833#discussion_r1221437510


More information about the panama-dev mailing list