charto / nbind

:sparkles: Magical headers that make your C++ library accessible from JavaScript :rocket:
MIT License
1.98k stars 119 forks source link

Default parameters become mandatory #53

Open arcanis opened 7 years ago

arcanis commented 7 years ago

I hope you won't mind me opening a shitload of issues here! 😅

Apparently, optional parameters become mandatory after being compiled. The V8 codepath triggers a "not enough parameters" error, which is good enough for most debugging purposes, but the emscripten codepath doesn't do anything special and silently ignores the default value, which can break any code relying on them.

The easy workaround is to override the JS methods of the affected prototypes to do this check, before calling the original function.

jjrv commented 7 years ago

For performance reasons, the Emscripten target skips many checks done on the V8 target. In asm.js code the tests would often require a wrapper causing an additional function call and making the code bigger. Also, it's not "dangerous" to trash the C++ stack and heap in asm.js, while on Node.js it might result in remote exploits.

You're recommended to use TypeScript and ndts to get type checking on the Emscripten target. Then the checks are done at compile time or inside your IDE for faster feedback, and there's no run-time penalty. It's a pretty nice workflow also when targeting Node.js.

In C++, technically default arguments result in different overloads of the function. Normally this would cause the type autodetection system to fail, but with default arguments, it simply only sees the overload with all the arguments. If you don't pass them on Asm.js, it still calls the overload expecting them, so they become undefined. If you're using TypeScript, it won't compile.

As you said the easiest way is to add the default arguments on the JavaScript side. Another option is to use multimethod or multifunction to bind several overloads, but unfortunately currently they all need unique names. This will be fixed later.