-
Notifications
You must be signed in to change notification settings - Fork 868
[wasm-merge] Make empty function names explicit with --output-manifest #8871
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -852,10 +852,15 @@ Input source maps can be specified by adding an -ism option right after the modu | |||||||
| // The functions in the module have been renamed and copied rather than | ||||||||
| // moved, so we can get their final names directly. (We don't need this | ||||||||
| // for the first module because it does not appear in the manifest.) | ||||||||
| auto& funcs = moduleFuncs[inputFileName]; | ||||||||
| for (auto& func : currModule->functions) { | ||||||||
| if (!func->imported()) { | ||||||||
| funcs.push_back(func->name); | ||||||||
| if (!manifestFile.empty()) { | ||||||||
| auto& funcs = moduleFuncs[inputFileName]; | ||||||||
| for (auto& func : currModule->functions) { | ||||||||
| if (!func->imported()) { | ||||||||
| funcs.push_back(func->name); | ||||||||
| // Even if the function name is empty, if we were to put it in the | ||||||||
| // output manifest, it has to be emitted in the name section. | ||||||||
| merged.getFunction(func->name)->hasExplicitName = true; | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The function might have been renamed when it was copied to the merged module, so
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renaming is done in in-place binaryen/src/tools/wasm-merge.cpp Lines 647 to 649 in d39a6d8
|
||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| ;; RUN: wasm-merge %s first %s second --rename-export-conflicts --output-manifest %t.manifest -o %t.wasm | ||
| ;; RUN: cat %t.manifest | filecheck %s | ||
| ;; RUN: wasm-dis %t.wasm -o - | filecheck %s --check-prefix MERGED | ||
|
|
||
| ;; This tests if the internal names of empty function names are correctly | ||
| ;; preserved in the name section so that they can match the names in the | ||
| ;; manifest file. | ||
|
|
||
| ;; CHECK: second | ||
| ;; CHECK-NEXT: 0_1 | ||
| ;; CHECK-NEXT: | ||
|
|
||
| ;; MERGED: (module | ||
| ;; MERGED-NEXT: (type $0 (func)) | ||
| ;; MERGED-NEXT: (export "foo" (func $0)) | ||
| ;; MERGED-NEXT: (export "foo_1" (func $0_1)) | ||
| ;; MERGED-NEXT: (func $0 | ||
| ;; MERGED-NEXT: (nop) | ||
| ;; MERGED-NEXT: ) | ||
| ;; MERGED-NEXT: (func $0_1 | ||
| ;; MERGED-NEXT: (nop) | ||
| ;; MERGED-NEXT: ) | ||
| ;; MERGED-NEXT: ) | ||
|
|
||
| (module | ||
| (func (export "foo") | ||
| nop | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NFC: I wrapped the whole
forloop withif(!manifstFile.empty()), given thatfuncswill only be used for the output manifest file generation.