Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve the OutParam API for wasi-http #1940

Merged
merged 1 commit into from
Oct 25, 2023
Merged

Conversation

rylev
Copy link
Collaborator

@rylev rylev commented Oct 25, 2023

This API is a bit awkward but this makes it slightly better by allowing method call syntax. I'm not sure I fully understand why the wit-bindgen generated version does allow for method call syntax.

Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
@rylev rylev requested a review from dicej October 25, 2023 16:21
Copy link
Contributor

@dicej dicej left a comment

Choose a reason for hiding this comment

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

WIT has three types of functions that can be associated with resources: constructors, methods, and static methods. Methods always take their implicit self/this parameter by borrow handle. Static methods don't have an implicit self/this parameter. There's simply no way (currently) to associate a non-static method with a resource that takes its self/this parameter by own handle, so the closest thing you can do is define a static method that has an explicit self/this parameter as an own handle.

We could add some kind of heurstic to wit-bindgen to detect that pattern and turn it into a method that takes self by owned value, but for now we treat all static methods as associated functions that do not have a real self parameter.

@rylev rylev requested a review from itowlson October 25, 2023 16:37
@rylev rylev merged commit 5376331 into main Oct 25, 2023
9 checks passed
@rylev rylev deleted the response-out-improvement branch October 25, 2023 20:48
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants