[foreign-memaccess+abi] RFR: ValueLayouts: Improve test coverage and more

[Jorn Vernee]

On Fri, 10 Feb 2023 13:56:18 GMT, Per Minborg <pminborg at openjdk.org> wrote:

> This PR:
> 
> * Adds tests to `ValueLayout` methods
> * Simplifies an `equals()` method
> * Changes from exception throwing to assert for parameters that cannot be changed client-side

src/java.base/share/classes/jdk/internal/foreign/layout/ValueLayouts.java line 155:

> 153:             assert carrier.isPrimitive()
> 154:                     ? bitSize == (carrier == boolean.class ? 8 : Wrapper.forPrimitiveType(carrier).bitWidth())
> 155:                     : true;

Could simplify to:
Suggestion:

            assert carrier != MemorySegment.class || bitSize == ADDRESS_SIZE_BITS;
            assert !carrier.isPrimitive() || bitSize == (carrier == boolean.class ? 8 : Wrapper.forPrimitiveType(carrier).bitWidth());

src/java.base/share/classes/jdk/internal/foreign/layout/ValueLayouts.java line 159:

> 157: 
> 158:         static boolean isValidCarrier(Class<?> carrier) {
> 159:             return carrier.isPrimitive()

This changes the semantics as `void.class.isPrimitive()` is also true.

test/jdk/java/foreign/TestLayouts.java line 52:

> 50:     @Test
> 51:     public void testNotEquals() {
> 52:         List<MemoryLayout> basic = Stream.concat(Stream.of(basicLayouts), Stream.of(ADDRESS)).toList();

Rather than using a stream and `forEach`, I suggest using a `@DataProvider` method, to stick with the theme in the existing tests. (That would also create a log line for each layout, so it's just a little bit easier to see which case(s) failed if there's a failure)

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

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