[foreign-abi] RFR: 8239345: need an enum for standard C types and a way to get a memory layout for those C types

Maurizio Cimadamore mcimadamore at openjdk.java.net
Wed Feb 19 19:08:54 UTC 2020


On Wed, 19 Feb 2020 17:47:51 GMT, Athijegannathan Sundararajan <sundar at openjdk.org> wrote:

> SystemABI.Type enum added and ABI_CLASS attribute of layout removed (inferred from Type instead)

Looks good - few minor nits here and there around usage of optional.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/CallArranger.java line 206:

> 205:         var optAbiType = baseType.abiType();
> 206:         ArgumentClassImpl baseArgClass = optAbiType.isPresent()? AArch64ABI.argumentClassFor(optAbiType.get()) : null;
> 207:         if (baseArgClass != ArgumentClassImpl.VECTOR)

Missing space before `?` Consider replacing with Optional::orElseThrow

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/CallArranger.java line 212:

> 211:             optAbiType = elem.abiType();
> 212:             ArgumentClassImpl argClass = optAbiType.isPresent()? AArch64ABI.argumentClassFor(optAbiType.get()) : null;
> 213:             if (!(elem instanceof ValueLayout) ||

Missing space before `?` Consider replacing with Optional::orElseThrow

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/x64/sysv/CallArranger.java line 432:

> 431:         ArrayList<ArgumentClassImpl> classes = new ArrayList<>();
> 432:         var optAbiType = type.abiType();
> 433:         if (!optAbiType.isPresent()) {

Consider replacing with Optional::orElseThrow

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/x64/windows/CallArranger.java line 165:

> 164:         var optAbiType = type.abiType();
> 165:         if (!optAbiType.isPresent()) {
> 166:             //padding not allowed here

Consider replacing with Optional::orElseThrow

test/jdk/java/foreign/NativeTestHelper.java line 40:

> 39:         var optAbiType = layout.abiType();
> 40:         if (!optAbiType.isPresent()) {
> 41:             return false;

Consider replacing with Optional::orElseThrow

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

Marked as reviewed by mcimadamore (Committer).

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


More information about the panama-dev mailing list