Skip to content

wit-component, wit-parser: preserve doc comments through embed path#2532

Open
Mees-Molenaar wants to merge 2 commits into
bytecodealliance:mainfrom
Mees-Molenaar:preserve-wit-docs-through-embed
Open

wit-component, wit-parser: preserve doc comments through embed path#2532
Mees-Molenaar wants to merge 2 commits into
bytecodealliance:mainfrom
Mees-Molenaar:preserve-wit-docs-through-embed

Conversation

@Mees-Molenaar
Copy link
Copy Markdown

For our use case, we want to store some metadata in comments in the wit file. According to the documentation, this should be available. However, we encountered that the embedding path (which is used by wit-bindgen) did not include the comments. This PR fixes that.

@Mees-Molenaar Mees-Molenaar requested a review from a team as a code owner May 28, 2026 13:06
@Mees-Molenaar Mees-Molenaar requested review from dicej and removed request for a team May 28, 2026 13:06
@alexcrichton alexcrichton requested review from alexcrichton and removed request for dicej May 28, 2026 14:26
@alexcrichton
Copy link
Copy Markdown
Member

Thanks! Given the test failures I think it might make sense to ignore errors when decoding this section rather than propagating them up? That looks to be masking other errors and/or causing minor issues.

@Mees-Molenaar
Copy link
Copy Markdown
Author

Thanks for quick review, I agree with your comment.
I have adjusted it, is this what you had in mind?

I only see that an offset is now not matching because of an offset. Is that something I need to commit, or do I need to change the implementation so that the offset keeps the same?

@alexcrichton
Copy link
Copy Markdown
Member

Oh offsets changing is fine, that's because there's more stuff in the wasm module now the docs). You can fix that by running the test suite with BLESS=1 for that. Also, could you add some comments for why the errors are ignored here?

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