ffi: remove function signature property aliases

Signed-off-by: Renegade334 <contact.9a5d6388@renegade334.me.uk>
PR-URL: #63482
Refs: https://github.com/nodejs/node/pull/62072/changes#r3067834658
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>

Author: Renegade334

doc/api/ffi.md

@@ -124,18 +124,18 @@ such as `0` and `1`; JavaScript `true` and `false` are not accepted.
 
 Functions and callbacks are described with signature objects.
 
-Supported fields:
+Signature objects may contain the following properties, both of which are
+optional:
 
-*`result`, `return`, or `returns` for the return type.
-*`parameters` or `arguments` for the parameter type list.
+*`return` {string} A [type name][type names] specifying the return type of the
+ function or callback. **Default:**`'void'`.
+*`arguments` {string\[]} An array of [type names][] specifying the argument
+ type list of the function or callback. **Default:**`[]`.
 
-Only one return-type field and one parameter-list field may be present in a
-single signature object.
-
-```cjs
+```js
 constsignature= {
-result:'i32',
-parameters: ['i32', 'i32'],
+return:'i32',
+arguments: ['i32', 'i32'],
 };
 ```
 
@@ -193,7 +193,7 @@ import { dlopen } from 'node:ffi';
 
 {
  using handle =dlopen('./mylib.so', {
- add_i32: { parameters: ['i32', 'i32'], result:'i32' },
+ add_i32: { arguments: ['i32', 'i32'], return:'i32' },
  });
 console.log(handle.functions.add_i32(20, 22));
 } // handle.lib.close() is invoked automatically here.
@@ -203,8 +203,8 @@ import { dlopen } from 'node:ffi';
 import { dlopen } from'node:ffi';
 
 const { lib, functions } =dlopen('./mylib.so', {
- add_i32: { parameters: ['i32', 'i32'], result:'i32' },
- string_length: { parameters: ['pointer'], result:'u64' },
+ add_i32: { arguments: ['i32', 'i32'], return:'i32' },
+ string_length: { arguments: ['pointer'], return:'u64' },
 });
 
 console.log(functions.add_i32(20, 22));
@@ -214,8 +214,8 @@ console.log(functions.add_i32(20, 22));
 const { dlopen } =require('node:ffi');
 
 const { lib, functions } =dlopen('./mylib.so', {
- add_i32: { parameters: ['i32', 'i32'], result:'i32' },
- string_length: { parameters: ['pointer'], result:'u64' },
+ add_i32: { arguments: ['i32', 'i32'], return:'i32' },
+ string_length: { arguments: ['pointer'], return:'u64' },
 });
 
 console.log(functions.add_i32(20, 22));
@@ -356,8 +356,8 @@ const { DynamicLibrary } = require('node:ffi');
 
 constlib=newDynamicLibrary('./mylib.so');
 constadd=lib.getFunction('add_i32', {
-parameters: ['i32', 'i32'],
-result:'i32',
+arguments: ['i32', 'i32'],
+return:'i32',
 });
 
 console.log(add(20, 22));
@@ -407,7 +407,7 @@ const { DynamicLibrary } = require('node:ffi');
 constlib=newDynamicLibrary('./mylib.so');
 
 constcallback=lib.registerCallback(
- { parameters: ['i32'], result:'i32' },
+ { arguments: ['i32'], return:'i32' },
  (value) => value *2,
 );
 ```
@@ -417,7 +417,7 @@ Callbacks are subject to the following restrictions:
 * They must be invoked on the same system thread where they were created.
 * They must not throw exceptions.
 * They must not return promises.
-* They must return a value compatible with the declared result type.
+* They must return a value compatible with the declared return type.
 * They must not call `library.close()` on their owning library while running.
 * They must not unregister themselves while running.
 
@@ -465,7 +465,7 @@ JavaScript `number` values that match the declared type.
 
 For 64-bit integer types (`i64` and `u64`), pass JavaScript `bigint` values.
 
-For pointer-like parameters:
+For pointer-like arguments:
 
 *`null` and `undefined` are passed as null pointers.
 *`string` values are copied to temporary NUL-terminated UTF-8 strings for the
@@ -727,3 +727,4 @@ and keep callback and pointer lifetimes explicit on the native side.
 [`--allow-ffi`]: cli.md#--allow-ffi
 [`ffi.toBuffer(pointer, length, copy)`]: #ffitobufferpointer-length-copy
 [`using`]: https://tc39.es/proposal-explicit-resource-management/#sec-using-declarations
+[type names]: #type-names

lib/internal/ffi-shared-buffer.js

@@ -31,18 +31,14 @@ const {
  TypeError,
 }=primordials;
 
-const{
-codes: {
-ERR_INTERNAL_ASSERTION,
-},
-}=require('internal/errors');
+constassert=require('internal/assert');
 
 const{
  DynamicLibrary,
  charIsSigned,
+ kSbArguments,
  kSbInvokeSlow,
- kSbParams,
- kSbResult,
+ kSbReturn,
  kSbSharedBuffer,
  uintptrMax,
 }=internalBinding('ffi');
@@ -159,11 +155,9 @@ function writeNumericArg(view, info, offset, arg, index) {
 return;
 }
 
