Skip to content

Embind: name functions in non-dynamic mode too #20498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 25, 2023
Merged

Embind: name functions in non-dynamic mode too #20498

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bae62e3
Embind: name functions in non-dynamic mode too
RReverser Oct 19, 2023
b8fb1fd
Update test
RReverser Oct 19, 2023
75b0333
Add dependency
RReverser Oct 20, 2023
edc34a8
Add stub for embind_ts
RReverser Oct 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/embind/embind.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,7 @@ var LibraryEmbind = {
// (hand-written JS code) -> (autogenerated JS invoker) -> (template-generated C++ invoker) -> (target C++ function)
// craftInvokerFunction generates the JS invoker function for each function exposed to JS through embind.
$craftInvokerFunction__deps: [
'$makeLegalFunctionName', '$runDestructors', '$throwBindingError',
'$createNamedFunction', '$runDestructors', '$throwBindingError',
#if DYNAMIC_EXECUTION
'$newFunc',
#endif
Expand Down Expand Up @@ -814,7 +814,7 @@ var LibraryEmbind = {
var argsWired = new Array(expectedArgCount);
var invokerFuncArgs = [];
var destructors = [];
return function() {
var invokerFn = function() {
if (arguments.length !== expectedArgCount) {
throwBindingError(`function ${humanName} called with ${arguments.length} arguments, expected ${expectedArgCount}`);
}
Expand Down Expand Up @@ -878,7 +878,7 @@ var LibraryEmbind = {
}

var invokerFnBody = `
return function ${makeLegalFunctionName(humanName)}(${argsList}) {
return function (${argsList}) {
if (arguments.length !== ${argCount - 2}) {
throwBindingError('function ${humanName} called with ' + arguments.length + ' arguments, expected ${argCount - 2}');
}`;
Expand Down Expand Up @@ -962,8 +962,9 @@ var LibraryEmbind = {

args1.push(invokerFnBody);

return newFunc(Function, args1).apply(null, args2);
var invokerFn = newFunc(Function, args1).apply(null, args2);
#endif
return createNamedFunction(humanName, invokerFn);
},

$embind__requireFunction__deps: ['$readLatin1String', '$throwBindingError'
Expand Down Expand Up @@ -1835,7 +1836,7 @@ var LibraryEmbind = {
basePrototype = ClassHandle.prototype;
}

var constructor = createNamedFunction(legalFunctionName, function() {
var constructor = createNamedFunction(name, function() {
if (Object.getPrototypeOf(this) !== instancePrototype) {
throw new BindingError("Use 'new' to construct " + name);
}
Expand Down
1 change: 1 addition & 0 deletions src/embind/embind_ts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,7 @@ var LibraryEmbind = {
$makeLegalFunctionName: () => assert(false, 'stub function should not be called'),
$newFunc: () => assert(false, 'stub function should not be called'),
$runDestructors: () => assert(false, 'stub function should not be called'),
$createNamedFunction: () => assert(false, 'stub function should not be called'),
};

extraLibraryFuncs.push('$embindEmitTypes');
Expand Down
9 changes: 4 additions & 5 deletions src/embind/emval.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ var LibraryEmVal = {

_emval_get_method_caller__deps: [
'$emval_addMethodCaller', '$emval_lookupTypes',
'$makeLegalFunctionName',
'$createNamedFunction',
'$reflectConstruct', '$emval_returnValue',
#if DYNAMIC_EXECUTION
'$newFunc',
Expand Down Expand Up @@ -343,10 +343,8 @@ var LibraryEmVal = {
return emval_returnValue(retType, destructorsRef, rv);
};
#else
var signatureName = retType.name + "_$" + types.map(t => t.name).join("_") + "$";
var functionName = makeLegalFunctionName("methodCaller_" + signatureName);
var functionBody =
`return function ${functionName}(obj, func, destructorsRef, args) {\n`;
`return function (obj, func, destructorsRef, args) {\n`;

var offset = 0;
var argsList = []; // 'obj?, arg0, arg1, arg2, ... , argN'
Expand Down Expand Up @@ -384,7 +382,8 @@ var LibraryEmVal = {
params.push(functionBody);
var invokerFunction = newFunc(Function, params).apply(null, args);
#endif
return emval_addMethodCaller(invokerFunction);
var functionName = `methodCaller<(${types.map(t => t.name).join(', ')}) => ${retType.name}>`;
return emval_addMethodCaller(createNamedFunction(functionName, invokerFunction));
},

_emval_call_method__deps: ['$getStringOrSymbol', '$emval_methodCallers', '$Emval'],
Expand Down
19 changes: 7 additions & 12 deletions test/embind/embind.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ module({
var e = cm.emval_test_take_and_return_std_string((new Int8Array([65, 66, 67, 68])).buffer);
assert.equal('ABCD', e);
});

test("can pass Uint8Array to std::basic_string<unsigned char>", function() {
var e = cm.emval_test_take_and_return_std_basic_string_unsigned_char(new Uint8Array([65, 66, 67, 68]));
assert.equal('ABCD', e);
Expand Down Expand Up @@ -475,7 +475,7 @@ module({
var e = cm.emval_test_take_and_return_std_string(string);
assert.equal(string, e);
});

var utf16TestString = String.fromCharCode(10) +
String.fromCharCode(1234) +
String.fromCharCode(2345) +
Expand Down Expand Up @@ -517,12 +517,12 @@ module({
cm.force_memory_growth();
assert.equal("get_literal_wstring", cm.get_literal_wstring());
});

test("can access a literal u16string after a memory growth", function() {
cm.force_memory_growth();
assert.equal("get_literal_u16string", cm.get_literal_u16string());
});

test("can access a literal u32string after a memory growth", function() {
cm.force_memory_growth();
assert.equal("get_literal_u32string", cm.get_literal_u32string());
Expand Down Expand Up @@ -2078,7 +2078,7 @@ module({
assert.equal(10, instance.property);
instance.delete();
});

test("pass derived object to c++", function() {
var Implementation = cm.AbstractClass.extend("Implementation", {
abstractMethod: function() {
Expand Down Expand Up @@ -2516,13 +2516,8 @@ module({
BaseFixture.extend("function names", function() {
assert.equal('ValHolder', cm.ValHolder.name);

if (!cm.getCompilerSetting('DYNAMIC_EXECUTION')) {
assert.equal('', cm.ValHolder.prototype.setVal.name);
assert.equal('', cm.ValHolder.makeConst.name);
} else {
assert.equal('ValHolder$setVal', cm.ValHolder.prototype.setVal.name);
assert.equal('ValHolder$makeConst', cm.ValHolder.makeConst.name);
}
assert.equal('ValHolder.setVal', cm.ValHolder.prototype.setVal.name);
assert.equal('ValHolder.makeConst', cm.ValHolder.makeConst.name);
});

BaseFixture.extend("constants", function() {
Expand Down