[foreign-memaccess] [Rev 01] RFR: Add MemorySegment::fill

[Maurizio Cimadamore]

On Wed, 13 May 2020 17:14:02 GMT, Paul Sandoz <psandoz at openjdk.org> wrote:

>> Chris Hegarty has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review comment: testFillWithNull -> testFillNull
>
> Marked as reviewed by psandoz (Committer).

Thought a bit more about this.

There's an additional angle to the problem of segment vs. address which we didn't discuss. While all segments have
a *base address*, the reverse is not true: if an address is *unchecked* (e.g. created from `MemoryAddress::ofLong`) it
has no backing segment and can therefore not be dereferenced.

Because of this, one could argue that the signature of the `MemoryAddress::copy` is overly general, as it allows
both *checked* and *unchecked* addresses, while only clearly making sense for the former.

A way out would be again, to express `copy` in terms of segment - and, as John points out, the starting point could be
to look at `System::arraycopy` - e.g. assuming that `MemorySegment` ~= array:

static void copy(MemorySegment src, long srcOffset, MemorySegment dst, long dstOffset, long length)

This is all and well; now we can only pass segments to the routine, which rules out the *unchecked* address issue. But
still feels a bit redundant - after all, we can easily express offsets in terms of segment slices (which was pointed
out by Paul at some point). So we can actually do this:

static void copy(MemorySegment src, MemorySegment dst)

where `dst` is assumed to be bigger (or equal) than `src`, where both starting offsets are assumed to be zero and where
the length of the copy is assumed to be the length of the `src` segment. Since now the signature is expressed in terms
of segment, it makes sense to move the method under `MemorySegment` too.

This is nice, but I think that once we view this method as operating on segments, I think it makes sense to turn them
into proper instance methods, and just do:

void copyTo(MemorySegment dst)
Can we do the same with `fill`? I think so:

void fill(byte value)
Very simple (instance) method.

And, looking a bit ahead, we will want to have a functionality equivalent to `Arrays.mismatch` - which we can also
express as an instance method:

int mismatch(MemorySegment that)

So my recommendation would be for:

1. moving all methods under `MemorySegment`
2. express all signatures in terms of `MemorySegment`
3. make them instance methods (rather than static)

This should lead to a more self-consistent, and discoverable API. Thoughts?

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

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