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

[Maurizio Cimadamore]

On Wed, 3 Aug 2022 09:10:48 GMT, Per-Åke Minborg <duke at openjdk.org> wrote:

>> src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 1903:
>> 
>>> 1901:      * @return a Stream of human-readable, lines with hexadecimal values
>>> 1902:      */
>>> 1903:     default Stream<String> hexStream() {
>> 
>> I think `hexDump` or `hexDumpStream` would be a better name
>
> I think it is possible to specify the `hexDump()` method in detail and also make it more general so it can be applied to virtually any memory representation such as `byte[]` or `ByteBuffer`.

Yep, I do think that hex dump doesn't really belong here - but should be a common functionality shared between all byte array views.

>> src/java.base/share/classes/java/lang/foreign/PaddingLayout.java line 38:
>> 
>>> 36:  * This class is immutable, thread-safe and <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>.
>>> 37:  */
>>> 38: public final class PaddingLayout extends AbstractLayout implements MemoryLayout {
>> 
>> I don't think this change should be part of this patch. Whether that's something we want o pursue, it's an orthogonal question (e.g. to make use of pattern matching more nice).
>
> We can re-write the check using a replacement for `instanceof` and revert the PaddingLayout change.

Note that there's a predicate `isPadding`

>> src/java.base/share/classes/jdk/internal/foreign/MemorySegmentRenderUtil.java line 207:
>> 
>>> 205:         if (memoryLayout instanceof GroupLayout groupLayout) {
>>> 206: 
>>> 207:             // Strictly, we should provide all permutations of nested unions.
>> 
>> Not sure I get this comment
>
> So, suppose there is a nested union such that: U = (A|B)|(C|D). Then the actual layout U could be either of AC, AD, BC or BD. Hence, we could present all these combinations.

Ah I see. I think that, perhaps, it would be better to collect all fields in a union, and present them all (like if it was a struct). I don't see any particular problem with that, and I don't think the API should "pick" a field.

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

PR: https://git.openjdk.org/panama-foreign/pull/695