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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
