Skip to content

[NFC][wasm-reduce] Use byte vectors for I/O - #9117

Open
tlively wants to merge 2 commits into
reduce-js-refactor-helpersfrom
reduce-js-refactor-0
Open

tlively wants to merge 2 commits into
reduce-js-refactor-helpersfrom
reduce-js-refactor-0

Conversation

@tlively

@tlively tlively commented Sep 18, 2026

Copy link
Copy Markdown
Member

This refactoring prepares for a follow-on PR that will abstract how the module bytes are read from and written to the test and working files to support reducing Wasm modules embedded in JS files.

This refactoring prepares for a follow-on PR that will abstract how the module bytes are read from and written to the test and working files to support reducing Wasm modules embedded in JS files.
@tlively
tlively requested a review from kripken September 18, 2026 19:18
@tlively
tlively requested a review from a team as a code owner September 18, 2026 19:18
Comment thread src/tools/wasm-reduce/wasm-reduce.cpp Outdated
const std::vector<char>& bytes) {
Output out(filename, Flags::Binary);
out.write(bytes.data(), bytes.size());
}

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.

If it is so useful to make a helper for this, should it be in support/file.h as a static function so other files can use it? (i.e. this doesn't seem more useful here than elsewhere)

if (result.failed()) {
return false;
}
outBytes = read_file<std::vector<char>>(testPath, Flags::Binary);

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.

i mean that we have read_file alreadyinfile.h, and write_file` may make sense too?

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.

Yep, makes sense.

@kripken kripken left a comment

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.

lgtm otherwise

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