Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp | ||
---|---|---|
136 ↗ | (On Diff #78596) | nit: You can leave out the braces here. |
This is ok, for now at least.
I was imagining that we'd eventually want to just take all undefined global symbols and automatically generate imports for them. Do you think this would work?
In my mind this makes it more explicit what the intent is, whereas auto-generating imports makes it unclear whether an undefined symbol is an error, or if it will be linked in, and we won't know until run-time. Even then we won't know whether the module is trying to import an invalid symbol, or whether the environment isn't providing that symbol. To me this makes it clearer what the module expects.
Although long-term we want to be outputting nearly-valid wasm as part of our .o format. I think at that point we'd still want to explicitly model imports/exports.
I think this is fine for now too. For the longer term, I kind of think it depends on the use case.
For the case of something that looks like a traditional self-contained C program, I think we'd want to have undefined symbols behave like they do in traditional toolchains (which will be less surprising, and diagnose errors at link time rather than at load time).
For dylibs meant to be loaded by LLVM-built "main" modules, we probably just want something explicit (in the source) like dllimport or ELF visibility or whatever.
For a new/hybrid use case of a wasm library meant to be used from JS, that will be sort of like linking a dylib but not exactly (may have a different ABI, etc).
For the dylib/JS-module use case it might be useful to have the linker automatically convert undefined symbols to imports (as is the default in ELF for example). Although given the popularity and usefulness of -fvisibility=hidden, maybe it's not what we actually want. In any case I think we may want some explicit way to model it in the contract between the compiler and the linker, to at least allow for different ways to make it work? Anyway, we should probably discuss more in a github issue or something.