Skip to content

Minify wasi imports and exports, and not just "env"#2323

Merged
kripken merged 2 commits intomasterfrom
minwasi
Sep 2, 2019
Merged

Minify wasi imports and exports, and not just "env"#2323
kripken merged 2 commits intomasterfrom
minwasi

Conversation

@kripken
Copy link
Copy Markdown
Member

@kripken kripken commented Sep 1, 2019

This makes the minification pass aware of wasi_unstable and wasi as well.

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Sep 2, 2019

Landing quickly as this blocks testing more interesting things with wasi on our CI, and is probably noncontroversial, but please review later.

@kripken kripken merged commit 6cec37e into master Sep 2, 2019
@kripken kripken deleted the minwasi branch September 2, 2019 00:51
Comment thread src/shared-constants.h
extern Name EVENT;
extern Name ATTR;
extern Name WASI;
extern Name WASI_UNSTABLE;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These probably don't belong in a header unless they are at least used by multiple source files.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think one of my next PRs will use them, as I add more wasi support. If it turns out no then I can remove these.

Comment thread src/wasm/wasm.cpp
Name EVENT("event");
Name ATTR("attr");
Name WASI("wasi");
Name WASI_UNSTABLE("wasi_unstable");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It premature to add WASI here, unless I'm missing something. The plan is not to go from wasi_unstable to just wasi, but rather split it into modules.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I didn't know that, will fix.

module->updateMaps();
// Emit the mapping.
for (auto& pair : oldToNew) {
for (auto& pair : newToOld) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you now outputting the mapping in reverse?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code was actually wrong before - it was called oldToNew but had the opposite data ;) This PR also corrects that.

sbc100 pushed a commit that referenced this pull request Sep 4, 2019
Remove wasi, as only wasi_unstable makes sense. Also remove shared constant for wasi as we don't know yet if it'll be needed later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants