Skip to content

[wasm-split] End module names with : in manifests#8003

Merged
aheejin merged 4 commits into
WebAssembly:mainfrom
aheejin:manifest_colon
Nov 19, 2025
Merged

[wasm-split] End module names with : in manifests#8003
aheejin merged 4 commits into
WebAssembly:mainfrom
aheejin:manifest_colon

Conversation

@aheejin
Copy link
Copy Markdown
Member

@aheejin aheejin commented Oct 27, 2025

This makes better distinction of module names and function lists. Suggested by by @sbc100 (emscripten-core/emscripten#25577 (comment)).

Currently this supports both module names with a : and without it for backwards compatibility and also to pass the CI.

Suggested by by @sbc100
(emscripten-core/emscripten#25577 (comment)).

Currently this supports both module names with a `:` and without it for
backwards compatibility and also to pass the CI.
@aheejin aheejin requested review from sbc100 and tlively October 27, 2025 14:47
aheejin added a commit to aheejin/emscripten that referenced this pull request Oct 27, 2025
This makes better distinction of module names and function lists.
Suggested by by @sbc100
(emscripten-core#25577 (comment)).

This has to land after
WebAssembly/binaryen#8003.
Comment thread src/support/istring.h

IString substr(size_t pos, size_t len = std::string_view::npos) const {
return IString(str.substr(pos, len));
}
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.

Has binararyen really gone this long without a substr() or endswith() operations on strings?

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.

Apparently 🤷🏻

Name name = WasmBinaryReader::escape(line);
if (newSection) {
if (name.endsWith(":")) {
name = name.substr(0, name.size() - 1);
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.

You could be clever and allow (0, -1) here like in high level languages?

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.

In basic_string_view::substr, the second argument is a count, the number of characters, so I think making the meaning of IString::substr different will be confusing. If we want that behavior we can add a separate function like slice(size_t start, size_end) or something. But we only have a single use here now, so I'm not sure if that's necessary at this point?

@aheejin
Copy link
Copy Markdown
Member Author

aheejin commented Nov 17, 2025

Ping

Copy link
Copy Markdown
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

Sorry for the delay!

@aheejin aheejin merged commit 0c63d8b into WebAssembly:main Nov 19, 2025
16 checks passed
@aheejin aheejin deleted the manifest_colon branch November 19, 2025 01:56
aheejin added a commit to emscripten-core/emscripten that referenced this pull request Nov 26, 2025
This makes better distinction of module names and function lists.
Suggested by by @sbc100
(#25577 (comment)).

This has to land after
WebAssembly/binaryen#8003.
inolen pushed a commit to inolen/emscripten that referenced this pull request Feb 13, 2026
…5659)

This makes better distinction of module names and function lists.
Suggested by by @sbc100
(emscripten-core#25577 (comment)).

This has to land after
WebAssembly/binaryen#8003.
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.

3 participants