[foreign-memaccess+abi] RFR: 8274157: java.foreign: Add method MemorySegment::isOverlapping [v3]

[Maurizio Cimadamore]

On Mon, 4 Oct 2021 12:57:00 GMT, Julia Boes <jboes at openjdk.org> wrote:

>> This change adds a method to the MemorySegment API that detects if two segments are overlapping.
>
> Julia Boes has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add test for overlap and offsetOf and update names

Getting there - some nitpicks around docs and naming.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 324:

> 322: 
> 323:     /**
> 324:      * Returns a new slice of this segment that is the overlap of the two

Suggestion:

     * Returns a slice of this segment that is the overlap between this and the provided segment.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 339:

> 337: 
> 338:     /**
> 339:      * Returns the offset, in bytes, from this segment to the other segment.

Suggestion:

     * Returns the offset, in bytes, of the provided segment, relative to this segment.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 341:

> 339:      * Returns the offset, in bytes, from this segment to the other segment.
> 340:      *
> 341:      * <p>The offset is relative to the {@linkplain #address() base address} of

Note that MemorySegment::address is only defined for native segments. Perhaps remove the link?

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 349:

> 347:      *         segment.
> 348:      */
> 349:     long offsetTo(MemorySegment other);

Not too sure about the name :-) I think overall I prefer `offsetOf` - although the main issue is to find a name which is clear about the direction. After looking at the test, I'm not sure `segmentOffset` would look bad?

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java line 260:

> 258:             final long thisStart = this.min();
> 259:             final long thatStart = that.min();
> 260:             final long thisEnd = thisStart + this.byteSize() - 1L;

do you need the `-1L` (since then you need `<=`) ? E.g. can we drop the `- 1L` and use strict comparisons instead?

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

PR: https://git.openjdk.java.net/panama-foreign/pull/585