[PATCH] D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names

Sun Dec 1 10:46:12 PST 2019

sbc100 added a comment.

In D70520#1754500 <https://reviews.llvm.org/D70520#1754500>, @sunfish wrote:

> It appears this doesn't handle exporting an imported function yet, which is fine for now, but it would be good to issue a warning, because wasm itself is capable of representing this:
>
>   void aaa(void) __attribute__((import_module("imp"), import_name("foo"), export_name("bar")));
>

Yes, although this is supported at the bitcode level I think the internal APIs would need some refactoring to support this in BinaryFormat and lld.  Thankfully I don't think the actual binary format would need to change.

================
Comment at: lld/wasm/Writer.cpp:523
+      StringRef exportName = f->function->getExportName();
+      if (!exportName.empty()) {
+        name = exportName;
----------------
sunfish wrote:
> For wasm exports, it's valid to have empty strings. In fact, I may even have a usecase which wants an empty-string export. It'd be good to use `Optional<>` for export names, rather than special-casing the empty string.
> 
> (wasm-ld does often special-case the empty string in symbol names, but wasm export strings aren't ordinary symbol names, so it'd be good to follow wasm's rules for them.)
This is a fairly widespread problem within the current codebase that applies to both import and export names.  If its OK with you I'll do this as a followup and at tests for empty string imports and exports.  I imagine it will require fairly widespread by simple changes.   

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70520/new/

https://reviews.llvm.org/D70520