On Mon, 21 Mar 2022 10:45:27 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org> wrote:
> This PR contains the API and implementation changes for JEP-424 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/424 src/java.base/share/classes/java/lang/foreign/CLinker.java line 176: > 174: * @param symbol the address of the target function. > 175: * @param function the function descriptor of the target function. > 176: * @return a new downcall method handle. The method handle type is > <a href="CLinker.html#downcall-method-handles"><em>inferred</em></a> Maybe drop the "new" from this. Since we want to do caching in the future. Suggestion: * @return a downcall method handle. The method handle type is <a href="CLinker.html#downcall-method-handles"><em>inferred</em></a> src/java.base/share/classes/java/lang/foreign/CLinker.java line 199: > 197: * > 198: * @param function the function descriptor of the target function. > 199: * @return a new downcall method handle. The method handle type is > <a href="CLinker.html#downcall-method-handles"><em>inferred</em></a> Suggestion: * @return a downcall method handle. The method handle type is <a href="CLinker.html#downcall-method-handles"><em>inferred</em></a> src/java.base/share/classes/java/lang/foreign/MemoryAddress.java line 159: > 157: * Creates a memory address from the given long value. > 158: * @param value the long value representing a raw address. > 159: * @return a new memory address instance. Similar here. A new address is not always returned. Suggestion: * @return a memory address instance. src/java.base/share/classes/java/lang/foreign/package-info.java line 230: > 228: * where {@code M1}, {@code M2}, {@code ... Mn} are module names (for > the unnamed module, the special value {@code ALL-UNNAMED} > 229: * can be used). If this option is specified, access to restricted > methods is only granted to the modules listed by that > 230: * option. If this option is not specified, access to restricted method > is enabled for all modules, but Suggestion: * option. If this option is not specified, access to restricted methods is enabled for all modules, but src/java.base/share/classes/jdk/internal/foreign/abi/aarch64/CallArranger.java line 53: > (failed to retrieve contents of file, check the PR for context) Keeping this static import would seem more readable here, instead of prefixing everything with `AArch64Architecture.`. (especially in the ABI definition below) src/java.base/share/classes/jdk/internal/foreign/abi/x64/sysv/CallArranger.java line 53: > (failed to retrieve contents of file, check the PR for context) Same here, I think keeping the static import for this would make things more readable. ------------- PR: https://git.openjdk.java.net/jdk/pull/7888
