BinaryenSetFunctionTable cannot (safely) accept imported functions

[jayphelps]

In wasm you're allowed to indirectly call imported functions, so their index must be in the function table. However Binaryen's C-API BinaryenSetFunctionTable currently only safely supports passing an array of BinaryenFunctionRef, which are defined with BinaryenAddFunction(). Function imports are BinaryenImportRefs.

If you're using C++ you could reach behind the curtain and add the table segments yourself, however when this is isn't possible you're only other option is to unsafely cast BinaryenImportRef to BinaryenFunctionRef which actually works because both wasm::Function and wasm::Import happen to have std::string name as their first field, but it might not always be that way so this is pretty unsafe.

BinaryenModuleRef module = BinaryenModuleCreate();
BinaryenFunctionTypeRef fun_type = BinaryenAddFunctionType(module, NULL, BinaryenTypeNone(), NULL, 0);
BinaryenImportRef fun_import = BinaryenAddFunctionImport(module, "example", "env", "example", fun_type);

// YOLO, this works because wasm::Function and wasm::Import both have std::string name as
// their first field and that's all that matters, but it might not always be that way
BinaryenFunctionRef fun = (BinaryenFunctionRef)fun_import;

BinaryenFunctionRef fun_table[] = { fun };
BinaryenSetFunctionTable(module, fun_table, 1);

Since the name of the function is what is actually used, perhaps BinaryenSetFunctionTable could be changed to accept const char ** instead?

---

btw--for my future reference how does binaryen handle breaking changes? haha

[kripken]

I agree, this looks like it should be fixed.

We don't have a strong policy about breaking changes in the C API. I think it's ok to do for something like this (where there isn't a reasonable alternative), but we should start a Changelog document (like say emscripten has) and mention it there so that people upgrading can easily get a list of breaking changes.