On Tue, 7 Jan 2025 11:30:13 GMT, Per Minborg <[email protected]> wrote:
>> This PR proposes to improve the current `MemorySegment.toString()` method
>> from:
>>
>> `MemorySegment{ address: 0x60000264c540, byteSize: 8 }`
>>
>> to
>>
>> `MemorySegment{ native, address: 0x60000264c540, byteSize: 8}`
>>
>> Tests passes tier1-tier3
>
> Per Minborg has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains four additional
> commits since the last revision:
>
> - Reduce elements in toString
> - Merge branch 'master' into ms-tostring-improvement2
> - Implement an alternate MS::toString
> - Improve MemorySegment::toString
src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java
line 490:
> 488: public String toString() {
> 489: return "MemorySegment{ " +
> 490: heapBase().map(hb -> "heapBase: " +
> hb).orElse(isMapped() ? "mapped" : "native") +
This is not correct. Heap base can be empty for read-only segments.
Have we considered printing one of:
* HeapMemorySegment
* NativeMemorySegment
* MappedMemorySegment
E.g. add the heap/native/mapped as part of the "class" qualifier, before the
`{` ? That seems more readable (of course the drawback is that it might suggest
that these classes exist somewhere, which they don't.
A possible compromise would be:
MemorySegment{ kind: heap/native/mapped, [heapBase: xyz ], address: xyz, size:
xyz}
This is consistent with the terminology found in the javadoc:
> There are two kinds of memory segments: ...
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22826#discussion_r1905337003
