Merge pull request emscripten-core#3450 from AlexPerrot/incoming

Fix `undefined` entries in overload table
mwolting · Dec 8, 2016 · f7a02e5 · f7a02e5
2 parents 5909eee + 03c0041
commit f7a02e5
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 0 deletions.
diff --git a/src/embind/embind.js b/src/embind/embind.js
@@ -144,6 +144,7 @@ var LibraryEmbind = {
     }
     else {
         Module[name] = value;
+        Module[name].argCount = numArguments;
     }
   },
 
@@ -1949,6 +1950,8 @@ var LibraryEmbind = {
             // Replace the initial unbound-handler-stub function with the appropriate member function, now that all types
             // are resolved. If multiple overloads are registered for this function, the function goes into an overload table.
             if (undefined === proto[methodName].overloadTable) {
+                // Set argCount in case an overload is registered later
+                memberFunction.argCount = argCount - 2;
                 proto[methodName] = memberFunction;
             } else {
                 proto[methodName].overloadTable[argCount - 2] = memberFunction;

diff --git a/tests/embind/embind.test.js b/tests/embind/embind.test.js
@@ -876,6 +876,34 @@ module({
             p.delete();
         });
 */
+
+        test("no undefined entry in overload table when depending on already bound types", function() {
+            var dummy_overloads = cm.MultipleOverloadsDependingOnDummy.prototype.dummy;
+            // check if the overloadTable is correctly named
+            // it can be minimized if using closure compiler
+            if (dummy_overloads.hasOwnProperty('overloadTable')) {
+                assert.false(dummy_overloads.overloadTable.hasOwnProperty('undefined'));
+            }
+
+            // this part should fail anyway if there is no overloadTable
+            var dependOnDummy = new cm.MultipleOverloadsDependingOnDummy();
+            var dummy = dependOnDummy.dummy();
+            dependOnDummy.dummy(dummy);
+            dummy.delete();
+            dependOnDummy.delete();
+        });
+
+        test("no undefined entry in overload table for free functions", function() {
+            var dummy_free_func = cm.getDummy;
+            console.log(dummy_free_func);
+
+            if (dummy_free_func.hasOwnProperty('overloadTable')) {
+                assert.false(dummy_free_func.overloadTable.hasOwnProperty('undefined'));
+            }
+
+            var dummy = cm.getDummy();
+            cm.getDummy(dummy);
+        });
     });
 
     BaseFixture.extend("vector", function() {

diff --git a/tests/embind/embind_test.cpp b/tests/embind/embind_test.cpp
@@ -2294,6 +2294,27 @@ struct ConstAndNonConst {
     }
 };
 
+class DummyForOverloads {};
+
+class MultipleOverloadsDependingOnDummy {
+public:
+    DummyForOverloads dummy() {
+        return DummyForOverloads();
+    }
+
+    DummyForOverloads dummy(DummyForOverloads d) {
+        return d;
+    }
+};
+
+DummyForOverloads getDummy() {
+    return DummyForOverloads();
+}
+
+DummyForOverloads getDummy(DummyForOverloads d) {
+    return d;
+}
+
 EMSCRIPTEN_BINDINGS(overloads) {
     function("overloaded_function", select_overload<int(int)>(&overloaded_function));
     function("overloaded_function", select_overload<int(int, int)>(&overloaded_function));
@@ -2337,6 +2358,14 @@ EMSCRIPTEN_BINDINGS(overloads) {
     class_<ConstAndNonConst>("ConstAndNonConst")
         .function("method", select_const(&ConstAndNonConst::method))
         ;
+
+    class_<DummyForOverloads>("DummyForOverloads").constructor();
+
+    class_<MultipleOverloadsDependingOnDummy>("MultipleOverloadsDependingOnDummy")
+        .constructor()
+        .function("dummy", select_overload<DummyForOverloads()>(&MultipleOverloadsDependingOnDummy::dummy))
+        .function("dummy", select_overload<DummyForOverloads(DummyForOverloads)>(&MultipleOverloadsDependingOnDummy::dummy))
+        ;
 }
 
 // tests for out-of-order registration