-/* c8 ignore start */
 // Unreachable: caller filters out non-numeric kinds.
-thrownewERR_INTERNAL_ASSERTION(
-`FFI: writeNumericArg reached with unexpected kind="${kind}"`);
-/* c8 ignore stop */
+/* c8 ignore next */
+assert.fail(`FFI: writeNumericArg reached with unexpected kind="${kind}"`);
 }
 
 // Returns true on fast-path success, false when the caller must fall back
@@ -208,51 +202,46 @@ function inheritMetadata(wrapper, rawFn, nargs) {
 // arguments out of it into invocation-local storage before `ffi_call` and
 // reads the return value back only after, so nested/reentrant calls into
 // the same function are safe.
-functionwrapWithSharedBuffer(rawFn,parameters,resultType){
-if(rawFn===undefined||rawFn===null)returnrawFn;
+functionwrapWithSharedBuffer(rawFn,signature){
+if(rawFn==null)returnrawFn;
 constbuffer=rawFn[kSbSharedBuffer];
 if(buffer===undefined)returnrawFn;
 
 // Callers without explicit signature info (the `functions` accessor
-// patch below) rely on the `kSbParams` / `kSbResult` metadata attached
+// patch below) rely on the `kSbArguments` / `kSbReturn` metadata attached
 // by the native `CreateFunction`.
-if(parameters===undefined)parameters=rawFn[kSbParams];
-if(resultType===undefined)resultType=rawFn[kSbResult];
-// `CreateFunction` always attaches these for SB-eligible functions.
-// Missing here means the native side and this wrapper are out of sync.
-/* c8 ignore start */
-if(parameters===undefined||resultType===undefined){
-thrownewERR_INTERNAL_ASSERTION(
-'FFI: shared-buffer raw function is missing kSbParams or kSbResult');
+letargumentTypes,returnType;
+if(signature===undefined){
+argumentTypes=rawFn[kSbArguments];
+returnType=rawFn[kSbReturn];
+
+// `CreateFunction` always attaches these for SB-eligible functions.
+// Missing here means the native side and this wrapper are out of sync.
+assert(argumentTypes!==undefined&&returnType!==undefined,
+'FFI: shared-buffer raw function is missing kSbArguments or kSbReturn');
+}else{
+argumentTypes=signature.arguments??[];
+returnType=signature.return??'void';
 }
-/* c8 ignore stop */
 
 constslowInvoke=rawFn[kSbInvokeSlow];
 constview=newDataView(buffer);
 letretGetter=null;
-if(resultType!=='void'){
-constretInfo=sbTypeInfo[resultType];
-/* c8 ignore start */
-if(retInfo===undefined){
-thrownewERR_INTERNAL_ASSERTION(
-`FFI: shared-buffer type table missing entry for result type "${resultType}"`);
-}
-/* c8 ignore stop */
+if(returnType!=='void'){
+constretInfo=sbTypeInfo[returnType];
+assert(retInfo!==undefined,
+`FFI: shared-buffer type table missing entry for return type "${returnType}"`);
 retGetter=retInfo.get;
 }
 
-constnargs=parameters.length;
+constnargs=argumentTypes.length;
 constargInfos=[];
 constargOffsets=[];
 letanyPointer=false;
 for(leti=0;i<nargs;i++){
-constinfo=sbTypeInfo[parameters[i]];
-/* c8 ignore start */
-if(info===undefined){
-thrownewERR_INTERNAL_ASSERTION(
-`FFI: shared-buffer type table missing entry for parameter type "${parameters[i]}"`);
-}
-/* c8 ignore stop */
+constinfo=sbTypeInfo[argumentTypes[i]];
+assert(info!==undefined,
+`FFI: shared-buffer type table missing entry for argument type "${argumentTypes[i]}"`);
 // Push the `sbTypeInfo` entry directly (entries with the same `kind`
 // share a shape, keeping `writeNumericArg`'s call sites
 // low-polymorphism) and store offsets in a parallel array to avoid
@@ -267,13 +256,8 @@ function wrapWithSharedBuffer(rawFn, parameters, resultType) {
 // Pointer signatures need a per-arg runtime type check and fall back
 // to the native slow-path invoker for non-BigInt pointer arguments,
 // so arity specialization wouldn't buy much here.
-/* c8 ignore start */
-if(slowInvoke===undefined){
-thrownewERR_INTERNAL_ASSERTION(
-'FFI: shared-buffer raw function with pointer arguments is '+
-'missing kSbInvokeSlow');
-}
-/* c8 ignore stop */
+assert(slowInvoke!==undefined,
+'FFI: shared-buffer raw function with pointer arguments is missing kSbInvokeSlow');
 wrapper=function(...args){
 if(args.length!==nargs){
 throwFFIArgCountError(nargs,args.length);
@@ -542,19 +526,6 @@ function buildNumericWrapper(
 };
 }
 
-// Accept-set mirrors the native `ParseFunctionSignature` in
-// `src/ffi/types.cc`. `ParseFunctionSignature` additionally throws when
-// multiple aliases are set at once. The wrapper runs before the native
-// call, so those conflicts still surface from the native side regardless
-// of which alias we happen to read here.
-functionsigParams(sig){
-returnsig.parameters??sig.arguments??[];
-}
-
-functionsigResult(sig){
-returnsig.result??sig.return??sig.returns??'void';
-}
-
 // The native invoker for SB-eligible symbols is `InvokeFunctionSB`, which
 // reads arguments from the shared buffer populated by
 // `wrapWithSharedBuffer`. These patches make sure every path that surfaces
@@ -563,11 +534,11 @@ function sigResult(sig) {
 constrawGetFunction=DynamicLibrary.prototype.getFunction;
 constrawGetFunctions=DynamicLibrary.prototype.getFunctions;
 
-DynamicLibrary.prototype.getFunction=functiongetFunction(name,sig){
-// Native `DynamicLibrary::GetFunction` validates `sig`, so by the time
-// we have `raw` we know `sig` is a valid object.
-constraw=FunctionPrototypeCall(rawGetFunction,this,name,sig);
-returnwrapWithSharedBuffer(raw,sigParams(sig),sigResult(sig));
+DynamicLibrary.prototype.getFunction=functiongetFunction(name,signature){
+// Native `DynamicLibrary::GetFunction` validates `signature`, so by the time
+// we have `raw` we know `signature` is a valid object.
+constraw=FunctionPrototypeCall(rawGetFunction,this,name,signature);
+returnwrapWithSharedBuffer(raw,signature);
 };
 
 DynamicLibrary.prototype.getFunctions=functiongetFunctions(definitions){
@@ -583,13 +554,12 @@ DynamicLibrary.prototype.getFunctions = function getFunctions(definitions) {
 for(leti=0;i<keys.length;i++){
 constname=keys[i];
 // No `definitions`: native side returned every cached function, so we
-// wrap using each function's own `kSbParams` / `kSbResult` metadata
+// wrap using each function's own `kSbArguments` / `kSbReturn` metadata
 // (same fallback as the `functions` accessor).
 if(definitions===undefined){
 out[name]=wrapWithSharedBuffer(raw[name]);
 }else{
-constsig=definitions[name];
-out[name]=wrapWithSharedBuffer(raw[name],sigParams(sig),sigResult(sig));
+out[name]=wrapWithSharedBuffer(raw[name],definitions[name]);
 }
 }
 returnout;
@@ -602,16 +572,12 @@ DynamicLibrary.prototype.getFunctions = function getFunctions(definitions) {
 // uninitialized buffer.
 constfunctionsDescriptor=
 ObjectGetOwnPropertyDescriptor(DynamicLibrary.prototype,'functions');
-/* c8 ignore start */
-if(functionsDescriptor===undefined||!functionsDescriptor.get){
-// Missing getter means the native and JS sides are out of sync; silently
-// skipping the patch would expose the fast-path-against-uninitialized-buffer
-// footgun this whole block exists to prevent.
-thrownewERR_INTERNAL_ASSERTION(
-'FFI: DynamicLibrary.prototype.functions accessor not found or has no getter');
-}
-/* c8 ignore stop */
-constorigGetter=functionsDescriptor.get;
+constorigGetter=functionsDescriptor?.get;
+// Missing getter means the native and JS sides are out of sync; silently
+// skipping the patch would expose the fast-path-against-uninitialized-buffer
+// footgun this whole block exists to prevent.
+assert(origGetter!==undefined,
+'FFI: DynamicLibrary.prototype.functions accessor not found or has no getter');
 ObjectDefineProperty(DynamicLibrary.prototype,'functions',{
 __proto__: null,
 configurable: true,

src/env_properties.h

@@ -48,8 +48,8 @@
  V(async_id_symbol, "async_id_symbol") \
  V(ffi_sb_shared_buffer_symbol, "ffi_sb_shared_buffer_symbol") \
  V(ffi_sb_invoke_slow_symbol, "ffi_sb_invoke_slow_symbol") \
- V(ffi_sb_params_symbol, "ffi_sb_params_symbol")  \
- V(ffi_sb_result_symbol, "ffi_sb_result_symbol") \
+ V(ffi_sb_arguments_symbol, "ffi_sb_arguments_symbol") \
+ V(ffi_sb_return_symbol, "ffi_sb_return_symbol") \
  V(constructor_key_symbol, "constructor_key_symbol") \
  V(handle_onclose_symbol, "handle_onclose") \
  V(no_message_symbol, "no_message_symbol") \
@@ -293,7 +293,6 @@
  V(password_string, "password") \
  V(path_string, "path") \
  V(pathname_string, "pathname") \
- V(parameters_string, "parameters") \
  V(pending_handle_string, "pendingHandle") \
  V(permission_string, "permission") \
  V(phase_string, "phase") \
@@ -329,9 +328,8 @@
  V(require_string, "require") \
  V(resource_string, "resource") \
  V(result_string, "result") \
- V(return_string, "return") \
- V(returns_string, "returns") \
  V(return_arrays_string, "returnArrays") \
+ V(return_string, "return") \
  V(salt_length_string, "saltLength") \
  V(search_string, "search") \
  V(servername_string, "servername") \