[embind] Change how the number of arguments are verified. (#22591)

This is really two changes, but they both will affect the same code so I
did them at once:
- The number of arguments is now only verified with ASSERTIONS enabled.
This will help code size and the speed of embind invoker functions.
 - Allow optional arguments to be omitted.

Fixes #22389

Author: brendandahl

ChangeLog.md

@@ -21,6 +21,9 @@ See docs/process.md for more on how version tagging works.
 3.1.68 (in development)
 -----------------------
 - The freetype port was updated from v2.6 to v2.13.3. (#22585)
+- The number of arguments passed to Embind function calls is now only verified
+  with ASSERTIONS enabled. (#22591)
+- Optional arguments can now be omitted from Embind function calls. (#22591)
 
 3.1.67 - 09/17/24
 -----------------

src/embind/embind.js

@@ -57,6 +57,8 @@ var LibraryEmbind = {
     // TODO: do we need a deleteObject here?  write a test where
     // emval is passed into JS via an interface
   }`,
+  $EmValOptionalType__deps: ['$EmValType'],
+  $EmValOptionalType: '=Object.assign({optional: true}, EmValType);',
   $init_embind__deps: [
     '$getInheritedInstanceCount', '$getLiveInheritedInstances',
     '$flushPendingDeletes', '$setDelayFunction'],
@@ -687,9 +689,9 @@ var LibraryEmbind = {
     __embind_register_emval(rawType);
   },
 
-  _embind_register_optional__deps: ['_embind_register_emval'],
+  _embind_register_optional__deps: ['$registerType', '$EmValOptionalType'],
   _embind_register_optional: (rawOptionalType, rawType) => {
-    __embind_register_emval(rawOptionalType);
+    registerType(rawOptionalType, EmValOptionalType);
   },
 
   _embind_register_memory_view__deps: ['$readLatin1String', '$registerType'],
@@ -778,6 +780,10 @@ var LibraryEmbind = {
 #endif
 #if ASYNCIFY
     '$Asyncify',
+#endif
+#if ASSERTIONS
+    '$getRequiredArgCount',
+    '$checkArgCount',
 #endif
   ],
   $craftInvokerFunction: function(humanName, argTypes, classType, cppInvokerFunc, cppTargetFunc, /** boolean= */ isAsync) {
@@ -821,15 +827,18 @@ var LibraryEmbind = {
 
     var returns = (argTypes[0].name !== "void");
 
-#if DYNAMIC_EXECUTION == 0 && !EMBIND_AOT
     var expectedArgCount = argCount - 2;
+#if ASSERTIONS
+    var minArgs = getRequiredArgCount(argTypes);
+#endif
+#if DYNAMIC_EXECUTION == 0 && !EMBIND_AOT
     var argsWired = new Array(expectedArgCount);
     var invokerFuncArgs = [];
     var destructors = [];
     var invokerFn = function(...args) {
-      if (args.length !== expectedArgCount) {
-        throwBindingError(`function ${humanName} called with ${args.length} arguments, expected ${expectedArgCount}`);
-      }
+#if ASSERTIONS
+      checkArgCount(args.length, minArgs, expectedArgCount, humanName, throwBindingError);
+#endif
 #if EMSCRIPTEN_TRACING
       Module.emscripten_trace_enter_context(`embind::${humanName}`);
 #endif
@@ -901,6 +910,9 @@ var LibraryEmbind = {
       }
     }
   }
+#if ASSERTIONS
+  closureArgs.push(checkArgCount, minArgs, expectedArgCount);
+#endif
 
 #if EMBIND_AOT
   var signature = createJsInvokerSignature(argTypes, isClassMethodFunc, returns, isAsync);

src/embind/embind_shared.js

@@ -204,7 +204,29 @@ var LibraryEmbindShared = {
     return signature.join('');
   },
 
-  $createJsInvoker__deps: ['$usesDestructorStack'],
+  $checkArgCount(numArgs, minArgs, maxArgs, humanName, throwBindingError) {
+    if (numArgs < minArgs || numArgs > maxArgs) {
+      var argCountMessage = minArgs == maxArgs ? minArgs : `${minArgs} to ${maxArgs}`;
+      throwBindingError(`function ${humanName} called with ${numArgs} arguments, expected ${argCountMessage}`);
+    }
+  },
+
+  $getRequiredArgCount(argTypes) {
+    var requiredArgCount = argTypes.length - 2;
+    for (var i = argTypes.length - 1; i >= 2; --i) {
+      if (!argTypes[i].optional) {
+        break;
+      }
+      requiredArgCount--;
+    }
+    return requiredArgCount;
+  },
+
+  $createJsInvoker__deps: ['$usesDestructorStack',
+#if ASSERTIONS
+    '$checkArgCount',
+#endif
+  ],
   $createJsInvoker(argTypes, isClassMethodFunc, returns, isAsync) {
     var needsDestructorStack = usesDestructorStack(argTypes);
     var argCount = argTypes.length - 2;
@@ -220,11 +242,11 @@ var LibraryEmbindShared = {
     argsList = argsList.join(',')
     argsListWired = argsListWired.join(',')
 
-    var invokerFnBody = `
-      return function (${argsList}) {
-      if (arguments.length !== ${argCount}) {
-        throwBindingError('function ' + humanName + ' called with ' + arguments.length + ' arguments, expected ${argCount}');
-      }`;
+    var invokerFnBody = `return function (${argsList}) {\n`;
+
+#if ASSERTIONS
+    invokerFnBody += "checkArgCount(arguments.length, minArgs, maxArgs, humanName, throwBindingError);\n";
+#endif
 
 #if EMSCRIPTEN_TRACING
     invokerFnBody += `Module.emscripten_trace_enter_context('embind::' + humanName );\n`;
@@ -295,6 +317,7 @@ var LibraryEmbindShared = {
     invokerFnBody += "}\n";
 
 #if ASSERTIONS
+    args1.push('checkArgCount', 'minArgs', 'maxArgs');
     invokerFnBody = `if (arguments.length !== ${args1.length}){ throw new Error(humanName + "Expected ${args1.length} closure arguments " + arguments.length + " given."); }\n${invokerFnBody}`;
 #endif
     return [args1, invokerFnBody];

test/code_size/embind_hello_wasm.json

@@ -1,10 +1,10 @@
 {
   "a.html": 552,
   "a.html.gz": 380,
-  "a.js": 9910,
-  "a.js.gz": 4352,
+  "a.js": 9727,
+  "a.js.gz": 4295,
   "a.wasm": 7728,
   "a.wasm.gz": 3519,
-  "total": 18190,
-  "total_gz": 8251
+  "total": 18007,
+  "total_gz": 8194
 }

test/embind/embind.test.js

@@ -1294,6 +1294,20 @@ module({
             value = cm.embind_test_optional_small_class_arg(undefined);
             assert.equal(-1, value);
         });
+
+        test("std::optional args can be omitted", function() {
+            if (cm.getCompilerSetting('ASSERTIONS')) {
+                // Argument length is only validated with assertions enabled.
+                assert.throws(cm.BindingError, function() {
+                    cm.embind_test_optional_multiple_arg();
+                });
+                assert.throws(cm.BindingError, function() {
+                    cm.embind_test_optional_multiple_arg(1, 2, 3, 4);
+                });
+            }
+            cm.embind_test_optional_multiple_arg(1);
+            cm.embind_test_optional_multiple_arg(1, 2);
+        });
     });
 
     BaseFixture.extend("functors", function() {

test/embind/embind_test.cpp

@@ -1377,6 +1377,7 @@ int embind_test_optional_small_class_arg(std::optional<SmallClass> arg) {
     }
     return -1;
 }
+void embind_test_optional_multiple_arg(int arg1, std::optional<int> arg2, std::optional<int> arg3) {}
 #endif
 
 val embind_test_getglobal() {
@@ -2412,6 +2413,7 @@ EMSCRIPTEN_BINDINGS(tests) {
     function("embind_test_optional_float_arg", &embind_test_optional_float_arg);
     function("embind_test_optional_string_arg", &embind_test_optional_string_arg);
     function("embind_test_optional_small_class_arg", &embind_test_optional_small_class_arg);
+    function("embind_test_optional_multiple_arg", &embind_test_optional_multiple_arg);
 #endif
 
     register_map<std::string, int>("StringIntMap");