Assert with an error message instead of silently return empty object #26
Conversation
| @@ -458,7 +458,10 @@ struct JsInterface { | |||
| // (interface +c) | |||
| template <typename, typename> | |||
| struct GetOrCreateCppProxy { | |||
| em::val operator() (const std::shared_ptr<I>& c) { return em::val::undefined(); } | |||
| em::val operator() (const std::shared_ptr<I>& c) { | |||
| assert(false && "Attempting to pass C++ object but interface lacks +c"); | |||
There was a problem hiding this comment.
This approach of using assert for error handling in wasm code does have a benefit that emscripten bridges asserts back out to JS with reasonable messages and stack traces, rather than just crashing/aborting.
But this has a negative that we are now relying on something that is only defined when NDEBUG is not present. In the current bazel toolchain, if we build with -c opt, NDEBUG is defined, and all these asserts will disappear I believe.
The asserts in the emscripten STL come from here. They do the typical ifdef, where NDEBUG no-ops it. When it isn't defined, depending on your flags, I think it either uses the C++ impl here or bridges out to JS here.
I'm not sure if there's an emscripten equivalent of some of the tricks we usually do to turn on asserts with NDEBUG defined, like __attribute__((noinline)) extern "C" void __assert
When passing JS interface objects but the interfaces lacks "+w", we currently pass an empty object into C++. This causes downstream code to fail silently and it confuses developers.
This change will trigger an assertion in this case with a clear error message about what happened. Also fix the case of passing C++ interface objects without "+c".