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.

Sign up for GitHub

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

Jump to bottom

Add an option shorthand to witx. #420

Closed
wants to merge 1 commit into from
Closed

Add an option shorthand to witx. #420

wants to merge 1 commit into from

Conversation

sunfishcode
Copy link
Member

I was mocking up a witx description for #322 and had a use for an
option type. Of course, I could also just emit a variant manually,
or reuse expected by omitting the error type, but option better
conveys the sense that the none case doesn't necessarily represent
an error.

@sunfishcode
Add an option shorthand to witx.
517829e 
I was mocking up a witx description for #322 and had a use for an
`option` type. Of course, I could also just emit a variant manually,
or reuse `expected` by omitting the error type, but `option` better
conveys the sense that the `none` case doesn't necessarily represent
an error.
@jedisct1
Copy link
Member

That would be a nice addition!

In wasi-crypto, a custom Option type is also used everywhere, which leads to non-idiomatic generated code in languages supporting optional types.

@alexcrichton
Copy link
Contributor

The code here looks fine to me, but I would hesitate because I'm not sure whether we want to continue to enhance the current ABI of WASI since it makes it more complicated to support in a future with support for a new ABI. I've been working on an abi-next branch locally which also has support for option, and I definitely agree it would be nice to support!

Do you think it would be better for me to try to land by "abi next" work and get option supported only there? Or do you think that it would be best to stick with this extension to the current ABI?

@sunfishcode
Copy link
Member Author

Ah, cool. I'm ok waiting for the new ABI; this was just for some prototyping I was doing.

@sunfishcode sunfishcode deleted the sunfishcode/option branch March 29, 2021 14:49
@alexcrichton
Copy link
Contributor

Ok! I'll work on cleaning that up for a PR this week.

@alexcrichton
Copy link
Contributor

Ok I posted #422 which has option in it as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexcrichton alexcrichton Awaiting requested review from alexcrichton

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sunfishcode @jedisct1 @alexcrichton