Skip to content
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

Add documentation to BinaryenIf #6870

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexdovzhanyn
Copy link

@alexdovzhanyn alexdovzhanyn commented Aug 22, 2024

Adds a comment to explain that a (result) is only added to BinaryenIf statements if both arms return concrete types.

Relates to #6868

@kripken
Copy link
Member

kripken commented Aug 26, 2024

Thanks for the PR!

I'm not sure where the best place to put this documentation is. We should perhaps point to wasm-builder.h or wasm.cpp as containing the implementations of the expression constructors. That is, I'm not sure if it's worth duplicating such docs into the C API, though at the same, the wasm-builder.h/wasm.cpp code is more code and not documentation, so maybe we need a new place for it..?

@alexdovzhanyn
Copy link
Author

@kripken I'm happy to move it wherever you'd like! I put it in the header file for the binaryen api directly as I see that as the point of interface for most people who are using the binaryen api -- but I could be wrong. That is, as I've been building my application using Binaryen, I've been referring to the header file as the "source of truth" for how things work.

It may be more tedious / less discoverable if it is moved into the source of the implementation itself

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