[foreign-memaccess+abi] RFR: 8311533: SegmentAllocator::allocateArray call can be ambiguous

[Jorn Vernee]

On Wed, 12 Jul 2023 10:20:43 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> This PR updates the name of some methods in the SegmentAllocator interface. As discussed in [1], the current naming scheme suffers from the following ambiguity:
> 
> allocator.allocateArray(JAVA_LONG, 3);
> 
> 
> This callsite actually matches two signatures:
> 
> * `allocateArray(MemoryLayout, long)`
> * `allocateArray(ValueLayout.OfLong, long...)`
> 
> Since the former is non-variadic, it is preferred. Note how the semantics of these two methods is completely different - the former allocates a segment that can store N longs (where N is the parameter being passed). The latter allocates a segment that is big enough to hold the provided array.
> 
> So, in the case of the above call, the first method would allocate 8 * 3 = 24 bytes, whereas the latter would only allocate 8 bytes.
> 
> To rectify this, we started from the observation that a call like:
> 
> 
> allocator.allocate(JAVA_LONG, 3);
> 
> 
> Will probably be intuitively associated with a calloc where the element size is 8 and the element count is 3 (e.g. request to allocate 24 bytes). So:
> 
> * `allocateArray(MemoryLayout, long)` has been renamed to just `allocate(MemoryLayout, long)`.
> * all the allocation methods that allocate some segment and initialize it to some provided value (which could be an array or a single value) are renamed from `allocate`/`allocateArray` to just `allocateFrom`.
> 
> I think this naming scheme is more robust, and less surprising. While there are still multiple candidates in a call like this:
> 
> 
> allocator.allocateFrom(JAVA_LONG, 3);
> 
> 
> Now all the candidates are semantically equivalent (allocate 8 bytes and stick the value `3` in it), and overload resolution would pick the variant that has less overhead, which is what we want.
> 
> There's an open question (which I have not addressed, and I'd like feedback on) as to whether the two `allocateString` methods should also be renamed to `allocateFrom`, since they also allocate a segment from a provided value. This would make naming more consistent (currently, the two `allocateString` methods stick out a little bit when staring at the javadoc).

This look good, thanks!

WRT renaming `allocateString` to `allocateFrom`, I'd say let's go for it. I don't think calling these methods `allocateString` makes it much clearer what they do when seeing such a call, and it's nice to have the consistency.

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

Marked as reviewed by jvernee (Committer).

PR Review: https://git.openjdk.org/panama-foreign/pull/845#pullrequestreview-1526296747